-
Notifications
You must be signed in to change notification settings - Fork 206
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
UIEditor : Add support for editing iconScale
node metadata
#6000
UIEditor : Add support for editing iconScale
node metadata
#6000
Conversation
c3bb2ef
to
1cf2f8b
Compare
python/GafferUI/MetadataWidget.py
Outdated
self.__numericWidget.valueChangedSignal().connect( | ||
Gaffer.WeakMethod( self.__editingFinished ), scoped = False | ||
) |
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's a bit of tension between the immediate updates we get from valueChangedSignal()
vs the many many undo steps we end up making for the virtual sliders and cursor-key increments. Might be worth spending half an hour or so to see if we can merge those undos into one, along the lines of what we do in NumericPlugValueWidget?
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 took a stab at the NumericPlugValueWidget style undo merging in 69d4316.
Should a reason of ValueChangedReason.InvalidEdit
result in an update using the current metadata value? Currently it is using the default value for the widget for lack of a better value.
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 think using the default value is reasonable enough given how much of a corner case this is. We're not guaranteed to have a current value, but I suppose if we do have one it might be considered more appropriate to revert to that than the default? I don't mind too much...
Thanks for the updates Murray! If you could squash and merge (with or without a tweak to the invalidedit thing) that would be grand... |
c58a54e
to
c413482
Compare
Thanks for the input! Squashed without tweaking the InvalidEdit thing and now merging. |
As mentioned in #5998, we were missing the ability to edit
iconScale
metadata from the UI Editor.