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

Make menus and placeholders display appropriate custom key combos. #4045

Merged
merged 12 commits into from
Oct 9, 2022

Conversation

Mm2PL
Copy link
Collaborator

@Mm2PL Mm2PL commented Oct 4, 2022

Pull request checklist:

  • CHANGELOG.md was updated, if applicable

Description

Closes #4039.
Todo:

  • make neededArguments work
  • [-] implement it everywhere:
    • SplitHeader
      2022-10-04_23-10

    • [-] Settings

      • search placeholder
      • Hide similar messages setting
      • prefs button setting
    • Notebook (hide tabs)

    • SplitInput that's a Qt menu.

    • something more?

  • Consider showing multiple shortcuts when possible (?)
  • Update documentation

All the commits mentioning scuffed-ness need some kind of relayout or app restart to update the key bind description.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

src/controllers/hotkeys/HotkeyController.cpp Show resolved Hide resolved
src/controllers/hotkeys/HotkeyController.cpp Outdated Show resolved Hide resolved
src/controllers/hotkeys/HotkeyController.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

src/controllers/hotkeys/HotkeyController.cpp Show resolved Hide resolved
src/controllers/hotkeys/HotkeyController.cpp Outdated Show resolved Hide resolved
src/widgets/dialogs/SettingsDialog.cpp Show resolved Hide resolved
@Mm2PL Mm2PL changed the title Add support for finding hotkey display key sequences Make menus and placeholders display appropriate custom key combos. Oct 6, 2022
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

src/widgets/Notebook.cpp Show resolved Hide resolved
src/widgets/Notebook.cpp Show resolved Hide resolved
this does not update dynamically!
@l4ejyetrgf
Copy link

If your messing with hotkeys, your may fix #3474 as well

@jupjohn
Copy link
Contributor

jupjohn commented Oct 7, 2022

If your messing with hotkeys, your may fix #3474 as well

That should be a different PR as it has nothing to do with the scope of this one.

@Mm2PL
Copy link
Collaborator Author

Mm2PL commented Oct 7, 2022

If your messing with hotkeys, your may fix #3474 as well

Getting side-tracked and introducing out-of-scope changes is the best way to never finish a PR. It's better to finish one and then start another. Custom hotkeys was already an awfully large PR (scope-wise, not code) and I honestly don't want to do anything that big.

@Mm2PL Mm2PL marked this pull request as ready for review October 8, 2022 14:45
@Mm2PL Mm2PL requested a review from pajlada October 8, 2022 15:13
Copy link
Member

@pajlada pajlada left a comment

Choose a reason for hiding this comment

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

just comment nitpick, works as expected

some hotkeys don't update on change while others do but that's ok with me as a first effort, especially since this is already an improvement of usability

src/controllers/hotkeys/HotkeyController.hpp Outdated Show resolved Hide resolved
@pajlada
Copy link
Member

pajlada commented Oct 9, 2022

@Mm2PL Feel free to merge in when you're happy!

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

src/controllers/hotkeys/HotkeyController.cpp Show resolved Hide resolved
src/controllers/hotkeys/HotkeyController.cpp Show resolved Hide resolved
@Mm2PL Mm2PL enabled auto-merge (squash) October 9, 2022 12:45
@pajlada pajlada disabled auto-merge October 9, 2022 15:20
@pajlada
Copy link
Member

pajlada commented Oct 9, 2022

Skipping FreeBSD build on Cirrus for now

@pajlada pajlada merged commit e604a36 into master Oct 9, 2022
@pajlada pajlada deleted the feature/custom_hotkeys_pr_two_the_electric_boogaloo branch October 9, 2022 15:20
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.

Show custom hotkeys all over the application
4 participants