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

Change Dialog flag to Window to allow minimize/maximize buttons to be shown #1409

Merged
merged 2 commits into from
Nov 24, 2020

Conversation

kitchoi
Copy link
Contributor

@kitchoi kitchoi commented Nov 23, 2020

Fixes enthought/pyface#802
This is an alternative fix to #1078, i.e. it overrides #1083 partially.

This PR:

  • Change the Qt StickyDialog window flag from Dialog to Window. The StickyDialog is the main widget used when configure_traits or edit_traits are called with default settings.

With this, the only difference compared to TraitsUI 7.0.1 is that when resizable is false, the window flag is Qt.Window | QtCore.Qt.WindowSystemMenuHint instead of Qt.Dialog | QtCore.Qt.WindowSystemMenuHint. Before this change, the dialog may not have minimize/maximize buttons in some window managers.

In this part of the code base, there is in fact no information as to whether the window is intended to be top-level or a secondary window. I'd not be surprised if this then brings about other undesirable changes to the visuals on yet another desktop environment, or when one actually wants a secondary dialog-looking window and did so by setting resizable to false. In that case, we shall look into allowing additional information to be passed via edit_traits to select the appropriate setup based on more specific contextual information.

Details:
The issue in #1078 was observed where the first window has a Qt.Dialog flag (because it is not resizable) but the new window has a Qt.Window flag (because it is resizable). This was the original code:

if ui.view.resizable:
flags = QtCore.Qt.Window
else:
flags = QtCore.Qt.Dialog | QtCore.Qt.WindowSystemMenuHint

In retrospect, #1083 fixed #1078 by making both windows to have the same flag (out of Dialog or Window flag). CentOS window manager may have decided that "Dialog" should always be on top of "Window" (this is a guess).

Whether the window is actually resizable is handled by the subsequent call to setSizeConstraint(QtGui.QLayout.SetFixedSize), the Window and Dialog flags do not change that. However, a window manager may decorate the window frame differently based on these flags. In Ubuntu default desktop environment, it ignores the subsequent WindowMinimizeButtonHint and WindowMaximizeButtonHint flags if the Dialog flag is active. In CentOS, it similarly excludes the capability to maximize the window by double clicking the frame. Yet some X11 desktop environments (e.g. LXDE) respect these resize button hints and provide the buttons despite the Dialog flag.

On Windows, none of the above had any noticeable effect: Both WindowMinimizeButtonHint and WindowMaximizeButtonHint are respected such that the window can be maximized or minimized.

On OSX, when the Dialog flag is used, the minimize button is disabled, but the maximize button can still be used.

Screenshots for #1078 on Windows with this branch

main
subwindow
(Windows has not been affected on #1078 to start with, and is not affected by #1083 nor changes in this PR)

Screenshots for #1078 on MacOSX with this branch

Screenshot 2020-11-20 at 09 50 56
Screenshot 2020-11-20 at 09 51 06

Screenshots for #1078 on MacOSX with master

Screenshot 2020-11-20 at 09 46 35

(Note that the minimize button is disabled.)

Screenshots for #1078 on Ubuntu 18 with this branch

Screenshot 2020-11-23 at 13 03 20
Screenshot 2020-11-23 at 13 03 28

Screenshots for #1078 on Ubuntu 18 on master

Screenshot 2020-11-23 at 14 14 24

(Note the minimize and maximize buttons are not there.)

Understandably, no self-checking tests can be written for this.

…shown by more window managers.

Some window managers (e.g. Ubuntu 18, 20) interprets the Dialog | WindowMinimizeButtonHint | WindowMaximizeButtonHint
such that the window does not have minimize and maximize buttons on the window frame. Some window managers
(e.g. Fedora) do not do buttons but they do double clicking, and those functionalities can be ignored when
the window flag includes Dialog.

Some window managers (e.g. OSX, Ubuntu 16) interprets the Dialog | WindowMinimizeButtonHint | WindowMaximizeButtonHint
such that the minimize button is disabled, but the maximize button can still be used.

Some window managers (e.g. Windows, LXDE) would happily combine these flags as union, and allow minimize
and maximize buttons to be placed on a Dialog frame.
@corranwebster
Copy link
Contributor

In this part of the code base, there is in fact no information as to whether the window is intended to be top-level or a secondary window.

I think you can infer this from parent attribute passed into _StickyDialog.init(). If parent is None then this is a top-level window; if parent is anything else, then this window is a secondary window and so we can use this to determine whether it needs to be shown on top of another window and adapt appropriately. But this may be orthogonal to this PR.

@kitchoi
Copy link
Contributor Author

kitchoi commented Nov 23, 2020

I think you can infer this from parent attribute passed into _StickyDialog.init(). If parent is None then this is a top-level window; if parent is anything else, then this window is a secondary window and so we can use this to determine whether it needs to be shown on top of another window and adapt appropriately. But this may be orthogonal to this PR.

Thank you!

I realize I might have used the wrong vocabulary here trying to distinguish "top-level" versus "secondary". Sorry about that. Dialog is also a top-level window. I think I should have said is that there is no information as to whether the window is intended to be decorated like a main application window that is more standalone or a dialog intended for small tasks. I guess this is perhaps where window managers apply different interpretations as to what a dialog should look and behave compared to just a window.

@corranwebster
Copy link
Contributor

Ah, OK, that's a but different from what I thought you were saying. Other than erring on the side of assuming that things are decorated as a mainwindow, that sort of intention is hard to get out of TraitsUI. I guess if the window is modal, then it is intended to be short-lived and secondary, but there are non-modal windows that can also be intended that way (eg. a non-model search dialog).

@kitchoi
Copy link
Contributor Author

kitchoi commented Nov 24, 2020

Agreed. Thank you.

As an attempt to make sure #1078 has not come back, here is the screenshot for CentOS 8.2 default desktop environment:
Screenshot 2020-11-20 at 18 27 45
Two points to note here for CentOS: (1) The second window is still open on top of the first one (the point of #1078) and (2) one can now double click the second window to maximize the window.

@kitchoi kitchoi merged commit 5a99887 into master Nov 24, 2020
@kitchoi kitchoi deleted the pyface-802-window branch November 24, 2020 10:04
@kitchoi kitchoi added the need backport to maint/* PRs that need to be backported to a maintenance branch label Nov 24, 2020
@rahulporuri rahulporuri added this to In Progress in Enthought OSS Q4 2020 Nov 24, 2020
@rahulporuri rahulporuri moved this from In Progress to Done in Enthought OSS Q4 2020 Nov 24, 2020
@rahulporuri rahulporuri removed this from Done in Enthought OSS Q4 2020 Nov 24, 2020
kitchoi added a commit that referenced this pull request Nov 30, 2020
… shown (#1409)

* Use Window flag instead to allow minimize and maximize buttons to be shown by more window managers.

Some window managers (e.g. Ubuntu 18, 20) interprets the Dialog | WindowMinimizeButtonHint | WindowMaximizeButtonHint
such that the window does not have minimize and maximize buttons on the window frame. Some window managers
(e.g. Fedora) do not do buttons but they do double clicking, and those functionalities can be ignored when
the window flag includes Dialog.

Some window managers (e.g. OSX, Ubuntu 16) interprets the Dialog | WindowMinimizeButtonHint | WindowMaximizeButtonHint
such that the minimize button is disabled, but the maximize button can still be used.

Some window managers (e.g. Windows, LXDE) would happily combine these flags as union, and allow minimize
and maximize buttons to be placed on a Dialog frame.

(cherry picked from commit 5a99887)
kitchoi added a commit that referenced this pull request Nov 30, 2020
* Fix Qt IconButton with a clickable area too small (#1383)

* Fix IconButton with a clickable area too small.

If we set the maximum size using the icon size, the clickable area
ends up being too small to click (tested on OSX with FileEditor,
RangeEditor, ListEditor). If we don't set the maximum size, the
button takes up too much space. This sizeHint logic is similar
to how QToolButton.sizeHint is implemented.

* Expand docstring for sizeHint

* Call ensurePolished as it was called in the original implementation

(cherry picked from commit 8912de3)

* Enable the scrollable trait of a Group in the Qt backend (#1406)

* Enable the scrollable trait of a Group in the Qt backend.

* Tests that should exercise each code path for scrollable groups.

* Be more specific about getting widgets to avoid cross-platform issues.

* Ensure UI gets disposed properly.

* Fixes and improvements to tests, code, based on PR review.

(cherry picked from commit b73c475)

* Change Dialog flag to Window to allow minimize/maximize buttons to be shown (#1409)

* Use Window flag instead to allow minimize and maximize buttons to be shown by more window managers.

Some window managers (e.g. Ubuntu 18, 20) interprets the Dialog | WindowMinimizeButtonHint | WindowMaximizeButtonHint
such that the window does not have minimize and maximize buttons on the window frame. Some window managers
(e.g. Fedora) do not do buttons but they do double clicking, and those functionalities can be ignored when
the window flag includes Dialog.

Some window managers (e.g. OSX, Ubuntu 16) interprets the Dialog | WindowMinimizeButtonHint | WindowMaximizeButtonHint
such that the minimize button is disabled, but the maximize button can still be used.

Some window managers (e.g. Windows, LXDE) would happily combine these flags as union, and allow minimize
and maximize buttons to be placed on a Dialog frame.

(cherry picked from commit 5a99887)

* Add GitHub Actions workflows targeting pip-installed dependencies (#1415)

* Add GitHub Actions workflow excluding EDM

* Shorten names

* Shortening name

* Add workflow for targeting Traits 6

* EXCLUDE_TESTS is no longer used

* Split minimal test to a separate workflow as it may be scheduled differently

* Remove job for wxPython + Ubuntu due to lack of allow-to-fail

* Update wheel before install

* Remove comment about prebuilt wheel being incompatible

The wheels for wxPython 4.1.0 did not work well on Python 3.8 and 3.9.
But the wheels for wxPython 4.1.1 work.

* Use Python 3.6 for per-PR run

* Final update to set on-schedule and on-pull-request

(cherry picked from commit e14ca3e)

* Reduce CI matrix on Appveyor (#1417)

* Reduce matrix on Appveyor as part of migration to GitHub Actions

* Add Windows to the workflow for null backend

(cherry picked from commit 38e5e33)

* Update changelog

* CLN : Use normal string, not f-string (#1393)

In this specific case, there was nothing that needed to be formatted in the string
so we can just use a normal string.

This was reported as a flake8 failure in the CI job
- https://travis-ci.org/github/enthought/traitsui/jobs/738921895

(cherry picked from commit c5438d5)

Co-authored-by: Corran Webster <cwebster@enthought.com>
Co-authored-by: Poruri Sai Rahul <rporuri@enthought.com>
@kitchoi kitchoi removed the need backport to maint/* PRs that need to be backported to a maintenance branch label Dec 1, 2020
kitchoi added a commit that referenced this pull request Dec 1, 2020
* Fix Qt IconButton with a clickable area too small (#1383)

* Fix IconButton with a clickable area too small.

If we set the maximum size using the icon size, the clickable area
ends up being too small to click (tested on OSX with FileEditor,
RangeEditor, ListEditor). If we don't set the maximum size, the
button takes up too much space. This sizeHint logic is similar
to how QToolButton.sizeHint is implemented.

* Expand docstring for sizeHint

* Call ensurePolished as it was called in the original implementation

(cherry picked from commit 8912de3)

* Enable the scrollable trait of a Group in the Qt backend (#1406)

* Enable the scrollable trait of a Group in the Qt backend.

* Tests that should exercise each code path for scrollable groups.

* Be more specific about getting widgets to avoid cross-platform issues.

* Ensure UI gets disposed properly.

* Fixes and improvements to tests, code, based on PR review.

(cherry picked from commit b73c475)

* Change Dialog flag to Window to allow minimize/maximize buttons to be shown (#1409)

* Use Window flag instead to allow minimize and maximize buttons to be shown by more window managers.

Some window managers (e.g. Ubuntu 18, 20) interprets the Dialog | WindowMinimizeButtonHint | WindowMaximizeButtonHint
such that the window does not have minimize and maximize buttons on the window frame. Some window managers
(e.g. Fedora) do not do buttons but they do double clicking, and those functionalities can be ignored when
the window flag includes Dialog.

Some window managers (e.g. OSX, Ubuntu 16) interprets the Dialog | WindowMinimizeButtonHint | WindowMaximizeButtonHint
such that the minimize button is disabled, but the maximize button can still be used.

Some window managers (e.g. Windows, LXDE) would happily combine these flags as union, and allow minimize
and maximize buttons to be placed on a Dialog frame.

(cherry picked from commit 5a99887)

* Add GitHub Actions workflows targeting pip-installed dependencies (#1415)

* Add GitHub Actions workflow excluding EDM

* Shorten names

* Shortening name

* Add workflow for targeting Traits 6

* EXCLUDE_TESTS is no longer used

* Split minimal test to a separate workflow as it may be scheduled differently

* Remove job for wxPython + Ubuntu due to lack of allow-to-fail

* Update wheel before install

* Remove comment about prebuilt wheel being incompatible

The wheels for wxPython 4.1.0 did not work well on Python 3.8 and 3.9.
But the wheels for wxPython 4.1.1 work.

* Use Python 3.6 for per-PR run

* Final update to set on-schedule and on-pull-request

(cherry picked from commit e14ca3e)

* Reduce CI matrix on Appveyor (#1417)

* Reduce matrix on Appveyor as part of migration to GitHub Actions

* Add Windows to the workflow for null backend

(cherry picked from commit 38e5e33)

* Update changelog

* CLN : Use normal string, not f-string (#1393)

In this specific case, there was nothing that needed to be formatted in the string
so we can just use a normal string.

This was reported as a flake8 failure in the CI job
- https://travis-ci.org/github/enthought/traitsui/jobs/738921895

(cherry picked from commit c5438d5)

Co-authored-by: Corran Webster <cwebster@enthought.com>
Co-authored-by: Poruri Sai Rahul <rporuri@enthought.com>
(cherry picked from commit 7e9a54a)
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.

No minimize and maximize buttons on a window bar on the latest pyface and linux
2 participants