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 layout fixes #1273

Merged
merged 10 commits into from May 6, 2015
Merged

Conversation

@chaosphere2112
Copy link
Contributor

@chaosphere2112 chaosphere2112 commented May 5, 2015

Fixes a number of minor layout/interaction issues with the toolbar, and adds tests that cover some of the basics (as well as some of the specific issues I encountered).

Ctests are passing for me, but I'm waiting on the upload (my home laptop does not love concurrent testing, so it'll be a bit).

Matching baselines coming shortly.

@chaosphere2112
Copy link
Contributor Author

@chaosphere2112 chaosphere2112 commented May 5, 2015

@chaosphere2112
Copy link
Contributor Author

@chaosphere2112 chaosphere2112 commented May 5, 2015

@aashish24 @doutriaux1 @dlonie Here are the changes I mentioned in the Syncup email.

# This makes them show up correctly. Weird, but it works.
h_state = self.repr.GetHighlightState()
self.repr.Highlight((h_state + 1) % 3)
self.repr.Highlight(h_state)
Copy link
Contributor

@allisonvacanti allisonvacanti May 6, 2015

Nice comment :P

Strange. Looking at the C++, all this does is trigger a HighlightEvent (which, according to git grep doesn't seem to be handled anywhere internally by VTK, nor by uvcdat/vistrails), and calls Modified(), which you do above. Strange.

Copy link
Contributor Author

@chaosphere2112 chaosphere2112 May 6, 2015

Yeah, you're telling me. I actually have a tendency to delve down into the C++ source when I find a weird hack like this, so I can ideally not have the weird hack anymore. This time I failed.

@allisonvacanti
Copy link
Contributor

@allisonvacanti allisonvacanti commented May 6, 2015

LGTM, tests pass.

allisonvacanti pushed a commit that referenced this issue May 6, 2015
@allisonvacanti allisonvacanti merged commit 5d7882e into CDAT:release May 6, 2015
1 check passed
@aashish24 aashish24 mentioned this pull request May 22, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants