Skip to content

Conversation

@hebasto
Copy link
Member

@hebasto hebasto commented Sep 15, 2018

Fix UX bug on Windows. Steps to reproduce (tested on Windows 10):

  1. In Options->Window enable "Minimize to the tray instead of the taskbar".
  2. Minimize the main window.
  3. Select Show/Hide on the tray icon menu (or just click on the tray icon).

The normal size window is expected.

Actually the main window is restored minimized to the taskbar.

Rationale: minimizing the window with fMinimizeToTray==true actually do both showMinimized() and hide(). During restoration isMinimized() should be checked first. Otherwise, the restored window will be minimized.

The logic is broken both on Windows and Linux, but on my Linux (Mint 19 + Cinnamon 3.8.9) the window manager renders the main window as if all is correct. Nevertheless, other Linux systems may be affected by this bug.

Ref:

Minimizing the window with fMinimizeToTray=true actually do both
showMinimized() and hide(). During restoration isMinimized() should be
checked first. Otherwise, the restored window will be minimized.
@fanquake fanquake added the GUI label Sep 15, 2018
@fanquake
Copy link
Member

@hebasto Can you have a look at #14123, as it might fix the issue you're seeing.

@DrahtBot
Copy link
Contributor

DrahtBot commented Sep 15, 2018

Reviewers, this pull request conflicts with the following ones:

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@jonasschnelli
Copy link
Contributor

Tested ACK f03c5c3

This successfully solves the "two step" show with the tray icon. Tested on all Ubuntu/macOS/Win.

@maflcko
Copy link
Member

maflcko commented Sep 27, 2018

Is this for backport?

@jonasschnelli
Copy link
Contributor

Not sure this is worth back-porting... All UI-glitch-fixes have the risk to act differently in back ported versions. I would recommend to not back port it (if its not a clear bug that has been fixed).

@DrahtBot
Copy link
Contributor

Coverage Change (pull 14222) Reference (master)
Lines +0.0044 % 87.0361 %
Functions +0.0154 % 84.1130 %
Branches -0.0190 % 51.5451 %

@hebasto
Copy link
Member Author

hebasto commented Oct 12, 2018

@Sjors Do you mind reviewing this PR?

@Sjors
Copy link
Member

Sjors commented Oct 13, 2018

I can't find the "Minimize to the tray instead of the taskbar" option on macOS 10.14 with QT 5.11.2.

@hebasto
Copy link
Member Author

hebasto commented Oct 13, 2018

@Sjors

/* Window elements init */
#ifdef Q_OS_MAC
/* remove Window tab on Mac */
ui->tabWidget->removeTab(ui->tabWidget->indexOf(ui->tabWindow));
#endif

@Sjors
Copy link
Member

Sjors commented Oct 14, 2018

Ok, this seems to have no effect on macOS, so works for me.

@promag does this get in the way of what you're trying to achieve in #14123? That PR fixes #13829, this PR doesn't.

@fanquake
Copy link
Member

Closing in favour of #14123, as that has more recent review, and according to this comment they should be fixing the same issue.

@fanquake fanquake closed this Oct 28, 2018
@hebasto hebasto deleted the windows-tray-icon branch November 8, 2018 22:12
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants