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

Layout: allow to set a custom shortcut to switch layouts #460

Closed
wants to merge 16 commits into from

Conversation

lenemter
Copy link
Member

@lenemter lenemter commented Aug 16, 2023

Fixes #472

image

A lot of changes were required to embed the list from shortcuts tab, so I'll try to propose a cleanup branch later to reduce the diff.

@lenemter lenemter requested a review from danirabbit August 16, 2023 14:01
@lenemter lenemter changed the title Layouts: redesign Layout: redesign Aug 16, 2023
@TomiOhl
Copy link
Contributor

TomiOhl commented Aug 16, 2023

  • Uses radio buttons inside flowboxes instead of combo boxes

I know there were some moves to the same direction in some plugs but personally I find this pretty overwhelming to look at.

@lenemter lenemter changed the title Layout: redesign Layout: allows to set a custom shortcut to switch layouts Aug 17, 2023
@lenemter
Copy link
Member Author

@TomiOhl I agree, I had an idea to use plain lists of radio buttons, but it takes too much vertical space. So I reverted it back to using combo boxes.

@lenemter lenemter changed the title Layout: allows to set a custom shortcut to switch layouts Layout: allow to set a custom shortcut to switch layouts Aug 17, 2023
@danirabbit
Copy link
Member

Good call reverting back to comboboxes. When there's only a handful of options I think radiobuttons are usually a little nicer for being able to see and compare the available options, but in this case there's just so many options that it's really overwhelming and it makes it actually more difficult to see what your overall settings actually are.

I definitely think it's cool to have this be a more open-ended keyboard shortcut like others are, but I'm not sure about the "additional shortcut" setting here. It seems weird to have a duplicate between a custom shortcut and a list of options and like just having the custom shortcut would satisfy everyone here.

This also makes me think that maybe these options should be in Shortcuts → System instead of in the Layout tab and we could link from the Layout tab to the Shortcuts tab to help discovery here. Thoughts on that?

@lenemter
Copy link
Member Author

just having the custom shortcut would satisfy everyone here

No, it wouldn't.

  1. You can't set custom accelerators-only shortcuts (like Alt + Shift or Ctrl + Alt)
  2. Caps Lock is a common shortcut to change layouts but original action doesn't get disabled when setting it as a custom keyboard shortcut
  3. Even if wanted to remove "Additional shortcut" (like Gnome did) we will have to wait for OS 8, because people have it set and we can't just remove it.

@lenemter
Copy link
Member Author

This also makes me think that maybe these options should be in Shortcuts → System instead of in the Layout tab and we could link from the Layout tab to the Shortcuts tab to help discovery here. Thoughts on that?

I don't like this idea because it's not convenient. Why put it in Shortcuts → System if we can put it closer to related settings?

@lenemter lenemter mentioned this pull request Aug 22, 2023
@danirabbit
Copy link
Member

This also makes me think that maybe these options should be in Shortcuts → System instead of in the Layout tab and we could link from the Layout tab to the Shortcuts tab to help discovery here. Thoughts on that?

I don't like this idea because it's not convenient. Why put it in Shortcuts → System if we can put it closer to related settings?

I'm thinking of in the future where #18 is solved and that it would be easy to have a discovery path from this tab to the shortcuts tab by adding a link, but it would be harder if someone went to the shortcuts tab first to point them back to this tab to look for the setting

@lenemter lenemter closed this Jan 29, 2024
@lenemter lenemter removed the request for review from danirabbit January 29, 2024 23:48
@lenemter lenemter deleted the lenemter/redesign branch January 29, 2024 23:48
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.

Free up the Super key
3 participants