-
Notifications
You must be signed in to change notification settings - Fork 68
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
Toolbar layout fixes #1273
Conversation
@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) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
LGTM, tests pass. |
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.