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

Do not use native dialog for reset shortcuts #6493

Merged
merged 3 commits into from Dec 3, 2023

Conversation

Czaki
Copy link
Collaborator

@Czaki Czaki commented Nov 23, 2023

References and relevant issues

closes #6490

Description

Since Qt6==6.5.0 on Macos, the Qt starts using native dialog even if it does not support all buttons (in our case QMessageBox.StandardButton.RestoreDefaults).

This solution is inspired by comments from https://bugreports.qt.io/browse/QTBUG-116757

If someone has time, please try to open the Bug report there. After the last discussions about patching enum, I do not feel fluent enough to write in English to talk with them.

An alternative solution will be to change the button on the supported one. Or change the native flag globally, but the second one may impact File Dialogs.

@Czaki Czaki added the bugfix PR with bugfix label Nov 23, 2023
@Czaki Czaki added this to the 0.4.19 milestone Nov 23, 2023
@github-actions github-actions bot added the qt Relates to qt label Nov 23, 2023
Copy link

codecov bot commented Nov 23, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (93af19a) 92.24% compared to head (11c4ce6) 92.17%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6493      +/-   ##
==========================================
- Coverage   92.24%   92.17%   -0.07%     
==========================================
  Files         601      601              
  Lines       53210    53216       +6     
==========================================
- Hits        49081    49052      -29     
- Misses       4129     4164      +35     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@psobolewskiPhD
Copy link
Member

This fixes the keybinding dialog, but when testing I noticed the overall preferences reset dialog has the same behavior. I made a PR to your branch with the analogous fix to the other dialog which fixes it locally for me--hopefully I didn't mess it up!

@github-actions github-actions bot added the preferences Issues relating to the creation of new preference fields/panels label Nov 23, 2023
@Czaki
Copy link
Collaborator Author

Czaki commented Nov 23, 2023

Maybe we should switch this into a contextmanager?

Copy link
Member

@psobolewskiPhD psobolewskiPhD left a comment

Choose a reason for hiding this comment

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

Maybe we should switch this into a contextmanager?

I'm not sure what this would entail to be honest.
As is, this PR seems like a good fix. Or we can wait and see if something is fixed in next Qt release?

@Czaki Czaki added the ready to merge Last chance for comments! Will be merged in ~24h label Nov 29, 2023
@Czaki Czaki merged commit 65ffc89 into napari:main Dec 3, 2023
33 checks passed
@Czaki Czaki deleted the fix_reset_shortcuts branch December 3, 2023 19:52
@Carreau Carreau removed the ready to merge Last chance for comments! Will be merged in ~24h label Dec 5, 2023
Czaki added a commit that referenced this pull request Dec 15, 2023
# References and relevant issues
closes #6490

# Description

Since Qt6==6.5.0 on Macos, the Qt starts using native dialog even if it
does not support all buttons (in our case
`QMessageBox.StandardButton.RestoreDefaults`).

This solution is inspired by comments from
https://bugreports.qt.io/browse/QTBUG-116757

If someone has time, please try to open the Bug report there. After the
last discussions about patching enum, I do not feel fluent enough to
write in English to talk with them.

An alternative solution will be to change the button on the supported
one. Or change the native flag globally, but the second one may impact
File Dialogs.

---------

Co-authored-by: Peter Sobolewski <pete.sd@gmail.com>
kne42 added a commit to kne42/napari that referenced this pull request Dec 20, 2023
* main: (80 commits)
  Check in LabelColormap that fewer than 2**16 colors are requested (napari#6540)
  Fix label color shuffling by also updating colormap size (napari#6460)
  Add `_block_refresh()` to layers (napari#6525)
  MNT: Use `partial` in samples menu to avoid leaking  (napari#6538)
  Update performance and reduce memory usage for big Labels layer in direct color mode (napari#6439)
  Reset single step and decimals on reset range slider in popup (napari#6523)
  Add copy operator to fix memory benchmarks (napari#6530)
  Restore quit shortcut (napari#6526)
  Fix problem with invalidate cache  (napari#6520)
  [pre-commit.ci] pre-commit autoupdate (napari#6505)
  Pass key event from Main window to our internal mechanism (napari#6419)
  Typing: Fully type 5 more files (napari#6361)
  Do not use native dialog for reset shortcuts (napari#6493)
  Use views rather than CPU-based hashing for 8- and 16-bit Labels data (napari#6467)
  Add import lint back to CI (napari#6506)
  Type napari.layers.image helper sub-modules (napari#6499)
  Bugfix: ensure gamma and opacity are floats (napari#6498)
  FIX: Remove `None` default from `_remove_dock_widget` (napari#6481)
  Add testing_extra and optional dependencies when creating constraints (napari#6487)
  Run test suite with optional dependencies and fix tests when `triangle` is installed (napari#6488)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix PR with bugfix preferences Issues relating to the creation of new preference fields/panels qt Relates to qt
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[PyQt6, Qt6] Unable to restore default keybindings: odd dialog
3 participants