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

2385 incosistent tabbing order across the widgets #2401

Merged
merged 6 commits into from
Jan 17, 2023

Conversation

rozyczko
Copy link
Member

@rozyczko rozyczko commented Dec 9, 2022

Description

This PR addresses one of the issues brought up in https://github.com/orgs/SasView/discussions/2342 and spun off into #2385.

Now, tab order for all widgets in SasView is consistent and goes in standard direction (left->right, up->down)

Fixes #2385

Review Checklist (please remove items if they don't apply):

  • Code has been reviewed
  • Functionality has been tested
  • Windows installer (GH artifact) has been tested (installed and worked)
  • MacOSX installer (GH artifact) has been tested (installed and worked)
  • The introduced changes comply with SasView license (BSD 3-Clause)

@krzywon
Copy link
Contributor

krzywon commented Dec 12, 2022

I've tested the functionality but haven't reviewed the code. Here are a few thoughts, some of which might be out of scope for this PR.

The good:

  • Tabbing is now left->right, top->bottom as expected in most places
  • Tabbing skips disabled inputs but not uneditable inputs (useful for copying results)
  • For the corfunc perspective, and SLD, Q-resolution, and Generic scattering calculator tools, tabbing seems reasonable

Needs work:

  • Tabbing travels through uneditable boxes when they are blank but gives no indication the box has focus
  • Data operations panel: tabbing takes you into the data windows. Is there a reason to ever give them focus?
  • Density/Volume calculator: Tabbing gets stuck at 'Molar Volume' and 'Mass Density' Inputs
  • Kiessig Calculator: Close is selected twice along the tabbing path, once when expected, but another time after the window tabs are highlighted
  • Invariant Perspective: Data Explorer is given focus when switching to this perspective. Tabbing seems to get stuck at 'Porod Constant' text box
  • Inversion Perspective: The Options section is still unpredictable where tabbing will take you
  • Fitting Perspective: Selecting a model often causes the focus to shift back to the Fitting window but should probably stay at the model selector.

Suggestions:

  • Tabbing is only applied to the window with focus. There is no way (in Windows) for the data explorer to gain focus so there is no way to tab into that panel. Can we add a Ctrl/Command-Tab option to tab between panels, menu, and options bar?
  • Add some indication a text box, especially when an uneditable text box is blank, receives tab focus.

@wpotrzebowski wpotrzebowski added the Discuss At The Call Issues to be discussed at the fortnightly call label Dec 19, 2022
@rozyczko
Copy link
Member Author

  • Tabbing travels through uneditable boxes when they are blank but gives no indication the box has focus

Can you give me an example of a widget that has those? I can't find empty, read only boxes with focus stop...

  • Data operations panel: tabbing takes you into the data windows. Is there a reason to ever give them focus?

There is no reason as these windows only show data. Fixed.

  • Density/Volume calculator: Tabbing gets stuck at 'Molar Volume' and 'Mass Density' Inputs

This one is interesting - the user NEEDS to enter a value in this field, which is why the textbox holds on to the focus.
Once entered, you can keep tabbing.

  • Kiessig Calculator: Close is selected twice along the tabbing path, once when expected, but another time after the window tabs are highlighted

Very, very weird. Working on it.

  • Invariant Perspective: Data Explorer is given focus when switching to this perspective. Tabbing seems to get stuck at 'Porod Constant' text box

Fixed the ordering. One has to wait a short while after tabbing after Max Distance for the calculations to finish (Focus loss causes recalculation? Is this correct?).

  • Inversion Perspective: The Options section is still unpredictable where tabbing will take you

This again is an issue of recalculation being done on focus lost. This potentially needs fixing.

  • Fitting Perspective: Selecting a model often causes the focus to shift back to the Fitting window but should probably stay at the model selector.

I tried to address it with mixed success. Will look closer.

  • Tabbing is only applied to the window with focus. There is no way (in Windows) for the data explorer to gain focus so there is no way to tab into that panel. Can we add a Ctrl/Command-Tab option to tab between panels, menu, and options bar?

Hmm this could be a nice extension. Will address it.

  • Add some indication a text box, especially when an uneditable text box is blank, receives tab focus.

I don't think this can be easily done. It's better to remove uneditable boxes from receiving focus.

@krzywon
Copy link
Contributor

krzywon commented Dec 19, 2022

  • Tabbing travels through uneditable boxes when they are blank but gives no indication the box has focus

Can you give me an example of a widget that has those? I can't find empty, read only boxes with focus stop...

The invariant perspective without data in the 'Invariant' tab - tabbing through takes 6 presses to go from max Q to the buttons, suggesting the volume fraction and other inputs are selected.

  • Invariant Perspective: Data Explorer is given focus when switching to this perspective. Tabbing seems to get stuck at 'Porod Constant' text box

Fixed the ordering. One has to wait a short while after tabbing after Max Distance for the calculations to finish (Focus loss causes recalculation? Is this correct?).

Seems better

  • Density/Volume calculator: Tabbing gets stuck at 'Molar Volume' and 'Mass Density' Inputs

This one is interesting - the user NEEDS to enter a value in this field, which is why the textbox holds on to the focus. Once entered, you can keep tabbing.

  • Add some indication a text box, especially when an uneditable text box is blank, receives tab focus.

I don't think this can be easily done. It's better to remove uneditable boxes from receiving focus.

On a side note, these two are handled in #2258. In that PR, anything with focus gets a new background color and a red border is added to any input with an error. I won't get back to that PR for a while but some of these suggestions will likely be addressed for v6.0, but that work has a ways to go still.

@wpotrzebowski wpotrzebowski removed the Discuss At The Call Issues to be discussed at the fortnightly call label Dec 20, 2022
src/sas/qtgui/Calculators/KiessigPanel.py Outdated Show resolved Hide resolved
src/sas/qtgui/Calculators/KiessigPanel.py Show resolved Hide resolved
</style></head><body style=" font-family:'MS Shell Dlg 2'; font-size:7.8pt; font-weight:400; font-style:normal;">
&lt;p style=&quot;-qt-paragraph-type:empty; margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px;&quot;&gt;&lt;br /&gt;&lt;/p&gt;&lt;/body&gt;&lt;/html&gt;</string>
&lt;/style&gt;&lt;/head&gt;&lt;body style=&quot; font-family:'MS Shell Dlg 2'; font-size:6.6pt; font-weight:400; font-style:normal;&quot;&gt;
&lt;p style=&quot;-qt-paragraph-type:empty; margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; font-size:7.8pt;&quot;&gt;&lt;br /&gt;&lt;/p&gt;&lt;/body&gt;&lt;/html&gt;</string>
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we be specifying font sizes absolutely?

Copy link
Member Author

Choose a reason for hiding this comment

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

This seems to be a standard Qt Designer thing. Quite a few .ui files have explicit font sizes for tooltips and html content displayed in forms. These values are never manually modified in the Designer so I assume they are defaults.
I could remove them by hand but then loading/saving the forms would write the font size directives again...

@rozyczko
Copy link
Member Author

rozyczko commented Jan 6, 2023

The invariant perspective without data in the 'Invariant' tab - tabbing through takes 6 presses to go from max Q to the buttons, suggesting the volume fraction and other inputs are selected.

Modified those fields to be focusable with clicks but not with tab.

@butlerpd butlerpd added the Discuss At The Call Issues to be discussed at the fortnightly call label Jan 17, 2023
@butlerpd butlerpd merged commit a4a2f2f into main Jan 17, 2023
@krzywon krzywon deleted the 2385-incosistent-tabbing-order-across-the-widgets branch January 27, 2023 21:43
@butlerpd butlerpd removed the Discuss At The Call Issues to be discussed at the fortnightly call label Aug 1, 2023
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.

Incosistent tabbing order across the widgets
5 participants