Skip to content

Don't create a statusbar in Qt, wx backends. #17092

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

Merged
merged 3 commits into from
May 8, 2020

Conversation

anntzer
Copy link
Contributor

@anntzer anntzer commented Apr 10, 2020

Display the coordinates text on the right of the toolbar, consistently
with the GTK3, wx, and (I think?) macosx backends. This helps when
embedding Matplotlib in larger GUIs, because Matplotlib may not
necessarily control the statusbar (which is typically global for the
whole window) but controls the toolbar if there's one.

If we decide to go this route we could probably also kill the status bar
for toolmanager.

xref #17085.

PR Summary

PR Checklist

  • Has Pytest style unit tests
  • Code is Flake 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

@anntzer anntzer added this to the v3.3.0 milestone Apr 10, 2020
@anntzer anntzer added the status: needs comment/discussion needs consensus on next step label Apr 10, 2020

Qt and wx backends no longer create a status bar by default
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
The coordinates information is not displayed in the toolbar, consistently with
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
The coordinates information is not displayed in the toolbar, consistently with
The coordinates information is now displayed in the toolbar, consistently with

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup, just a typo.

Copy link
Member

Choose a reason for hiding this comment

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

but a very confusing typo!

Copy link
Member

@timhoffm timhoffm left a comment

Choose a reason for hiding this comment

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

I understand the motivation for unification, and I also see the argument that we don't necessarily control the status bar. However I have some concerns with this:

  • The coordinates were shown in the status bar in the bottom left. Now they move to the top right in the toolbar, which may be surprising for users.
  • semi-related: Some backends have the toolbar at the top of the figure window, some at the bottom. This is also confusing.
  • The coordinates clunch to the top right corner of the status bar with an arbitrary margin to the right. This looks really ugly (personal taste, but really).
    image
  • If one make the figure a little smaller, the coordinates are not displayed anymore because the label is clipped
    image

@@ -555,23 +555,21 @@ def __init__(self, canvas, num):
if self.toolbar:
backend_tools.add_tools_to_container(self.toolbar)
self.statusbar = StatusbarQt(self.window, self.toolmanager)
sb_height = self.statusbar.sizeHint().height()
else:
sbs_height = 0
Copy link
Member

@timhoffm timhoffm Apr 11, 2020

Choose a reason for hiding this comment

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

Typo? Is this the same as sb_height? If so, please also make sure that the variable is set via all condition code paths. If not, can we make this variables more distinctive?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

actually they should all be sbs_height, fixed

@anntzer anntzer force-pushed the nostatbar branch 2 times, most recently from 88000b1 to e82fc97 Compare April 11, 2020 08:30
@anntzer
Copy link
Contributor Author

anntzer commented Apr 11, 2020

The coordinates were shown in the status bar in the bottom left. Now they move to the top right in the toolbar, which may be surprising for users.

In general I know this may be a bit too disruptive to be worth it -- that's why I tagged as "needs comments/discussion". But I'm not sure there's much for me to argue about here, other than "I don't think it's that bad?"

semi-related: Some backends have the toolbar at the top of the figure window, some at the bottom. This is also confusing.

Well, fixing that would be even more disruptive :-)

The coordinates clunch to the top right corner of the status bar with an arbitrary margin to the right. This looks really ugly (personal taste, but really).

Looks like it was just a matter of .rstrip()ing the string (done), does that help for you?

If one make the figure a little smaller, the coordinates are not displayed anymore because the label is clipped

  • This has always been the case for gtk3/tk, which are admittedly less used than qt (although tk was/is(?) the default on C. Gohlke's Windows wheels) but still this suggests this isn't too much of a real problem.
  • Make cursor text precision actually correspond to pointing precision. #16776 (wink) will make things better by shortening the cursor texts, removing non-significant digits.
  • I pushed a separate commit with two more things that help here: 1) move the image pixel value to a second line (this particularly helps if the image uses RGB values (i.e. 3 floats); 2) don't display the "mode" (zoom/pan), which is now quite redundant with the toolbar button which should appear as being depressed on the same toolbar.

@timhoffm
Copy link
Member

[...] which may be surprising for users.

In general I know this may be a bit too disruptive to be worth it -- that's why I tagged as "needs comments/discussion". But I'm not sure there's much for me to argue about here, other than "I don't think it's that bad?"

Probably. I'm not categorically opposed, but we should consider this in our decision.

The coordinates [...] looks really ugly (personal taste, but really).

Looks like it was just a matter of .rstrip()ing the string (done), does that help for you?

Yes, that's better, but only half of it. Can we center the label vertically? I don't see a reason why we need to align it to the top (backend_qt5.py l.716). For multi-line text the label will expand in both directions, so two lines should still be fine. There's only an issue for more lines, which then would show only the middle section, not the beginning i guess (not tested). If one wanted to be 100% save, one could change the alignment to center for single lines and top for multi line texts.

Merged. Please rebase so that I can test that more easily 😄.

  • I pushed a separate commit with two more things that help here: 1) move the image pixel value to a second line (this particularly helps if the image uses RGB values (i.e. 3 floats); 2) don't display the "mode" (zoom/pan), which is now quite redundant with the toolbar button which should appear as being depressed on the same toolbar.

looks good.

@anntzer
Copy link
Contributor Author

anntzer commented Apr 11, 2020

Rebased.

Yes, that's better, but only half of it. Can we center the label vertically? I don't see a reason why we need to align it to the top (backend_qt5.py l.716). For multi-line text the label will expand in both directions, so two lines should still be fine. There's only an issue for more lines, which then would show only the middle section, not the beginning i guess (not tested). If one wanted to be 100% save, one could change the alignment to center for single lines and top for multi line texts.

Agreed centered looks better; changed that.
I don't think we ever manually set the message to more than two lines (or even more than one line before this PR -- it used to go to the status bar after all, so probably setting more than one line wasn't supported at all anyways); a quick check shows that setting the text to more than two lines just shows the first two lines and crops the rest.

@anntzer
Copy link
Contributor Author

anntzer commented Apr 11, 2020

Also, I think in the wx case, this is "clearly" an esthetic improvement (sure, it's all subjective, but heh):

before
old

after
new

@timhoffm
Copy link
Member

Out of curiosity: What are your system settings so that the wx toolbar is dark. I'm on Kubuntu with a dark theme. The Qt toolbar is dark as expected, but the wx toolbar is still light and also has larger icon margins:

image

@anntzer
Copy link
Contributor Author

anntzer commented Apr 11, 2020

Arch + Breeze Dark (also for GTK, per https://wiki.archlinux.org/index.php/Uniform_look_for_Qt_and_GTK_applications#Styles_for_both_Qt_and_GTK).

Probably would be nice to have the same thing in wx to make buttons light when on a dark background as you did for Qt (that was great :-)), but I don't use wx often enough for this to be worth the trouble...

@timhoffm
Copy link
Member

Probably would be nice to have the same thing in wx to make buttons light when on a dark background as you did for Qt (that was great :-))

I was about to write that, but couldn't get the dark backfround to test.

@ImportanceOfBeingErnest
Copy link
Member

One reason to not use the tk backend for me was always that it resizes the figure due to the coordinates being shown inside the navigation bar.

tkresize

Now even if no resizing occurs in Qt, the fact that the coordinates are not visible for small figures is not desireable.

@anntzer
Copy link
Contributor Author

anntzer commented Apr 11, 2020

@timhoffm If you want to fix icon colors for dark themes + gtk, I have a mostly working patch at https://gist.github.com/anntzer/8909837f8438210538336651a87aed9c; feel free to adopt it.

@ImportanceOfBeingErnest I'm a bit confused by what you're saying about resizes due to the coordinates being shown? Most often I'd assume you use the mouse to resize the window, in which case the cursor is not over the canvas and thus no coordinates are being shown? I agree the situation with very small windows is perhaps not great.

OTOH the other solutions are either 1) to also move the text to the statusbar for tk/gtk/macosx (not sure for the last one), together with, hopefully, a consistent decision as to what to do in the embedding case where we may or may not control the statusbar; or 2) to just not bother about consistency here at all.

@ImportanceOfBeingErnest
Copy link
Member

I'm a bit confused by what you're saying about resizes due to the coordinates being shown? Most often I'd assume you use the mouse to resize the window, in which case the cursor is not over the canvas and thus no coordinates are being shown?

Maybe it's not actually obvious from the screen capture above, but the problem is that the window and hence the figure resizes itself because while the mouse is over the figure, the coordinates change and thereby the width of the nav bar.

@anntzer
Copy link
Contributor Author

anntzer commented Apr 11, 2020

Ah, but that just looks like a bug? The window should just keep its size and crop the text as appropriate?

@ImportanceOfBeingErnest
Copy link
Member

Might be a bug, but if the text is cropped, you have no chance of reading the text.

@anntzer
Copy link
Contributor Author

anntzer commented Apr 11, 2020

Fair enough, but I cannot think of another program which changes its window size just because a label would be cropped.

Thinking about it again, another option would be to switch to a tooltip when the label would be cropped...

@timhoffm
Copy link
Member

Is it really that bad if the coordinates are not displayed for small plots? I'd argue that you don't have much precision in small plots anyway so the benefit would be limited. But tooltip might be good.

@ImportanceOfBeingErnest
Copy link
Member

Yes, I consider it as bad, if just because you have a small figure, you cannot see the coordinates any more.

A tooltip may be a solution, just maybe not within the axes, because that would hide the data beneath it. Maybe at the lower corner of the figure, or even within the navigation bar (which will not be used anyways while having the mouse in the figure).
image

@anntzer
Copy link
Contributor Author

anntzer commented Apr 11, 2020

Do you have an implementation, or was that just some inkscaping? :-)

@ImportanceOfBeingErnest
Copy link
Member

No implementation, just 20 seconds of making a screenshot and putting a text box in it.

@anntzer
Copy link
Contributor Author

anntzer commented Apr 11, 2020

Ok, so over the toolbar semms fine?

@ImportanceOfBeingErnest
Copy link
Member

I think so. The widget would only cover up the underlying buttons, if the figure is small, and in all the "usual" cases it would look exactly as if the label was part of the widget layout, right? I would say that such small aestetical problem is bearable.
Is it then still possible to verically center-align the thing?

@jklymak
Copy link
Member

jklymak commented Apr 11, 2020

Will over the tool bar will cover the axes annotations if they are against the edge of the figure (ie after calling constrained/tight layout)?

@ImportanceOfBeingErnest
Copy link
Member

(I think) we're currently discussing something along the lines of the right image in this comment.

@anntzer
Copy link
Contributor Author

anntzer commented Apr 11, 2020

(I think) we're currently discussing something along the lines of the right image in this comment.

Yes.

in all the "usual" cases it would look exactly as if the label was part of the widget layout, right? I would say that such small aestetical problem is bearable.

Yes.

Is it then still possible to verically center-align the thing?

Just a bit more work, but not anything difficult, so sure (it's just measuring toolbar height and manual positioning and yada yada).

@jklymak
Copy link
Member

jklymak commented Apr 11, 2020

I guess I prefer that to the left image, but, still kinda looks like a mistake. I'd be fine if the toolbar were 12 points higher and the output went above...

@jklymak
Copy link
Member

jklymak commented Apr 11, 2020

...though I guess it would only cover the toolbar if it were in a small window? I guess that seems fine.

@anntzer
Copy link
Contributor Author

anntzer commented Apr 11, 2020

Yes, it's only for tiny windows that this matters.

@timhoffm
Copy link
Member

The sketched tooltip examples look like graphics errors to me. Waiting for the actual tooltip, but I think I don‘t like the covering of the controls. It could be an alternative to collapse all controls into a dropdown toolbutton if there is not enough space.

Copy link
Member

@timhoffm timhoffm left a comment

Choose a reason for hiding this comment

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

To me, this is good as is (If the window gets too small, coordinates are not shown; but with the new precision-aware formatting the coordinates are not too large.)

@tacaswell
Copy link
Member

I'm having a bit of trouble following this discussion. @ImportanceOfBeingErnest can you comment on if you are happy with where this landed? If so please merge it for 3.3, if not we can push it to 3.4.

@anntzer
Copy link
Contributor Author

anntzer commented Apr 30, 2020

rebased

@QuLogic
Copy link
Member

QuLogic commented Apr 30, 2020

Qt will add a little arrow when the toolbar is overfull, so moving the label there will not cause the same behaviour as in Tk. Whether we should instead move the label into a tooltip or overlay is a different question, but as this brings our backends in line with each other, I'm 👍 on this even if we don't answer the broader question yet.

@QuLogic
Copy link
Member

QuLogic commented Apr 30, 2020

On wx, it uses GTK as its backend (for me), which crops the toolbar as well, so no weird figure resizing either. Not sure what other toolkits it might use.

anntzer added 3 commits May 7, 2020 14:24
Display the coordinates text on the right of the toolbar, consistently
with the GTK3, wx, and (I think?) macosx backends.  This helps when
embedding Matplotlib in larger GUIs, because Matplotlib may not
necessarily control the statusbar (which is typically global for the
whole window) but controls the toolbar if there's one.

If we decide to go this route we could probably also kill the status bar
for toolmanager.
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.

6 participants