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

Toolbar tracks views if axes are added during use #3347

Closed
wants to merge 1 commit into from

Conversation

KevKeating
Copy link
Contributor

This is an initial implementation of a fix for #2511. This pull request should not be merged. Once MEP22 is merged, I'll port this code to backend_tools.ViewsPositionsMixin. (The porting should be fairly trivial.) I finished the implemented of this recently for in-house use, so I figured I'd put up a PR against master in case anyone had any initial comments.

With this PR, the back, forward, and home buttons in the toolbar will continue to work even if the number of axes changes. I've added a _home_views dictionary that stores the initial views of all axes. If an axes object didn't exist when a view was created, the initial view will be used for that axes. Positions will only be restored if there is a position for all axes objects. This avoids having newer axes overlap with older ones when clicking on back or home.

I haven't disconnected toolbar.update() as an axobserver in this PR since that will be hooked up slightly differently once MEP22 is merged.

@tacaswell tacaswell added this to the v1.5.x milestone Aug 17, 2014
@fariza
Copy link
Member

fariza commented Sep 8, 2014

@tacaswell what do you think:
Would it be a good idea if @KevKeating submit a PR agains my branch of MEP22?
Or do you think it is too close to be merged that any change would delay things?

@tacaswell
Copy link
Member

I have not had the bandwidth to look at any of this. I would trust your judgement.

@fariza
Copy link
Member

fariza commented Sep 9, 2014

@KevKeating if you want, you can submit a PR against my MEP22 branch

@KevKeating
Copy link
Contributor Author

Sorry for the delayed response. I'll put together a PR against MEP22 when I get a chance. It should be a fairly trivial port, so it shouldn't take long. I can always rebase the port against master if it turns out that I'd delay the MEP22 merge.

@tacaswell tacaswell modified the milestones: proposed next point release, next point release Feb 19, 2015
@tacaswell tacaswell modified the milestones: proposed next point release, next point release Jun 23, 2015
@KevKeating
Copy link
Contributor Author

I've just posted #4857, which combines this PR with fariza#5, so it updates both the navtoolbar2 toolbars as well as the new toolmanager toolbars. Assuming no one objects to updating both toolbar types in one PR, I'll close this PR in favor of #4857.

@tacaswell
Copy link
Member

Sounds good

On Mon, Aug 3, 2015, 12:45 PM Kevin Keating notifications@github.com
wrote:

I've just posted #4857
#4857, which combines this
PR with fariza#5 fariza#5,
so it updates both the navtoolbar2 toolbars as well as the new toolmanager
toolbars. Assuming no one objects to updating both toolbar types in one PR,
I'll close this PR in favor of #4857
#4857.


Reply to this email directly or view it on GitHub
#3347 (comment)
.

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.

None yet

3 participants