Keep focus on "Hide" while ModalOverlay is visible

[jadijadi]

During the initial sync, the Tab moves the focus to the widgets of the main window, even when the ModalOverlay is visible. This creates some weird rectangular selections on the screen.

This PR fixes this by keeping the focus on the "Hide" button while the ModalOverlay is visible.

Fixes #783

[DrahtBot]

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	pablomartin4btc
Concept ACK	BrandonOdiwuor
Stale ACK	hebasto

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

[jadijadi]

To reproduce the issue (tested on Mac & KDE):

• open bitcoin gui with wallet
• Hide the sync modal
• Go to send tab
• Highlight the Label
• Click on the progress bar to open the sync modal
• press TAB and a misplaced rectangular highlight should appear

[hebasto]

Why does the issue happen only on certain platforms?

[hebasto]

... keeping the focus on the "Hide" button while the ModalOverlay is visible.

That looks like a better commit message / PR description.

[jadijadi]

Why does the issue happen only on certain platforms?

Not sure about other platforms. I had a mac and a KDE machine and happened on both of them. Have not checked on other platforms.

[hebasto]

I can actually see two distinct issues here:

1. The "Hide" button doesn't have focus when the overlay shows for the first time. This can be resolved with ui->closeButton->setFocus(); in the ModalOverlay constructor.

2. The other issue is a bit convoluted. As a child widget, the overlay also follows its parent's tab order when processing "Tab" or "Shift+Tab" keys. Isolating the overlay's tab order might also be a solution to the issue.

This PR indeed forces focus on the "Hide" button as long as the overlay is visible. This approach is acceptable as the "Hide" button is the only one in the overlay.

Concept and approach ACK 1b34e0d. Also tested on Ubuntu 22.04.

[jadijadi]

I can actually see two distinct issues here:

1. The "Hide" button doesn't have focus when the overlay shows for the first time. This can be resolved with ui->closeButton->setFocus(); in the ModalOverlay constructor.
2. The other issue is a bit convoluted. As a child widget, the overlay also follows its parent's tab order when processing "Tab" or "Shift+Tab" keys. Isolating the overlay's tab order might also be a solution to the issue.

This PR indeed forces focus on the "Hide" button as long as the overlay is visible. This approach is acceptable as the "Hide" button is the only one in the overlay.

Concept and approach ACK 1b34e0d. Also tested on Ubuntu 22.04.

Thanks for the ACK. Algo agree with your point on better readability of layerIsVisible. Added that to the commit & squashed.

[hebasto]

cc @jarolrod @furszy @Sjors

[luke-jr]

Wouldn't it be better to hide the covered widgets instead?

[hebasto]

@jadijadi

Are you still working on this? If so, do you mind addressing #795 (comment)?

[jadijadi]

@jadijadi

Are you still working on this? If so, do you mind addressing #795 (comment)?

sorry I have missed that comment. Will check it and come back to you soon.

[jadijadi]

Wouldn't it be better to hide the covered widgets instead?

Thanks for the suggestion @luke-jr . This is the path I tried first but moved to the current solution because of two issues:

1. If I used parent()->findChildren<QWidget *>() to find the widgets "behind" this modal and call their hide(), the current modal and its closeButton will be disabled too. To prevent this we have to enable all the direct parents of the closeButton and this needs another loop or a few IMO ugly if (child != ui->closeButton....) clauses.
2. The above method might interfere with other sourced whom are "hiding" widget in the parent gui and can make future bugs. Imagine this situation:
   a. an element is disabled on the gui for any reason
   b. user opens the modal
   c. I will disable all widgets on the parent (and enable all parents on my CloseButton to enable it)
   d. user closes the modal
   e. I enable all widgets on the main window
   As you can see in this situation the initial state is lost. I have to keep track of the status of all widgets to prevent this.

Because of these two, I chose the method I implemented but I will be more than happy to hear your comments and update the PR accordingly.

[BrandonOdiwuor]

Concept ACK

[pablomartin4btc]

Concept & approach ACK 992b1bb

Tested this PR on Ubuntu 22.04 with GNOME desktop, not seen any issues with it but I couldn't reproduce the original issue, perhaps it depends on the platform or it's KDE's specific (?).

I agree with the reasoning on the author's taken approach, unless there's another problem that @luke-jr still sees with it.

[jadijadi]

Please do not hesitate to ask me if any other update is needed on this from my side. Thanks.