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

fix(graphics): only show the button if the graphics can be switched #174

Merged
merged 2 commits into from
Dec 19, 2023

Conversation

wash2
Copy link
Contributor

@wash2 wash2 commented Dec 14, 2023

should fix #173. This makes the button press-able if the graphics can be switched.

@wash2 wash2 requested a review from a team December 14, 2023 16:06
@jacobgkau
Copy link
Member

Confirmed this does make the button unclickable (which means it also doesn't highlight on mouse-over) on an integrated system. The icon and the word Integrated still show up by default. Not sure if @pop-os/ux would want to take a look at this to ensure it's not potentially confusing. (This would probably still be an improvement over the non-functional menu, even if it was confusing.)

@maria-komarova
Copy link

If there are no modes available to switch, then the applet shouldn't show on the panel. The applet should only be present for systems where the modes are available.

@wash2
Copy link
Contributor Author

wash2 commented Dec 15, 2023

Ok this should exit now when the graphics are not switchable

@jacobgkau
Copy link
Member

jacobgkau commented Dec 15, 2023

This now causes the button to not show up on an integrated system.

This currently does cause an extra bit of space to appear between the surrounding icons where the applet is positioned (possibly just 1px, looking at the code?) See below for a comparison of the Graphics applet being in two positions (open them in tabs and flip between them to see the difference):

Screenshot_2023-12-15_13-51-48
Screenshot_2023-12-15_13-51-55

The space is also possible to find by moving the mouse across all the icons and seeing where there's a larger gap between the hover states.

@wash2
Copy link
Contributor Author

wash2 commented Dec 15, 2023

Hmm ok Maybe the panel is not updating the layout after it exits? I'll test it and maybe make a fix for the panel.

@wash2
Copy link
Contributor Author

wash2 commented Dec 18, 2023

pop-os/cosmic-panel#79 should fix the gap. It wasn't removing the spacing for the removed applet after it exited.

@jacobgkau jacobgkau requested a review from a team December 19, 2023 02:15
Copy link
Member

@jacobgkau jacobgkau left a comment

Choose a reason for hiding this comment

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

Confirmed that issue is fixed; on a non-switchable system, the panel now looks identical regardless of the graphics applet's position, and there's no extra space between icons where that applet is:

Screenshot_2023-12-18_19-11-47
Screenshot_2023-12-18_19-11-55

I've also confirmed the menu still shows up as expected on a system with switchable graphics.

@jacobgkau jacobgkau changed the title fix(graphics): only make the button pressable if the graphics can be switched fix(graphics): only show the button if the graphics can be switched Dec 19, 2023
@wash2 wash2 merged commit 3c863ed into master_jammy Dec 19, 2023
7 checks passed
@wash2 wash2 deleted the graphics-switchable_jammy branch December 19, 2023 04:27
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.

Graphics applet shows nonexistant options on non-switchable hardware
4 participants