8000 Enabling tool positions by OceanWolf · Pull Request #7 · fariza/matplotlib · GitHub
[go: up one dir, main page]
Skip to content

Enabling tool positions #7

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

Closed
wants to merge 7 commits into from
Closed

Enabling tool positions #7

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
6238ceb
Got Tool positions working in the toolbar.
OceanWolf Feb 8, 2015
c9e55de
Update on removal
OceanWolf Feb 8, 2015
c286849
Moved the code around to make it easier to rewrite the groups code fo…
OceanWolf Feb 12, 2015
c643e53
Reimplemented seperators on GTK3 using new generic format.
OceanWolf Feb 13, 2015
9e70c13
Rewrote backend_gtk3 to use multiple toolbars.
OceanWolf Feb 14, 2015
a756229
Bug fix
OceanWolf Feb 15, 2015
043d32f
Implementation of OrderedDict for python 2.6
OceanWolf Feb 13, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 92 additions & 11 deletions lib/matplotlib/backend_bases.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,11 @@
import warnings
import time
import io
try:
from collections import OrderedDict
except:
# simple python 2.6 implementation, core-features only
from matplotlib.cbook import OrderedDict

import numpy as np
import matplotlib.cbook as cbook
Expand Down Expand Up @@ -3397,7 +3402,7 @@ def add_tools(self, tools):
for position, tool in enumerate(grouptools):
self.add_tool(tool[1], tool[0], group, position)

def add_tool(self, name, tool, group=None, position=None):
def add_tool(self, name, tool, group=None, position=-1):
"""Add tool to `NavigationBase`

Add a tool to the tools controlled by Navigation
Expand All @@ -3414,8 +3419,8 @@ def add_tool(self, name, tool, group=None, position=None):
Reference to find the class of the Tool to be added
group: String
Group to position the tool in
position : int or None (default)
Position within its group in the toolbar, if None, it goes at the end
position : int
Position within its group in the toolbar, if -1, it goes at the end
"""

tool_cls = self._get_cls_to_instantiate(tool)
Expand Down Expand Up @@ -3582,6 +3587,7 @@ class ToolbarBase(object):

def __init__(self, navigation):
self.navigation = navigation
self._groups = OrderedDict()

self.navigation.nav_connect('tool_message_event', self._message_cbk)
self.navigation.nav_connect('tool_added_event', self._add_tool_cbk)
Expand All @@ -3604,21 +3610,28 @@ def _tool_triggered_cbk(self, event):

def _add_tool_cbk(self, event):
"""Captures 'tool_added_event' and adds the tool to the toolbar"""
image = self._get_image_filename(event.tool.image)
tool, group, position = event.tool, event.data['group'], \
event.data['position']
if group is None:
return
Copy link
Owner

Choose a reason for hiding this comment

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

This should be done by backend, maybe you want to create a separate menu with all the tools which group is None

Copy link
Owner

Choose a reason for hiding this comment

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

yes you are right, I did change the in_toolbar attribute for this and make sense. But again, the toolbar should be informed of all tools and it can decide what to include or not. The group=None is just a suggestion (implemented in the default backends)

Copy link
Author

Choose a reason for hiding this comment

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

I agree with what you want to achieve, but I really disagree with implementing it like this. My next PR, the one that I have been talking about, will, in my opinion give a better implementation of this.

Perhaps I should push ahead and just release my next PR as a w-i-p so that you can see for yourself.

image = self._get_image_filename(tool.image)
toggle = getattr(event.tool, 'toggled', None) is not None
self.add_toolitem(event.tool.name,
event.data['group'],
event.data['position'],
image,
event.tool.description,
8000 toggle)

position, insert_index = self.get_positions(tool.name, group, position)
self.add_toolitem(tool.name, group, insert_index, image,
tool.description, toggle)
self.add_tool_to_group(tool, group, position)

if toggle:
self.navigation.nav_connect('tool_trigger_%s' % event.tool.name,
self.navigation.nav_connect('tool_trigger_%s' % tool.name,
self._tool_triggered_cbk)

def _remove_tool_cbk(self, event):
"""Captures the 'tool_removed_event' signal and removes the tool"""
self.remove_toolitem(event.tool.name)
for group in self._groups:
if event.tool.name in self._groups[group]:
self._groups[group].remove(event.tool.name)

def _get_image_filename(self, image):
"""Find the image based on its name"""
Expand All @@ -3642,6 +3655,74 @@ def trigger_tool(self, name):
"""
self.navigation.tool_trigger_event(name, sender=self)

def get_group_count(self, name):
"""Gets useful indicies for a group.

Parameters
----------
name : string
The name of the group.

Returns
-------
(start_index, group_pos) : where:
- `start_index, the index at the first item in the toolbar.
- `group_pos`, the index of this group within the toolbar.
"""
index = 0
group_count = 0
for group in self._groups:
if group == name:
break
index += len(self._groups[group])
group_count += 1
return index, group_count

def get_positions(self, name, group, position=-1):
"""Adds the tool to the group

Parameters
----------
name : string
The name of the tool.
group : string
The name of the group.
position : int
The position within the group, if -1, it goes at the end.

Returns
-------
(position, insert_index) : where:

- `position` a validated version of the input argument, i.e. > 0.
- `insert index` the position used to insert into the toolbar.
"""
Copy link
Author

Choose a reason for hiding this comment

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

Note to self, need to add some more documentation here, i.e. say it returns the position that this should get inserted into the toolbar.


if position < 0:
position += len(self._groups.get(group, [])) + 1
if position < 0:
position = 0

return position, self.get_group_count(group)[0] + position

def add_tool_to_group(self, tool, group, position=-1):
Copy link
Owner

Choose a reason for hiding this comment

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

I don't like this, you are forcing the toolbar and toolitems to be created at different stages, it might not be possible for some strage framework.
Again, this should be handled by the backend.

Copy link
Author

Choose a reason for hiding this comment

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

I did it like this so that one can either add a tool by overall position if the backend doesn't support a way to implement groups, thus the backend can add the tool in add_toolitem using absolute position, or by this method using group position.

I don't think a framework exists where this would not work for. I imagine you refer to one that does do groups, but doesn't return a toolitem, in which case the backend could just save the raw information from add_toolitem.

I guess I try and implement here a default for the most basic case, i.e. the no-toolbar-groups situation, so that we don't have to duplicate this code.

Copy link
Owner

Choose a reason for hiding this comment

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

The problem with the overall position, is that you have to decide if you include or not the None group,

""" Adds the tool to the group

Parameters
----------
tool : ToolBase
The tool.
group : string
The name of the group.
position: int
The position of the tool within the group.
"""

if not group in self._groups:
self._groups[group] = []

self._groups[group].insert(position, tool)

def add_toolitem(self, name, group, position, image, description, toggle):
"""Add a toolitem to the toolbar

Expand Down
40 changes: 23 additions & 17 deletions lib/matplotlib/backends/backend_gtk3.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -758,10 +758,10 @@ def __init__(self, navigation):
Gtk.Box.__init__(self)
self.set_property("orientation", Gtk.Orientation.VERTICAL)

self._toolbar = Gtk.Toolbar()
self._toolbar.set_style(Gtk.ToolbarStyle.ICONS)
self.pack_start(self._toolbar, False, False, 0)
self._toolbar.show_all()
self._toolarea = Gtk.Box()
self._toolarea.set_property('orientation', Gtk.Orientation.HORIZONTAL)
self.pack_start(self._toolarea, False, False, 0)
self._toolarea.show_all()
self._toolitems = {}
self._signals = {}
self._setup_message_area()
Expand All @@ -784,9 +784,6 @@ def _setup_message_area(self):

def add_toolitem(self, name, group, position, image_file, description,
toggle):
if group is None:
return

if toggle:
tbutton = Gtk.ToggleToolButton()
else:
Expand All @@ -798,16 +795,23 @@ def add_toolitem(self, name, group, position, image_file, description,
image.set_from_file(image_file)
tbutton.set_icon_widget(image)

if position is None:
position = -1
# TODO implement groups positions
self._toolbar.insert(tbutton, -1)
signal = tbutton.connect('clicked', self._call_tool, name)
tbutton.set_tooltip_text(description)
tbutton.show_all()
self._toolitems[name] = tbutton
self._signals[name] = signal

def add_tool_to_group(self, tool, group, position=-1):
if not group in self._groups:
if len(self._groups) != 0:
self.add_separator()
toolbar = Gtk.Toolbar()
toolbar.set_style(Gtk.ToolbarStyle.ICONS)
self._toolarea.pack_start(toolbar, False, False, 0)
toolbar.show_all()
self._groups[group] = toolbar
self._groups[group].insert(self._toolitems[tool.name], position)

def _call_tool(self, btn, name):
self.trigger_tool(name)

Expand All @@ -827,14 +831,16 @@ def remove_toolitem(self, name):
if name not in self._toolitems:
self.set_message('%s Not in toolbar' % name)
return
self._toolbar.remove(self._toolitems[name])
for group in self._groups:
if self._toolitems[name] in self._groups[group]:
self._groups[group].remove(self._toolitems[name])
del self._toolitems[name]

def add_separator(self, pos=-1):
toolitem = Gtk.SeparatorToolItem()
self._toolbar.insert(toolitem, pos)
toolitem.show()
return toolitem
def add_separator(self):
sep = Gtk.Separator()
sep.set_property("orientation", Gtk.Orientation.VERTICAL)
self._toolarea.pack_start(sep, False, True, 0)
sep.show_all()


class SaveFigureGTK3(SaveFigureBase):
Expand Down
Loading
0