This repository has been archived by the owner on Apr 18, 2024. It is now read-only.
fix: LSDV-5248: BBoxes optimizations #1523
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
# Conflicts: # src/tags/control/TextArea/TextArea.js
Codecov ReportPatch coverage is
📢 Thoughts on this report? Let us know!. |
yyassi-heartex
approved these changes
Aug 29, 2023
hlomzik
suggested changes
Aug 30, 2023
src/regions/AliveRegion.tsx
Outdated
| @@ -28,13 +38,20 @@ export const AliveRegion = ( | |||
|
|
|||
| return observer(({ item, ...rest }: RegionComponentProps) => { | |||
| const canRender = options?.renderHidden || !item.hidden; | |||
| const shouldNotUsePortal = isFF(FF_DBLCLICK_DELAY) || options?.shouldNotUsePortal; | |||
There was a problem hiding this comment.
options.shouldNotUsePortal is set to true only in one region (Brush) and the value is isFF(FF_DBLCLICK_DELAY), so it can be just removed.
hlomzik
suggested changes
Aug 30, 2023
| @@ -388,7 +392,10 @@ const SelectionLayer = observer(({ item, selectionArea }) => { | |||
| const Selection = observer(({ item, selectionArea, ...triggeredOnResize }) => { | |||
| return ( | |||
| <> | |||
| <SelectedRegions item={item} selectedRegions={item.selectedRegions} {...triggeredOnResize} /> | |||
| { !isFF(FF_DBLCLICK_DELAY) | |||
There was a problem hiding this comment.
reverse condition for readability, please
| if ('ruler' === el?.attrs?.name) { | ||
| return true; | ||
| } | ||
| if ((!isFF(FF_DBLCLICK_DELAY) || !isMoveTool) |
There was a problem hiding this comment.
please, rewrite this! extract lambda into separate named function, add comment, simplify condition
src/mixins/KonvaRegion.js
Outdated
| @@ -35,7 +34,7 @@ export const KonvaRegionMixin = types.model({}) | |||
| }; | |||
| }) | |||
| .actions(self => { | |||
| let deferredSelectId = null; | |||
| let deferredSelectId; | |||
There was a problem hiding this comment.
are there some importance in this change from null to undefined?
There was a problem hiding this comment.
Nope, it's just an accident
hlomzik
approved these changes
Sep 1, 2023
…bs/label-studio-frontend into fb-lsdv-5248/bbox-optimization
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously, we used the deferred action call approach for double-clicking on a region. It was caused by the fact that there was no way to catch dblclick interaction 'cause re-rendering elements on selection prevents emiting events.
Right now this problem should be solved by using
Portalelement ofKonva. As long as the elements are still the same we can catch second click on them so we do not need ta wait anymore.PR fulfills these requirements
[fix|feat|ci|chore|doc]: TICKET-ID: Short description of change madeex.fix: DEV-XXXX: Removed inconsistent code usage causing intermittent errorsChange has impacts in these area(s)
(check all that apply)
What libraries were added/updated?
N/A
Does this change affect performance?
Nope
Does this change affect security?
Nope
What alternative approaches were there?
(briefly list any if applicable)
What feature flags were used to cover this change?
fflag_fix_front_lsdv_5248_double_click_delay_280823_shortDoes this PR introduce a breaking change?
(check only one)
What level of testing was included in the change?
(check all that apply)
Which logical domain(s) does this change affect?
Image Segmentation