NotificationsYou must be signed in to change notification settings
Fork7.9k
Star21.4k

Switch tk pan/zoom to use togglable buttons.#17102

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

QuLogic merged 1 commit intomatplotlib:masterfromanntzer:tkactions

Apr 20, 2020

Merged

Switch tk pan/zoom to use togglable buttons.#17102

QuLogic merged 1 commit intomatplotlib:masterfromanntzer:tkactions

Apr 20, 2020

Conversation

Copy link

Contributor

anntzer commentedApr 11, 2020

... consistently with other backends.

The code for syncing button state is similar to the one for gtk3.

PR Summary

PR Checklist

Has Pytest style unit tests
Code isFlake 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 added the GUI: tk label

Apr 11, 2020

anntzer added this to thev3.3.0 milestone

Apr 11, 2020

Copy link

Member

timhoffm commentedApr 12, 2020•
edited
Loading

Seems to work, but the toggle state is hardly visible for me:

Nothing toggled:

Pan toggled:

Is there a way to make this more obvious?

Also, if you look closely, the toogle buttons have a smaller edge than the regular buttons:

[toggled pan] [untoggled zoom] [regular button]

which makes the whole toolbar look a bit uneven

Copy link

ContributorAuthor

anntzer commentedApr 12, 2020

I think you'd need ttk (https://docs.python.org/3/library/tkinter.ttk.html#module-tkinter.ttk) for that, not sure how widely available it is.

diff --git i/lib/matplotlib/backends/_backend_tk.py w/lib/matplotlib/backends/_backend_tk.pyindex d8bf524f2..065b78766 100644--- i/lib/matplotlib/backends/_backend_tk.py+++ w/lib/matplotlib/backends/_backend_tk.py@@ -4,6 +4,7 @@ import math import os.path import sys import tkinter as tk+from tkinter import ttk from tkinter.simpledialog import SimpleDialog import tkinter.filedialog import tkinter.messagebox@@ -539,8 +540,8 @@ class NavigationToolbar2Tk(NavigationToolbar2, tk.Frame):         else:             im = None         if not toggle:-            b = tk.Button(master=self, text=text, padx=2, pady=2, image=im,-                          command=command)+            b = ttk.Button(master=self, text=text, padding=0, image=im,+                           command=command)         else:             # There is a bug in tkinter included in some python 3.6 versions             # that without this variable, produces a "visual" toggling of@@ -548,10 +549,12 @@ class NavigationToolbar2Tk(NavigationToolbar2, tk.Frame):             # https://bugs.python.org/issue29402             # https://bugs.python.org/issue25684             var = tk.IntVar()-            b = tk.Checkbutton(master=self, text=text, padx=2, pady=2,-                               image=im, indicatoron=False,-                               command=command,-                               variable=var)+            # Matplotlib.Toolbutton "inherits" from Toolbutton.+            ttk.Style().configure("Matplotlib.Toolbutton", relief="raised")+            b = ttk.Checkbutton(master=self, text=text, padding=2,+                                image=im,,+                                command=command,+                                variable=var)             b.var = var         b._ntimage = im         b.pack(side=tk.LEFT)

works nicely here. (Not sure things work well on a dark theme though.)

Copy link

Member

timhoffm commentedApr 13, 2020

Works reasonably on dark mode:

Tooltips are not readable, but that was the same with tk.Button.

tacaswell reviewed

Apr 13, 2020

View reviewed changes

lib/matplotlib/backends/_backend_tk.py

		@@ -717,7 +754,8 @@ def __init__(self, toolmanager, window):
		def add_toolitem(
		self, name, group, position, image_file, description, toggle):
		frame = self._get_groupframe(group)
		button = self._Button(name, image_file, toggle, frame)
		button = NavigationToolbar2Tk._Button(self, name, image_file, toggle,

Copy link

Member

Choose a reason for hiding this comment

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

Might as well make this an un-bound top level function?

Copy link

ContributorAuthor

Choose a reason for hiding this comment

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

I've been redoing most of the MEP22 implementation to redirect to NavigationToolbar2 everywhere instead of having duplicate implementations; I'd rather keep things there and if we ever switch to MEP22-by-default (which I would rather not, I have deep misgivings about that API design discussed at length elsewhere, but no need to relitigate that here), move everything to the MEP22 classes and point NavigationToolbar2 to them. IOW: I don't think it's worth turning this into a free function "just because" MEP22 also uses it.

QuLogic approved these changes

Apr 14, 2020

View reviewed changes

timhoffm reviewed

Apr 19, 2020

View reviewed changes

lib/matplotlib/backends/_backend_tk.py Outdated

		else:
		im = None
		if not toggle:
		b = tk.Button(master=self, text=text, padx=2, pady=2, image=im,

Copy link

Member

timhoffmApr 19, 2020•
edited
Loading

Choose a reason for hiding this comment

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

Note thatpadx/y do not have any effect on images with buttons:
https://www.tcl.tk/man/tcl8.6/TkCmd/options.htm#M-padx

Most widgets only use this option for padding text: if they are displaying a bitmap or image, then they usually ignore padding options.

These options could be removed (also below for Checkbutton).

Copy link

Member

timhoffm commentedApr 19, 2020•
edited
Loading

I'm a bit reluctant of using ttk. It changes at least the visual impression and maybe would also be an API change. Therefore I tried to achieve same-size buttons in pure tk by tuning options.

The border for Checkbuttons is drawn differently and seems to be smaller than for regular buttons (see screenshots).

I've played around with Checkbuttonborderwidth but thid did not give satisfying results. The best solution I found for getting equal-sized buttons is manually increasing the image size:

            # workaround for making the Checkbutton equally large to a Button            im = tk.PhotoImage(master=self, file=image_file,                               width=im.width() + 2, height=im.height() + 2)

But with the large background one can see that the image is rendered in the top left.

Probably the visually best solution would be to have a 1px transparent border around the original image. I'm not an Tk expert and the docs are rather cryptic, so I did not manage this. If anybody knows please let me know.

anntzer force-pushed thetkactions branch fromaf79080 to7e1c484Compare

April 19, 2020 15:22

Copy link

ContributorAuthor

anntzer commentedApr 19, 2020

Got rid of the padding. Didn't play more with the styling.

Copy link

Member

timhoffm commentedApr 19, 2020•
edited
Loading

Did you already see my last message on styling?

anntzer force-pushed thetkactions branch from7e1c484 to39aea35Compare

April 19, 2020 15:32

Copy link

ContributorAuthor

anntzer commentedApr 19, 2020

Yes. I didn't do any of it but can manually add the two pixels to the size if you prefer (I personally think having checkbuttons, even with a small discrepancy in button size (tbh I didn't even notice it before you mentioned it) is already an improvement).

timhoffm reviewed

Apr 19, 2020

View reviewed changes

Copy link

Member

timhoffm left a comment•
edited
Loading

Choose a reason for hiding this comment

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

Conditional on CI. (seems like a crossed PR issue).

Toggle buttons are an improvement. Improving the style can also be done later if you don't want to dig into it.

timhoffm approved these changes

Apr 19, 2020

View reviewed changes

Copy link

Member

timhoffm left a comment•
edited
Loading

Choose a reason for hiding this comment

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

Conditional on CI. (seems like a crossed PR issue).

Toggle buttons are an improvement. Improving the style can also be done later if you don't want to dig into it.

Switch tk pan/zoom to use togglable buttons.

9ce05e6

... consistently with other backends.The code for syncing button state is similar to the one for gtk3.

anntzer force-pushed thetkactions branch from39aea35 to9ce05e6Compare

April 19, 2020 22:09

Copy link

ContributorAuthor

anntzer commentedApr 20, 2020

good to go?

QuLogic merged commit3a2c8f3 intomatplotlib:master

Apr 20, 2020

anntzer deleted the tkactions branch

April 20, 2020 19:56

richardsheridan mentioned this pull request

Aug 14, 2020

New IntVar needs a parent widget#18259

Closed

Labels

GUI: tk

4 participants

Movatterモバイル変換

Uh oh!

Switch tk pan/zoom to use togglable buttons.#17102

Switch tk pan/zoom to use togglable buttons.#17102

Uh oh!

Conversation

anntzer commentedApr 11, 2020

PR Summary

PR Checklist

Uh oh!

timhoffm commentedApr 12, 2020• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Uh oh!

anntzer commentedApr 12, 2020

Uh oh!

timhoffm commentedApr 13, 2020

Uh oh!

tacaswellApr 13, 2020

Choose a reason for hiding this comment

Uh oh!

anntzerApr 13, 2020

Choose a reason for hiding this comment

Uh oh!

timhoffmApr 19, 2020• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

timhoffm commentedApr 19, 2020• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Uh oh!

anntzer commentedApr 19, 2020

Uh oh!

timhoffm commentedApr 19, 2020• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Uh oh!

anntzer commentedApr 19, 2020

Uh oh!

timhoffm left a comment• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

timhoffm left a comment• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anntzer commentedApr 20, 2020

Uh oh!

Uh oh!

timhoffm commentedApr 12, 2020•
edited
Loading

timhoffmApr 19, 2020•
edited
Loading

timhoffm commentedApr 19, 2020•
edited
Loading

timhoffm commentedApr 19, 2020•
edited
Loading

timhoffm left a comment•
edited
Loading

timhoffm left a comment•
edited
Loading