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

feat(desktop): Set sidebar to visible child #187

Closed
wants to merge 87 commits into from

Conversation

mmstick
Copy link
Member

@mmstick mmstick commented Jul 8, 2021

Closes #174

This commit includes all the changes that seem to be necessary for
CcKeyboardItem to be used for dealing with multiple keybindings, without
(yet) changing the user interface to expose this.

The `primary_combo` and `binding` fields of `CcKeyboardItem` are
removed, in favor of the existing `key_combos`. No combination is
"primary", since all of them can now be seen and changed equally.

We treat `CcKeyboardItem.key_combos` as a set, that a combo can be added
to or removed from. Though it continues to be represented as a `GList`,
instead of a `GHashTable`, to preserve ordering.

A lot of the keyboard panel code relied on the assumption that only one
combo can be set for each setting, so this required a variety of
miscellaneous changes.
This adds a widget called `CcKeyboardShortcutRow`, sub-classing
`GtkListBoxRow`, to handle a shortcut row. This makes the implementation
a bit tidier, rather than handling it all in `CcKeyboardPanel`, and
allows the widgets that compose the row to be laid out in xml.

This is a prerequisite for moving the shortcuts to a new dialog.
For some reason, the ungrab was making the window impossible to interact
with. Without these calls, it seems to work as expected.
Previously, there were multiple redundant lists of possible values for
this setting. This uses one struct to define not only all the possible
values, but also anything specific to the alternate characters key, so
this can be used for other modifiers.

This also changes the style of the dialog to use a GtkListBox.
This was previously available only in Gnome Tweaks.
Implementation borrowed from cc-search-panel-row.c
Copied and pasted older shortcut dialog and row, then fixed issues.

May have some problems.
This seems to avoid frezing. I think this was meant for symmetry with
grab_seat()... let's see if this works as expected.
@bflanagin
Copy link

@mmstick It looks like this PR only fixes one sub-panel (Backgrounds) in the Desktop settings. From what I understand #174 wouldn't be closed as it refers to "Dock" directly and all other sub-panels indirectly. It does fix pop-os/beta#152, but I'm not sure if that is intended.

@jacobgkau
Copy link
Member

@bflanagin Not sure if this is what you mean, but I see that searching within Settings, all five sub-pages return Desktop as a result, while the Privacy panel's subsections return their own names as the result:

Screenshot from 2021-07-08 14-15-32
Screenshot from 2021-07-08 14-15-36
Screenshot from 2021-07-08 14-15-39

Search for the Launcher should be fixed with pop-os/desktop-widget#31 once it's fixed, but I don't think that will help for the search within Settings.

Regarding pop-os/beta#152, if I already have another desktop-widget page selected (like Dock), then Change Background... does take me to the Background page; but if I have a different Settings page selected (like Bluetooth), then Change Background... still brings me to the General page instead of the Background page.

@mmstick
Copy link
Member Author

mmstick commented Jul 8, 2021

This and the other PR make the panels searchable in the launcher. It wouldn't yet work inside settings itself.

@bflanagin
Copy link

From the launcher
Working: Tiling,Dock,Workspaces,Appearance, Cosmic

Mostly working: Background

Notes: Using the keywords: desktop,screens,wallpaper in the launcher displays Desktop (linked to Background), which is desirable if a little confusing, where as the keyword "background" displays all available options in the Desktop settings widget, instead of just the Background and possibly Desktop items. Screenshot for reference:
Screenshot from 2021-07-13 11-12-12

Also there is a small disconnect between using the word Cosmic to get to the Desktop->General view when we call the section "Desktop" in GCC. This is made even more confusing in conjunction with how the launcher displays "Desktop" (which goes to Backgrounds) when you search for Workspaces,or Dock and not Cosmic. I imagine removing the Desktop option from the search results will fix the latter, but I'm not sure what the best option is for the former. Would it be against the grain to change the top level menu item from Desktop to Cosmic?

@mmstick
Copy link
Member Author

mmstick commented Jul 13, 2021

We can change Desktop, but that would be in gnome-control-center, since that is where that desktop entry presides.

@bflanagin
Copy link

bflanagin commented Jul 21, 2021

I'm cool with keeping it Cosmic for now if it makes merging this PR easier. The only thing that really stands out as "unintended behavior" is the background keyword displaying every option (as mentioned above.) I don't know how trivial the change is, but at the moment its the only blocker.

@bflanagin
Copy link

Looking at the code here and in https://github.com/pop-os/desktop-widget/pull/31/files I realize the issue with the "background" keyword isn't directly connected with either PR and have created an issue for it. I'll create an issue about the naming miss match as well.

@ids1024
Copy link
Member

ids1024 commented Mar 24, 2022

I believe this is superseded by #210. Closing.

@ids1024 ids1024 closed this Mar 24, 2022
@jackpot51 jackpot51 deleted the setup_hirsute branch January 31, 2024 21:23
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

5 participants