GUI: focus and tabbing

[gzotti]

Description

As discussed in #1834 the Qt-based interface allows using the tab key to switch between GUI elements. So far the GUI was not designed to be used that way, and the GUI did not indicate which button had the "focus" to receive the next keypress.

However, for a few users this seems a critically important feature to add. This branch should be used to add the focus highlight to GUI elements where it makes sense, and then to adapt the .ui files to define a "tabbing order".

• Note that the tabbing order will then have to be checked and re-developed every time a UI file is changed.
• Note that all UI file elements "colorrole" must be deleted after every edit of an ui file with QtCreator to make them compilable with Qt5.9. Or why do we have to do that? I don't find documentation on that.
• A new GUI element "SpaceCheckBox" can be used to modify QCheckBoxes into spacebar-toggling elements.

Fixes # (issue)

Screenshots (if appropriate):

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 change)
• This change requires a documentation update

How Has This Been Tested?

Test Configuration:

• Operating system: all
• Graphics Card: all

Checklist:

• My code follows the code style of this project.
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes
• Any dependent changes have been merged and published in downstream modules

[github-actions]

Hello @gzotti! Thank you for this enhancement.

[axd1967]

@gzotti if you like I can review other UIs via this branch

[axd1967]

I don't understand the "colorrole" note - I only use Qt Designer (5.12.8), it does not appear there.

https://doc.qt.io/qt-5/qpalette.html#ColorRole-enum

@gzotti sure there is no relation with this:

stellarium/src/gui/StelGui.cpp

Lines 402 to 408 in 0b0ece4

	QPalette p = QGuiApplication::palette();
	p.setColor(QPalette::Disabled, QPalette::Light, QColor(0,0,0,0));
	// And this is for the focus... apparently the focus indicator is the inverted value for Active/Button.
	p.setColor(QPalette::Active, QPalette::Button, QColor(255,255,255));
	QGuiApplication::setPalette(p);

[alex-w]

Note that all UI file elements "colorrole" must be deleted after every edit of an ui file with QtCreator to make them compilable with Qt5.9. Or why do we have to do that? I don't find documentation on that.

The “colorrole” for placeholder’s text was introduced in version 5.12 and, of course, for Qt 5.9 this is unknown instruction.

[gzotti]

@gzotti if you like I can review other UIs via this branch

Yes. If you rebase #1834 on current master and PR to this branch, we can start with that. But note that there is only so much time to invest in this topic. Nothing will ever be 100% perfect for all users, but some improvements are possible. The new GUI element variants provide focus highlights now for buttons and checkboxes , so they can be included when they make sense. (Not sure about pages like view/markings, would anybody really not use a mouse here?)

[axd1967]

How about doing this in small iterations, so this PR doesn't drag on for too long. I already rebased my branch, you might want to merge that to this PR.

(update: still need to rebase to 0b0ece4)

However, make sure to check #1834 (comment) before finalizing this PR; did you see it?

Once this is closed, I can have a look at other places that could benefit from this change.

[gzotti]

@gzotti please note that the (meanwhile long chain of) observations made in #1971 (comment) still apply and prevent to merge this.

did you read those?

To which I replied:

To see focus highlights on space-pressable buttons, you need to use the custom classes SpaceCheckBox and SpacePushButton now. Regular QPushButtons do not take Space or other keypresses (?) even if in (invisible) focus.

As far as I understand your repository "axd1967" is not part of the Stellarium repo at github, therefore I cannot simply check out your branch. (Sure, I can clone your repo, but this is overkill to retrieve 9 lines for a copy&paste issue) I can manage a PR on github when done to the right branch, which is why I had asked you to re-address your PR to the feature branch gui-show-focus.

[axd1967]

aaa...

To which I replied:

To see focus highlights on space-pressable buttons, you need to use the custom classes SpaceCheckBox and SpacePushButton now. Regular QPushButtons do not take Space or other keypresses (?) even if in (invisible) focus.

You are right, I forgot that. To be honest, I have a difficult time after all these rough treatments.
Maybe I was supposed to use those classes?

As far as I understand your repository "axd1967" is not part of the Stellarium repo at github, therefore I cannot simply check out your branch. (Sure, I can clone your repo, but this is overkill to retrieve 9 lines for a copy&paste issue) I can manage a PR on github when done to the right branch, which is why I had asked you to re-address your PR to the feature branch gui-show-focus.

That's why I pushed my branch as a PR on top of your branch :-)
But indeed, I could as well have pushed to your branch, it's just the same, except that I'm not sure I have push rights, therefore GitHub might force me to do the push via a PR.

[gzotti]

No, you can only send pull requests.
I have now switched the buttons to SpacePushButtons, also the button to call the shortcut editor on the F1 opening screen. But I am not sure this is really the super great perfect user interface now. Presumably >95% will prefer to keep using the mouse.

[axd1967]

As far as I understand your repository "axd1967" is not part of the Stellarium repo at github, therefore I cannot simply check out your branch. (Sure, I can clone your repo, but this is overkill to retrieve 9 lines for a copy&paste issue) I can manage a PR on github when done to the right branch, which is why I had asked you to re-address your PR to the feature branch gui-show-focus.

Note there is no need to clone; what you do is to add my repo as a remote to your repo (git remote add user/axd ...), after which you can fetch the relevant branch, test locally and push to Stellarium.

[axd1967]

LGTM!

Note that the Help dialog is an example where ENTER could trigger the bottom buttons.
To realise this, it should suffice to set the intial focus to those buttons.

Shall I open a separate issue to review tab/focus issues in some key dialogs? I will PR the required modifications.

[gzotti]

I have enabled SPACE as the key to trigger the button action. Would ENTER be the better one? Or do the buttons react to ENTER on Ubuntu? I think it is not good to have both SPACE and ENTER available.

I suggest you add one GUI panel per PR. As often, Alexander and I may have diverging preferences about these changes, and I definitely cannot guarantee that we will accept all changes. (or even ... almost guarantee we will not... but we can hopefully work out something useful.) I like those changes only when I see the usability becomes better, more intuitive, and tabbing always switches to some element that indicates that it is in focus. (What about the tabs in TabViews? Is there keyboard interaction intended?)

For now, there are SpacePushButtons and SpaceCheckBoxes which can replace QPushButtons and QCheckBoxes, respectively. If you need others, I should be able to add them as well. During the next weeks of this development, there should be no need to rebase this branch.

I have seen one issue while re-assigning keys. Sometimes the keypress "slips through" and triggers the existing key action. Not sure what happens here.

[axd1967]

I have enabled SPACE as the key to trigger the button action. Would ENTER be the better one? Or do the buttons react to ENTER on Ubuntu? I think it is not good to have both SPACE and ENTER available.

SPACE is a very common way to press a button or perform some task on the UI that has focus.

ENTER is less common; it is often connected to a "master" button in a dialog, AND can act on the UI element that has the focus. Just look at how Qt does it: ENTER also allows to confirm+close the confirmation dialog (see also #1292 ) (as well as SPACE).

I suggest you add one GUI panel per PR.

OK

As often, Alexander and I may have diverging preferences about these changes, and I definitely cannot guarantee that we will accept all changes. (or even ... almost guarantee we will not... but we can hopefully work out something useful.) I like those changes only when I see the usability becomes better, more intuitive, and tabbing always switches to some element that indicates that it is in focus.

Take a moment to ask yourself: if keyboard interaction obviously never was the highest priority, why would we now impose heavy requirements? There are some simple rules to follow, and tab order is a basic rule that has until now been ignored for whatever reason.

please remember: this is all about tab order. that's it. I just want to fix the order, that's all; there is no need to go into deep discussions, all that needs to be done is to re-order the tabbing for the simple reason that developers added new buttons everywhere without paying attention to the tab order, which is worse than imposing wrong tab orders...

Just open any decent desktop application and observe how tabbing is a standard and prominent feature I don't know many applications that fall short in tab ordering.

The focus issue is indeed not trivial, I'm still surprised that we need to go for a custom UI class, I still cannot believe that Qt is so fussy on that issue. But at least there is a simple solution, so it shouldn't hurt too much if one day a developer comes up and says "guys, this was not the way to do it", or maybe Qt will provide a simple solution in some future release.

(What about the tabs in TabViews? Is there keyboard interaction intended?)

The tabs also require attention. (a typical keyboard action is CTRL-PgUp/Down). That can be addressed in a separate issue.

For now, there are SpacePushButtons and SpaceCheckBoxes which can replace QPushButtons and QCheckBoxes, respectively. If you need others, I should be able to add them as well. During the next weeks of this development, there should be no need to rebase this branch.

OK. Can we then merge and close this PR?

I have seen one issue while re-assigning keys. Sometimes the keypress "slips through" and triggers the existing key action. Not sure what happens here.

I think you are referring to #1294.

[gzotti]

I intended to develop things around tabbing a bit further in this branch. The point is that tabbing was a non-issue, it was not bad, it was basically NOT AVAILABLE for the last 20 years of this project. Nobody cared for it, and I don't remember a single user complaining before 2021. You want to introduce it, so it must be done well. There are a few weeks to go before this could go to master.

[axd1967]

I intended to develop things around tabbing a bit further in this branch. The point is that tabbing was a non-issue, it was not bad, it was basically NOT AVAILABLE for the last 20 years of this project. Nobody cared for it, and I don't remember a single user complaining before 2021. You want to introduce it, so it must be done well. There are a few weeks to go before this could go to master.

OK. I will continue on this branch (via #1997 ), so please don't rebase it any more.

[axd1967]

Just one note: please remember that this is not a "critically important" feature; it has simply been neglected by developers and never contemplated by the vast majority of the users (my guess is that the UI is already unusual, therefore not inviting for this kind of change requests). Now is an opportunity to improve the keyboard UI because I am able to assist here to review that tabbing order (plus some other minor changes).

[github-actions]

This pull request has conflicts, please resolve those before we can evaluate the pull request.