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

Fix disabling of groups (#709) #740

Merged
merged 1 commit into from
Apr 29, 2014

Conversation

RainCT
Copy link
Contributor

@RainCT RainCT commented Mar 13, 2014

This was broken with commit 5897285, which reverted the changes from
commit c6dacb1, but rather than only removing the change concerning
the read-only attribute, commented out the entire check, including
the parent_->getDisableChildren() call (which existed prior to
commit 5897285).

@wjwwood
Copy link
Member

wjwwood commented Mar 13, 2014

@dgossow can you comment on this?

This was broken with commit 5897285, which reverted the changes in
commit c6dacb1, but rather than only removing the change concerning
the read-only attribute, commented out the entire check, including
the parent_->getDisableChildren() call (which existed prior to
commit 5897285).
@RainCT
Copy link
Contributor Author

RainCT commented Apr 29, 2014

Any update on this?

@wjwwood
Copy link
Member

wjwwood commented Apr 29, 2014

I'm not willing to merge this until I get positive feedback from @dgossow or @hersh, or until I look into myself. I don't have time right now, but I'll try to squeeze in some time this afternoon, but since this is touching code recently changed by @dgossow I was hoping he could just glance at it and give it a 👍 or 👎.

@hersh
Copy link
Contributor

hersh commented Apr 29, 2014

I'm looking at this now...

@hersh
Copy link
Contributor

hersh commented Apr 29, 2014

I just tested rviz with and without this change. I see the need for it, as the current hydro-devel version does not disable the children of a DisplayGroup when you disable the group. Makes the groups feature pretty lame.

This change fixes that and doesn't seem to hurt the Views list behavior, so I'm giving this a +1.

+1.

@wjwwood
Copy link
Member

wjwwood commented Apr 29, 2014

Sounds good to me.

wjwwood added a commit that referenced this pull request Apr 29, 2014
@wjwwood wjwwood merged commit 8ed2809 into ros-visualization:indigo-devel Apr 29, 2014
@wjwwood
Copy link
Member

wjwwood commented Apr 29, 2014

I'll make sure to have a reasonable soak time for the next release of rviz.

@efernandez
Copy link

@wjwwood Could this be backported to hydro-devel?

This was referenced May 14, 2014
@wjwwood
Copy link
Member

wjwwood commented May 14, 2014

#769

wjwwood added a commit that referenced this pull request May 14, 2014
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.

4 participants