Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add driver view #300

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from
Draft

Add driver view #300

wants to merge 10 commits into from

Conversation

leolost2605
Copy link
Member

@leolost2605 leolost2605 commented Feb 17, 2024

UI side of elementary/settings-daemon#132

Screenshot from 2024-02-18 18 52 10

I modeled this after the firmware view. Any suggestions are welcome
Unfortunately I think we can't do anything about the icons :(

@leolost2605 leolost2605 marked this pull request as ready for review February 18, 2024 00:45
Copy link
Member

@danirabbit danirabbit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very excited about this, fantastic work! Left a couple of inline comments.

For the overarching design direction, in looking at other driver managers it looks like maybe a better way to list drivers would be to have a toplevel item that represents the device and then a list of radio buttons that represent the available driver options. For example here's Ubuntu drivers:

software-updates-drivers-781529376

Object (
icon: new ThemedIcon ("application-x-firmware"),
title: _("Drivers"),
description: _("Additional drivers provided by device manufacturers can improve performance.")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can copy the description text from Installer:

Suggested change
description: _("Additional drivers provided by device manufacturers can improve performance.")
description: _("Broadcom® Wi-Fi adapters, NVIDIA® graphics, and some virtual machines may not function properly without additional drivers.")


construct {
var none_placeholder = new Granite.Placeholder (_("No drivers available")) {
description = _("Your system doesn't need any additional drivers."),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We usually try to avoid "you" forms for localization. I think also "system" is usually more used to refer to the operating system software, and "device" is usually more used to refer to the computer hardware

Suggested change
description = _("Your system doesn't need any additional drivers."),
description = _("This device doesn't need any additional drivers."),

xalign = 0
};

var install_button = new Gtk.Button.with_label (installed ? _("Installed") : _("Install")) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it might be more appropriate to use a radio here to make it more clear that this is a mutually exclusive selection

@leolost2605
Copy link
Member Author

@danirabbit ok I created a different branch (#309) with radio buttons. It (or rather settings daemon side of it) has a larger surface for errors so it should be tested carefully.

@leolost2605 leolost2605 marked this pull request as draft February 18, 2024 22:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants