Skip to content

raise helpMessageDialog #167

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.

Sign up for GitHub

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

Jump to bottom
Merged
maflcko merged 1 commit into from
Jan 26, 2021
Merged

raise helpMessageDialog #167

maflcko merged 1 commit into from
Jan 26, 2021

Conversation

@RandyMcMillan
Copy link
Contributor

@RandyMcMillan RandyMcMillan commented Dec 29, 2020

the raise() method brings the helpMessageDialog to the top if it is obscured by another window.

promag
promag reviewed Dec 29, 2020
View reviewed changes
src/qt/bitcoingui.cpp Outdated
void BitcoinGUI::showHelpMessageClicked()
{
helpMessageDialog->show();
helpMessageDialog->raise();//bring to top if obscured
Copy link
Contributor

@promag promag Dec 29, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Not sure if you are aware of GUIUtil::bringToFront(helpMessageDialog), seems the right call here (
and remove the above show()).

Copy link
Contributor Author

@RandyMcMillan RandyMcMillan Jan 25, 2021

Choose a reason for hiding this comment

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

Thanks!

@hebasto
Copy link
Member

hebasto commented Dec 30, 2020

@RandyMcMillan

Mind providing steps to reproduce helpMessageDialog obscuring (as it is a top-level window, and it is activated via a menu item only)?

jarolrod
jarolrod reviewed Jan 10, 2021
View reviewed changes
Copy link
Contributor

@jarolrod jarolrod left a comment

Choose a reason for hiding this comment

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

No strong opinion, but I do not think this is needed.

The help menu always shows on top, I cannot come up with a scenario where it is obfuscated. Furthermore, If the help window has been moved to the background by clicking on another window, clicking on Help -> Command-line Options will bring back the help window to the top.

@RandyMcMillan
Copy link
Contributor Author

RandyMcMillan commented Jan 23, 2021

i will post screen shots - maybe this is a macOS specific issue..

@RandyMcMillan
Copy link
Contributor Author

RandyMcMillan commented Jan 23, 2021

Maybe this is an "accessibility" issue and warrants an accessibility label - note that if the user is using tab-thru navigation. Once the window is presented and then obfuscated - using the nav bar again will not re-present the window.

I think is is critical that we incorporate usability/accessibility considerations - I would hope that this gui centric fork allows for these very nuanced issues.

Not everybody can use a mouse in gui applications. :)

Screen Shot 2021-01-22 at 7 21 41 PM

Screen Shot 2021-01-22 at 7 21 33 PM

@hebasto
Copy link
Member

hebasto commented Jan 23, 2021

Concept ACK.

I can confirm this bug on macOS Big Sur 11.1 (20C69) + Homebrew's Qt 5.15.2.

@RandyMcMillan Do you mind using a more general approach suggested by @promag:

--- a/src/qt/bitcoingui.cpp
+++ b/src/qt/bitcoingui.cpp
@@ -845,7 +845,7 @@ void BitcoinGUI::showDebugWindowActivateConsole()
 
 void BitcoinGUI::showHelpMessageClicked()
 {
-    helpMessageDialog->show();
+    GUIUtil::bringToFront(helpMessageDialog);
 }
 
 #ifdef ENABLE_WALLET

@RandyMcMillan
Copy link
Contributor Author

RandyMcMillan commented Jan 25, 2021

Thanks for posting the snippet - saved a lot of time!

promag
promag reviewed Jan 25, 2021
View reviewed changes
Copy link
Contributor

@promag promag left a comment

Choose a reason for hiding this comment

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

Code review ACK 7711446.

hebasto
hebasto approved these changes Jan 25, 2021
View reviewed changes
Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

ACK 7711446, tested on:

  • macOS Big Sur 11.1 (20C69) + Homebrew's Qt 5.15.2
  • Linux Mint 20.1 + Cinnamon + Qt 5.12.8

@maflcko
Copy link
Contributor

maflcko commented Jan 26, 2021

Does this need backport?

@hebasto
Copy link
Member

hebasto commented Jan 26, 2021

Does this need backport?

Yes, I think, as it is a bugfix for macOS builds.

@maflcko
Copy link
Contributor

maflcko commented Jan 26, 2021

ok, thanks. Will backport.

@maflcko maflcko merged commit cbe4396 into bitcoin-core:master Jan 26, 2021
maflcko pushed a commit to maflcko/bitcoin-core that referenced this pull request Jan 26, 2021
@RandyMcMillan
raise helpMessageDialog
e2ebc85 
Github-Pull: bitcoin-core/gui#167
Rebased-From: 7711446
@maflcko
Copy link
Contributor

maflcko commented Jan 26, 2021

Backported in bitcoin/bitcoin#20901

@maflcko maflcko added this to the 0.21.1 milestone Jan 26, 2021
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Jan 26, 2021
Merge bitcoin-core/gui#167: raise helpMessageDialog
a2953bd 
7711446 raise helpMessageDialog (randymcmillan)

Pull request description:

  the raise() method brings the helpMessageDialog to the top if it is obscured by another window.

ACKs for top commit:
  promag:
    Code review ACK 7711446.
  hebasto:
    ACK 7711446, tested on:

Tree-SHA512: 0d5b107aa9a5ce3891e88ef69f64461c8b23d17476b798691119e84bfc78e16b2491c798adb5d6cc347af3b7f18729593d7924090c336114a3cf34fbee344bfb
@hebasto hebasto added Bug Something isn't working macOS labels Mar 6, 2021
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jul 1, 2021
@PastaPastaPasta
Merge bitcoin-core/gui#167: raise helpMessageDialog
56bde91 
7711446 raise helpMessageDialog (randymcmillan)

Pull request description:

  the raise() method brings the helpMessageDialog to the top if it is obscured by another window.

ACKs for top commit:
  promag:
    Code review ACK 7711446.
  hebasto:
    ACK 7711446, tested on:

Tree-SHA512: 0d5b107aa9a5ce3891e88ef69f64461c8b23d17476b798691119e84bfc78e16b2491c798adb5d6cc347af3b7f18729593d7924090c336114a3cf34fbee344bfb
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jul 16, 2021
@PastaPastaPasta
Merge bitcoin-core/gui#167: raise helpMessageDialog
403d2a0 
7711446 raise helpMessageDialog (randymcmillan)

Pull request description:

  the raise() method brings the helpMessageDialog to the top if it is obscured by another window.

ACKs for top commit:
  promag:
    Code review ACK 7711446.
  hebasto:
    ACK 7711446, tested on:

Tree-SHA512: 0d5b107aa9a5ce3891e88ef69f64461c8b23d17476b798691119e84bfc78e16b2491c798adb5d6cc347af3b7f18729593d7924090c336114a3cf34fbee344bfb
apoelstra pushed a commit to apoelstra/elements that referenced this pull request Sep 21, 2021
@RandyMcMillan @apoelstra
raise helpMessageDialog
5d4db43 
Github-Pull: bitcoin-core/gui#167
Rebased-From: 7711446
(cherry picked from commit e2ebc85)
apoelstra pushed a commit to apoelstra/elements that referenced this pull request Sep 21, 2021
@RandyMcMillan @apoelstra
raise helpMessageDialog
176a257 
Github-Pull: bitcoin-core/gui#167
Rebased-From: 7711446
(cherry picked from commit e2ebc85)
apoelstra pushed a commit to apoelstra/elements that referenced this pull request Sep 22, 2021
@RandyMcMillan @apoelstra
raise helpMessageDialog
b514a87 
Github-Pull: bitcoin-core/gui#167
Rebased-From: 7711446
(cherry picked from commit e2ebc85)
gades pushed a commit to cosanta/cosanta-core that referenced this pull request May 9, 2022
@gades
Merge bitcoin-core/gui#167: raise helpMessageDialog
e7c2087 
7711446 raise helpMessageDialog (randymcmillan)

Pull request description:

  the raise() method brings the helpMessageDialog to the top if it is obscured by another window.

ACKs for top commit:
  promag:
    Code review ACK 7711446.
  hebasto:
    ACK 7711446, tested on:

Tree-SHA512: 0d5b107aa9a5ce3891e88ef69f64461c8b23d17476b798691119e84bfc78e16b2491c798adb5d6cc347af3b7f18729593d7924090c336114a3cf34fbee344bfb
@bitcoin-core bitcoin-core locked as resolved and limited conversation to collaborators Aug 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@hebasto hebasto hebasto approved these changes

+2 more reviewers

@promag promag promag left review comments

@jarolrod jarolrod jarolrod left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Bug Something isn't working macOS

Projects

None yet

Milestone

0.21.1

Development

Successfully merging this pull request may close these issues.

5 participants

@RandyMcMillan @hebasto @maflcko @promag @jarolrod