fix: BROS-482: Hiding and locking #8853
Conversation
✅ Deploy Preview for label-studio-docs-new-theme canceled.
|
✅ Deploy Preview for label-studio-playground canceled.
|
✅ Deploy Preview for label-studio-storybook canceled.
|
✅ Deploy Preview for heartex-docs canceled.
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #8853 +/- ##
============================================
+ Coverage 37.94% 57.70% +19.75%
============================================
Files 735 554 -181
Lines 57849 40332 -17517
Branches 9403 10810 +1407
============================================
+ Hits 21953 23273 +1320
+ Misses 35892 17055 -18837
Partials 4 4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| <> | ||
| {/* White outline ring for selected points - rendered outside the colored stroke */} | ||
| {!disabled && isSelected && ( | ||
| {selected && isSelected && ( |
There was a problem hiding this comment.
selected AND isSelected? feels a little awkward
There was a problem hiding this comment.
+ isExplicitlySelected + isMultiSelection + selectedPoints
can it all be simplified? feels that we went too deep
| } = currentValuesRef.current; | ||
|
|
||
| if (disabled || isDragging.current || isDraggingNewBezier || ghostPointDragInfo?.isDragging) { | ||
| if (disabled || !selected || isDragging.current || isDraggingNewBezier || ghostPointDragInfo?.isDragging) { |
There was a problem hiding this comment.
you removed disabled from props but still use it in this condition. is it correct?
There was a problem hiding this comment.
i didn't remove it. it was renamed and re-introduced
original disabled was renamed to selected to better reflect what it does. newly added disabled is specifically for disabling user interactions
There was a problem hiding this comment.
you removed it couple of lines earlier in var destructuring and replaced with selected. so apparently it's used from some code above, which makes it not obvious plus I believe it's missing from useEffect dependencies.
I'd suggest to return it back to this function.
Co-authored-by: Andrew <[email protected]>
hlomzik
left a comment
hlomzik
left a comment
There was a problem hiding this comment.
in general code looks like a good fit (after fixing issue with missing disabled var)
|
/git merge
|
|
/git merge
|
This PR fixes inability to lock or hide Vector regions.
disabledprop was renamed toselectedand inverted logic to clearly reflect what it doesdisabledprop that will actually disable all shape interactions