-
Notifications
You must be signed in to change notification settings - Fork 466
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
Add scale and color properties to Marker selection #1436
Add scale and color properties to Marker selection #1436
Conversation
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.
Thanks for your contribution and the quick adaption to my comments.
I have pushed some more changes to your PR branch. See the following comments.
@@ -85,53 +85,49 @@ QColor MarkerSelectionHandler::getColor() | |||
(int)(marker_->getMessage()->color.a * 255) ); | |||
} | |||
|
|||
QString MarkerSelectionHandler::getMarkerTypeName() |
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.
I moved getMarkerTypeName
to marker_utils.cpp
.
} | ||
|
||
void MarkerSelectionHandler::createProperties( const Picked& obj, Property* parent_property ) | ||
{ | ||
Property* group = new Property( "Marker " + marker_id_ + " (" + getMarkerTypeName() + ")", QVariant(), "", parent_property ); |
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.
Here, my intention was to use the type name in the value field. Minor change.
@@ -140,7 +136,7 @@ void MarkerSelectionHandler::createProperties( const Picked& obj, Property* pare | |||
orientation_property_ = new QuaternionProperty( "Orientation", getOrientation(), "", group ); | |||
orientation_property_->setReadOnly( true ); | |||
|
|||
scale_property_ = new MarkerScaleProperty( "Scale", getScale(), marker_->getMessage()->type, "", group ); |
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.
There is no need to duplicate code for a new MarkerScaleProperty
, which actually behaves like a VectorProperty.
It's possible to rename (and hide) the fields.
This PR addresses issue #1380 and adds some extra functionality to the marker selection pane.
This PR started from #1435 and was intended to target
melodic-devel
, but it wasn't possible to make the necessary changes and not break the ABI. Therefore, this PR targetsnoetic-devel
instead. Here is an updated screenshot with some changes made since the initial PR was opened:Before modifications:
After modifications: