Bugfix: improve layout of Preferences > Shortcuts tables #5679

psobolewskiPhD · 2023-03-28T15:56:35Z

Description

This PR adjusts the layout of the tables used for editing shortcuts such that long action descriptions can be read in their entirety. Briefly:

it changes the column widths, to give the text more room
as a result of 1, it left aligns the action descriptions so they are visually closer to the keybindings
it enables wrapping of long action descriptions
as a result of 3, it adjusts the row height and padding

Here's the new look: Viewer

versus main without this PR:

Notice how Show/hide IPython ... action has the full description readable, because it's line wrapped. Also, in my opinion the overall layout is more readable and pleasing.
Open to suggestions though!

New look Labels (main is shown in the issue):

Note: the new widths of the keybind columns are set to be sufficient for Ctrl-Alt-Meta- with a bit extra left-over.

Type of change

Bug-fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

References

Closes #5671
Some discussion of the table is also here #5018
this was the PR that implemented the second keybinding column.

How has this been tested?

example: the test suite for my feature covers cases x, y, and z
example: all tests pass with my change
example: I check if my changes works with both PySide and PyQt backends
as there are small differences between the two Qt bindings.

Final checklist:

My PR is the minimum possible work for the desired functionality
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
I have added tests that prove my fix is effective or that my feature works
If I included new strings, I have used trans. to make them localizable.
For more information see our translations guide.

Carreau · 2023-03-28T16:24:26Z

That will likely conflict with #5604 and #5064, but +1

codecov · 2023-03-28T19:29:12Z

Codecov Report

Merging #5679 (99c0463) into main (bd69fd2) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #5679   +/-   ##
=======================================
  Coverage   90.01%   90.02%           
=======================================
  Files         618      618           
  Lines       52827    52831    +4     
=======================================
+ Hits        47553    47561    +8     
+ Misses       5274     5270    -4

Impacted Files	Coverage Δ
napari/components/_viewer_key_bindings.py	`97.33% <ø> (ø)`
napari/_qt/widgets/qt_keyboard_settings.py	`70.80% <100.00%> (+0.39%)`	⬆️

... and 3 files with indirect coverage changes

# Description     Improvements in settings generation when settings have only one field. Also, update name show on top of group to use `title` instead of class name This PR: ![obraz](https://user-images.githubusercontent.com/3826210/230038230-f65c26d9-29b9-4725-9626-a54d558ad9a4.png) Current main: ![obraz](https://user-images.githubusercontent.com/3826210/230038392-f77d9ecf-260c-46db-82d2-e5eec26d9804.png) # References  This PR is inspired by #5679 showed on community meeting ## Type of change  - [ ] Refactor # How has this been tested?  - [ ] example: the test suite for my feature covers cases x, y, and z - [ ] example: all tests pass with my change - [ ] example: I check if my changes works with both PySide and PyQt backends as there are small differences between the two Qt bindings. ## Final checklist: - [ ] My PR is the minimum possible work for the desired functionality - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] If I included new strings, I have used `trans.` to make them localizable. For more information see our [translations guide](https://napari.org/developers/translations.html).

Czaki · 2023-04-14T20:35:44Z

As #5696 is merged, could you update the branch and post new screenshots?

psobolewskiPhD · 2023-04-15T09:24:51Z

As #5696 is merged, could you update the branch and post new screenshots?

Yes, sorry was traveling. I will get on this this weekend.

psobolewskiPhD · 2023-04-15T16:48:48Z

I think it will be impossible to satisfy pyside6, pyqt6 and pyqt5 all the way back to 5.12.3
I think pyqt5 5.13 is needed for the alignment flags to work.
So either need some logic to check Qt version or bump minimum requirements...

Czaki · 2023-04-15T16:51:43Z

Maybe add napari._qt._compat and import the alignment flag from _compat?

But also 0.5.0 may be enough to bump Qt requirements.

Czaki · 2023-05-12T16:51:11Z

Could you update the screenshots in the description?

psobolewskiPhD · 2023-05-25T20:02:59Z

Coming back to this, I sort of hated the right justify after all.
I ended up using non-breaking spaces in the Viewer action names and changing the columns to be a bit more balanced.
Then left justify everything (including headers)

(I updated the screenshots in OP)

jni · 2023-05-26T03:04:52Z

@psobolewskiPhD I might call this an enhancement more than a bugfix?

psobolewskiPhD · 2023-05-27T20:52:21Z

@jni I guess? depends on how you look at the truncated action names.
🤷‍♂️
But sure, we can call it enhancement and move it to the 0.5.0 milestone.

Czaki · 2023-05-28T10:44:44Z

Why move to 0.5.0? It is not breaking change, so it is worth trying to backport it.

psobolewskiPhD · 2023-05-28T13:43:07Z

I was going off @jni comment, thought he was meaning to move it to 0.5.0 because it's not a bug fix?

# Description Closes napari#5671 This PR adjusts the layout of the tables used for editing shortcuts such that long action descriptions can be read in their entirety. Briefly: 1) it changes the column widths, to give the text more room 2) as a result of 1, it left aligns the action descriptions so they are visually closer to the keybindings 3) it enables wrapping of long action descriptions 4) as a result of 3, it adjusts the row height and padding Here's the new look: Viewer <img width="1232" alt="image" src="https://github.com/napari/napari/assets/76622105/cfa97346-efeb-425b-b767-00e10da9f71d"> versus `main` without this PR: <img width="1198" alt="image" src="https://user-images.githubusercontent.com/76622105/228291024-39ca5495-75ef-4017-99c4-0ee4bf4238bf.png"> Notice how `Show/hide IPython ...` action has the full description readable, because it's line wrapped. Also, in my opinion the overall layout is more readable and pleasing. Open to suggestions though! New look Labels (main is shown in the issue): <img width="1232" alt="image" src="https://github.com/napari/napari/assets/76622105/3de31dc3-a28a-48ac-b1a8-0be85b1555d7"> Note: the new widths of the keybind columns are set to be sufficient for Ctrl-Alt-Meta-<Letter> with a bit extra left-over. ## Type of change  - [x] Bug-fix (non-breaking change which fixes an issue) # References Closes napari#5671 Some discussion of the table is also here napari#5018 this was the PR that implemented the second keybinding column. # How has this been tested?  - [x] example: all tests pass with my change ## Final checklist: - [x] My PR is the minimum possible work for the desired functionality - [x] I have commented my code, particularly in hard-to-understand areas --------- Co-authored-by: Grzegorz Bokota <bokota+github@gmail.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

@Czaki

…5854) # Fixes/Closes Closes #5853 # Description In `qt_keyboard_settings.py` the use of `item.setFlags(Qt.ItemFlag.NoItemFlags)` https://github.com/napari/napari/blob/0dfb37b474490b4d9829c4fa93cb3d648b4b186d/napari/_qt/widgets/qt_keyboard_settings.py#L233 results in these items being `disabled` so as a consequence of #5745 these table items get the reduced opacity leading to reduced contrast. This PR instead uses (❤️ @Czaki ): `item.setFlags(Qt.ItemFlag.ItemIsEnabled)` to ensure these items are `enabled` so that the opacity change does not affect them. # References Noted by @Czaki here #5679 (comment) ## Type of change  - [x] Bug-fix (non-breaking change which fixes an issue) # How has this been tested? - [x] example: all tests pass with my change - [x] example: I check if my changes works with both PySide and PyQt backends as there are small differences between the two Qt bindings. ## Final checklist: - [x] My PR is the minimum possible work for the desired functionality --------- Co-authored-by: Grzegorz Bokota <bokota+github@gmail.com>

# Description Closes #5671 This PR adjusts the layout of the tables used for editing shortcuts such that long action descriptions can be read in their entirety. Briefly: 1) it changes the column widths, to give the text more room 2) as a result of 1, it left aligns the action descriptions so they are visually closer to the keybindings 3) it enables wrapping of long action descriptions 4) as a result of 3, it adjusts the row height and padding Here's the new look: Viewer <img width="1232" alt="image" src="https://github.com/napari/napari/assets/76622105/cfa97346-efeb-425b-b767-00e10da9f71d"> versus `main` without this PR: <img width="1198" alt="image" src="https://user-images.githubusercontent.com/76622105/228291024-39ca5495-75ef-4017-99c4-0ee4bf4238bf.png"> Notice how `Show/hide IPython ...` action has the full description readable, because it's line wrapped. Also, in my opinion the overall layout is more readable and pleasing. Open to suggestions though! New look Labels (main is shown in the issue): <img width="1232" alt="image" src="https://github.com/napari/napari/assets/76622105/3de31dc3-a28a-48ac-b1a8-0be85b1555d7"> Note: the new widths of the keybind columns are set to be sufficient for Ctrl-Alt-Meta-<Letter> with a bit extra left-over. ## Type of change  - [x] Bug-fix (non-breaking change which fixes an issue) # References Closes #5671 Some discussion of the table is also here #5018 this was the PR that implemented the second keybinding column. # How has this been tested?  - [x] example: all tests pass with my change ## Final checklist: - [x] My PR is the minimum possible work for the desired functionality - [x] I have commented my code, particularly in hard-to-understand areas --------- Co-authored-by: Grzegorz Bokota <bokota+github@gmail.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

@Czaki

…5854) # Fixes/Closes Closes #5853 # Description In `qt_keyboard_settings.py` the use of `item.setFlags(Qt.ItemFlag.NoItemFlags)` https://github.com/napari/napari/blob/0dfb37b474490b4d9829c4fa93cb3d648b4b186d/napari/_qt/widgets/qt_keyboard_settings.py#L233 results in these items being `disabled` so as a consequence of #5745 these table items get the reduced opacity leading to reduced contrast. This PR instead uses (❤️ @Czaki ): `item.setFlags(Qt.ItemFlag.ItemIsEnabled)` to ensure these items are `enabled` so that the opacity change does not affect them. # References Noted by @Czaki here #5679 (comment) ## Type of change  - [x] Bug-fix (non-breaking change which fixes an issue) # How has this been tested? - [x] example: all tests pass with my change - [x] example: I check if my changes works with both PySide and PyQt backends as there are small differences between the two Qt bindings. ## Final checklist: - [x] My PR is the minimum possible work for the desired functionality --------- Co-authored-by: Grzegorz Bokota <bokota+github@gmail.com>

# Description Closes #5671 This PR adjusts the layout of the tables used for editing shortcuts such that long action descriptions can be read in their entirety. Briefly: 1) it changes the column widths, to give the text more room 2) as a result of 1, it left aligns the action descriptions so they are visually closer to the keybindings 3) it enables wrapping of long action descriptions 4) as a result of 3, it adjusts the row height and padding Here's the new look: Viewer <img width="1232" alt="image" src="https://github.com/napari/napari/assets/76622105/cfa97346-efeb-425b-b767-00e10da9f71d"> versus `main` without this PR: <img width="1198" alt="image" src="https://user-images.githubusercontent.com/76622105/228291024-39ca5495-75ef-4017-99c4-0ee4bf4238bf.png"> Notice how `Show/hide IPython ...` action has the full description readable, because it's line wrapped. Also, in my opinion the overall layout is more readable and pleasing. Open to suggestions though! New look Labels (main is shown in the issue): <img width="1232" alt="image" src="https://github.com/napari/napari/assets/76622105/3de31dc3-a28a-48ac-b1a8-0be85b1555d7"> Note: the new widths of the keybind columns are set to be sufficient for Ctrl-Alt-Meta-<Letter> with a bit extra left-over. ## Type of change  - [x] Bug-fix (non-breaking change which fixes an issue) # References Closes #5671 Some discussion of the table is also here #5018 this was the PR that implemented the second keybinding column. # How has this been tested?  - [x] example: all tests pass with my change ## Final checklist: - [x] My PR is the minimum possible work for the desired functionality - [x] I have commented my code, particularly in hard-to-understand areas --------- Co-authored-by: Grzegorz Bokota <bokota+github@gmail.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

@Czaki

…5854) # Fixes/Closes Closes #5853 # Description In `qt_keyboard_settings.py` the use of `item.setFlags(Qt.ItemFlag.NoItemFlags)` https://github.com/napari/napari/blob/0dfb37b474490b4d9829c4fa93cb3d648b4b186d/napari/_qt/widgets/qt_keyboard_settings.py#L233 results in these items being `disabled` so as a consequence of #5745 these table items get the reduced opacity leading to reduced contrast. This PR instead uses (❤️ @Czaki ): `item.setFlags(Qt.ItemFlag.ItemIsEnabled)` to ensure these items are `enabled` so that the opacity change does not affect them. # References Noted by @Czaki here #5679 (comment) ## Type of change  - [x] Bug-fix (non-breaking change which fixes an issue) # How has this been tested? - [x] example: all tests pass with my change - [x] example: I check if my changes works with both PySide and PyQt backends as there are small differences between the two Qt bindings. ## Final checklist: - [x] My PR is the minimum possible work for the desired functionality --------- Co-authored-by: Grzegorz Bokota <bokota+github@gmail.com>

# Description Closes #5671 This PR adjusts the layout of the tables used for editing shortcuts such that long action descriptions can be read in their entirety. Briefly: 1) it changes the column widths, to give the text more room 2) as a result of 1, it left aligns the action descriptions so they are visually closer to the keybindings 3) it enables wrapping of long action descriptions 4) as a result of 3, it adjusts the row height and padding Here's the new look: Viewer <img width="1232" alt="image" src="https://github.com/napari/napari/assets/76622105/cfa97346-efeb-425b-b767-00e10da9f71d"> versus `main` without this PR: <img width="1198" alt="image" src="https://user-images.githubusercontent.com/76622105/228291024-39ca5495-75ef-4017-99c4-0ee4bf4238bf.png"> Notice how `Show/hide IPython ...` action has the full description readable, because it's line wrapped. Also, in my opinion the overall layout is more readable and pleasing. Open to suggestions though! New look Labels (main is shown in the issue): <img width="1232" alt="image" src="https://github.com/napari/napari/assets/76622105/3de31dc3-a28a-48ac-b1a8-0be85b1555d7"> Note: the new widths of the keybind columns are set to be sufficient for Ctrl-Alt-Meta-<Letter> with a bit extra left-over. ## Type of change  - [x] Bug-fix (non-breaking change which fixes an issue) # References Closes #5671 Some discussion of the table is also here #5018 this was the PR that implemented the second keybinding column. # How has this been tested?  - [x] example: all tests pass with my change ## Final checklist: - [x] My PR is the minimum possible work for the desired functionality - [x] I have commented my code, particularly in hard-to-understand areas --------- Co-authored-by: Grzegorz Bokota <bokota+github@gmail.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

@Czaki

…5854) # Fixes/Closes Closes #5853 # Description In `qt_keyboard_settings.py` the use of `item.setFlags(Qt.ItemFlag.NoItemFlags)` https://github.com/napari/napari/blob/0dfb37b474490b4d9829c4fa93cb3d648b4b186d/napari/_qt/widgets/qt_keyboard_settings.py#L233 results in these items being `disabled` so as a consequence of #5745 these table items get the reduced opacity leading to reduced contrast. This PR instead uses (❤️ @Czaki ): `item.setFlags(Qt.ItemFlag.ItemIsEnabled)` to ensure these items are `enabled` so that the opacity change does not affect them. # References Noted by @Czaki here #5679 (comment) ## Type of change  - [x] Bug-fix (non-breaking change which fixes an issue) # How has this been tested? - [x] example: all tests pass with my change - [x] example: I check if my changes works with both PySide and PyQt backends as there are small differences between the two Qt bindings. ## Final checklist: - [x] My PR is the minimum possible work for the desired functionality --------- Co-authored-by: Grzegorz Bokota <bokota+github@gmail.com>

# Description Closes #5671 This PR adjusts the layout of the tables used for editing shortcuts such that long action descriptions can be read in their entirety. Briefly: 1) it changes the column widths, to give the text more room 2) as a result of 1, it left aligns the action descriptions so they are visually closer to the keybindings 3) it enables wrapping of long action descriptions 4) as a result of 3, it adjusts the row height and padding Here's the new look: Viewer <img width="1232" alt="image" src="https://github.com/napari/napari/assets/76622105/cfa97346-efeb-425b-b767-00e10da9f71d"> versus `main` without this PR: <img width="1198" alt="image" src="https://user-images.githubusercontent.com/76622105/228291024-39ca5495-75ef-4017-99c4-0ee4bf4238bf.png"> Notice how `Show/hide IPython ...` action has the full description readable, because it's line wrapped. Also, in my opinion the overall layout is more readable and pleasing. Open to suggestions though! New look Labels (main is shown in the issue): <img width="1232" alt="image" src="https://github.com/napari/napari/assets/76622105/3de31dc3-a28a-48ac-b1a8-0be85b1555d7"> Note: the new widths of the keybind columns are set to be sufficient for Ctrl-Alt-Meta-<Letter> with a bit extra left-over. ## Type of change  - [x] Bug-fix (non-breaking change which fixes an issue) # References Closes #5671 Some discussion of the table is also here #5018 this was the PR that implemented the second keybinding column. # How has this been tested?  - [x] example: all tests pass with my change ## Final checklist: - [x] My PR is the minimum possible work for the desired functionality - [x] I have commented my code, particularly in hard-to-understand areas --------- Co-authored-by: Grzegorz Bokota <bokota+github@gmail.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

Improvements in settings generation when settings have only one field. Also, update name show on top of group to use `title` instead of class name This PR: ![obraz](https://user-images.githubusercontent.com/3826210/230038230-f65c26d9-29b9-4725-9626-a54d558ad9a4.png) Current main: ![obraz](https://user-images.githubusercontent.com/3826210/230038392-f77d9ecf-260c-46db-82d2-e5eec26d9804.png)  This PR is inspired by #5679 showed on community meeting  - [ ] Refactor  - [ ] example: the test suite for my feature covers cases x, y, and z - [ ] example: all tests pass with my change - [ ] example: I check if my changes works with both PySide and PyQt backends as there are small differences between the two Qt bindings. - [ ] My PR is the minimum possible work for the desired functionality - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] If I included new strings, I have used `trans.` to make them localizable. For more information see our [translations guide](https://napari.org/developers/translations.html).

psobolewskiPhD requested a review from Czaki March 28, 2023 15:56

github-actions bot assigned psobolewskiPhD Mar 28, 2023

github-actions bot added the qt Relates to qt label Mar 28, 2023

Czaki mentioned this pull request Apr 5, 2023

Improve preferences genration #5696

Merged

9 tasks

psobolewskiPhD force-pushed the bugfix/spacing_keybind_settings branch from 06c5980 to 66ed7d2 Compare April 15, 2023 14:41

psobolewskiPhD added 14 commits April 15, 2023 16:41

Initial try: widen and wrap

af5bcbc

Improve row spacing and column width

c92ec3a

Widen keybind column a bit

af6a709

Fix for Qt6 enums

639a10c

Now fix for pyqt5

7241b57

really make it work on qt5 and qt5 (❤️ ChatGPT)

f9e37f1

Try to fix for 5.12.3

851a709

Use the int() fix in all the places and proper flag

495635a

fix for qt6

4913f5e

different shot at qt6 fix

b9186aa

Fix a copy-pasta error

66ed7d2

loosen spacing after napari#5696

3d5a95c

fix pyside6 DeprecationWarning

d25958c

use Qt.AlignmentFlag instead

12ce0a7

psobolewskiPhD added 3 commits April 16, 2023 11:49

Merge branch 'main' into bugfix/spacing_keybind_settings

b083dfe

Merge branch 'main' into bugfix/spacing_keybind_settings

d8139ee

Merge branch 'main' into bugfix/spacing_keybind_settings

3d51e33

psobolewskiPhD and others added 2 commits May 25, 2023 20:33

Merge branch 'main' into bugfix/spacing_keybind_settings

e755c4a

Left justify using non-breaking spaces as needed

e1e0b1b

Czaki added the ready to merge Last chance for comments! Will be merged in ~24h label May 26, 2023

psobolewskiPhD added enhancement and removed maintenance PR with maintance changes, pr-party-update-async labels May 27, 2023

psobolewskiPhD modified the milestones: 0.4.18, 0.5.0 May 27, 2023

Merge branch 'main' into bugfix/spacing_keybind_settings

99c0463

Czaki modified the milestones: 0.5.0, 0.4.18 Jun 1, 2023

Czaki merged commit 894c03c into napari:main Jun 1, 2023
32 checks passed

Czaki removed the ready to merge Last chance for comments! Will be merged in ~24h label Jun 1, 2023

Czaki mentioned this pull request Jun 7, 2023

v0.4.18 - release process #5911

Closed

psobolewskiPhD mentioned this pull request Jan 18, 2024

[Qt6, PyQt6 6.6.1] Breaks keyboard shortcuts layout in Preferences #6601

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bugfix: improve layout of Preferences > Shortcuts tables #5679

Bugfix: improve layout of Preferences > Shortcuts tables #5679

psobolewskiPhD commented Mar 28, 2023 •

edited

Loading

Carreau commented Mar 28, 2023

codecov bot commented Mar 28, 2023 •

edited

Loading

Czaki commented Apr 14, 2023

psobolewskiPhD commented Apr 15, 2023

psobolewskiPhD commented Apr 15, 2023

Czaki commented Apr 15, 2023 •

edited

Loading

Czaki commented May 12, 2023

psobolewskiPhD commented May 25, 2023 •

edited

Loading

jni commented May 26, 2023

psobolewskiPhD commented May 27, 2023 •

edited

Loading

Czaki commented May 28, 2023

psobolewskiPhD commented May 28, 2023

Bugfix: improve layout of Preferences > Shortcuts tables #5679

Bugfix: improve layout of Preferences > Shortcuts tables #5679

Conversation

psobolewskiPhD commented Mar 28, 2023 • edited Loading

Description

Type of change

References

How has this been tested?

Final checklist:

Carreau commented Mar 28, 2023

codecov bot commented Mar 28, 2023 • edited Loading

Codecov Report

Czaki commented Apr 14, 2023

psobolewskiPhD commented Apr 15, 2023

psobolewskiPhD commented Apr 15, 2023

Czaki commented Apr 15, 2023 • edited Loading

Czaki commented May 12, 2023

psobolewskiPhD commented May 25, 2023 • edited Loading

jni commented May 26, 2023

psobolewskiPhD commented May 27, 2023 • edited Loading

Czaki commented May 28, 2023

psobolewskiPhD commented May 28, 2023

psobolewskiPhD commented Mar 28, 2023 •

edited

Loading

codecov bot commented Mar 28, 2023 •

edited

Loading

Czaki commented Apr 15, 2023 •

edited

Loading

psobolewskiPhD commented May 25, 2023 •

edited

Loading

psobolewskiPhD commented May 27, 2023 •

edited

Loading