Fixed reset bug with issue tool
#8236
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe recent changes enhance the functionality of the issue tool in the annotation application by addressing a bug that left the tool active after switching modes. Modifications include resetting the cursor state in the canvas interaction, refining the control flow in the canvas wrapper, and introducing a new property for creating issues in the dialog. These updates collectively improve user experience and ensure better responsiveness during the annotation process. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CanvasWrapper
participant IssueDialog
participant IssueAggregator
User->>CanvasWrapper: Select position
CanvasWrapper-->>User: No reset action
User->>IssueDialog: Create issue
IssueDialog->>IssueAggregator: Trigger onCreateIssue
IssueAggregator-->>CanvasWrapper: Update active control
Note right of User: Issue created successfully
Assessment against linked issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- changelog.d/20240731_121418_klakhov_fix_issue_tool.md (1 hunks)
- cvat-canvas/src/typescript/canvasView.ts (1 hunks)
- cvat-ui/src/components/annotation-page/canvas/views/canvas2d/canvas-wrapper.tsx (1 hunks)
- cvat-ui/src/components/annotation-page/review/create-issue-dialog.tsx (2 hunks)
- cvat-ui/src/components/annotation-page/review/issues-aggregator.tsx (3 hunks)
Files skipped from review due to trivial changes (1)
- changelog.d/20240731_121418_klakhov_fix_issue_tool.md
Additional comments not posted (6)
cvat-ui/src/components/annotation-page/review/create-issue-dialog.tsx (2)
80-80: LGTM!The
onCreateIssueproperty is correctly typed as a function returning void.
Line range hint
88-104: LGTM! But verify the function usage in the codebase.The
onCreateIssueproperty is correctly used in the component logic.However, ensure that the
onCreateIssueproperty is correctly passed and utilized throughout the codebase.Verification successful
LGTM! But verify the function usage in the codebase.
The
onCreateIssueproperty is correctly used in the component logic and passed down as a prop. The verification confirms its proper usage in the codebase.
cvat-ui/src/components/annotation-page/review/issues-aggregator.tsxcvat-ui/src/components/annotation-page/review/create-issue-dialog.tsxScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the `onCreateIssue` property in the codebase. # Test: Search for the usage of `onCreateIssue` property. Expect: Proper usage in all instances. rg --type typescript -A 5 $'onCreateIssue'Length of output: 76
Script:
#!/bin/bash # Description: Verify the usage of the `onCreateIssue` property in the codebase. # Test: Search for the usage of `onCreateIssue` property. Expect: Proper usage in all instances. rg --type ts -A 5 $'onCreateIssue'Length of output: 3036
cvat-ui/src/components/annotation-page/review/issues-aggregator.tsx (2)
18-18: LGTM!The
updateActiveControlfunction is correctly imported fromactions/annotation-actions.
91-96: LGTM! But verify the function usage in the codebase.The
onCreateIssuefunction is correctly implemented usinguseCallbackand passed as a prop.However, ensure that the
onCreateIssuefunction is correctly utilized throughout the codebase.Verification successful
Let's correct the file type and re-run the search to verify the usage of the
onCreateIssuefunction in the codebase.
The
onCreateIssuefunction is correctly utilized throughout the codebase.The function is defined and used in
cvat-ui/src/components/annotation-page/review/issues-aggregator.tsxand is also invoked incvat-ui/src/components/annotation-page/review/create-issue-dialog.tsx.
issues-aggregator.tsx: The function is defined and passed as a prop.create-issue-dialog.tsx: The function is invoked as expected.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the `onCreateIssue` function in the codebase. # Test: Search for the usage of `onCreateIssue` function. Expect: Proper usage in all instances. rg --type typescript -A 5 $'onCreateIssue'Length of output: 76
Script:
#!/bin/bash # Description: Verify the usage of the `onCreateIssue` function in the codebase. # Test: Search for the usage of `onCreateIssue` function in .ts and .tsx files. Expect: Proper usage in all instances. rg --type-add 'ts:*.{ts,tsx}' -t ts -A 5 'onCreateIssue'Length of output: 3059
cvat-ui/src/components/annotation-page/canvas/views/canvas2d/canvas-wrapper.tsx (1)
729-729: LGTM! But verify the method's functionality.The
onCanvasPositionSelectedmethod is correctly modified to remove the call toonResetCanvas().However, ensure that the method functions correctly without the call to
onResetCanvas().Verification successful
Let's correct the
rgcommand and search for theonCanvasPositionSelectedmethod definition and its usage across the codebase.
Let's correct the
rgcommand to search for the usage of theonCanvasPositionSelectedmethod in TypeScript and JSX files.
Let's correct the
rgcommand to search for the usage of theonCanvasPositionSelectedmethod in TypeScript and JavaScript files.
Verification Successful
The
onCanvasPositionSelectedmethod is correctly modified to remove the call toonResetCanvas(). The method's functionality is localized to thecanvas-wrapper.tsxfile, and no issues were found with its usage.
cvat-ui/src/components/annotation-page/canvas/views/canvas2d/canvas-wrapper.tsxScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the functionality of the `onCanvasPositionSelected` method in the codebase. # Test: Search for the usage of `onCanvasPositionSelected` method. Expect: Proper functionality in all instances. rg --type typescript -A 5 $'onCanvasPositionSelected'Length of output: 87
Script:
#!/bin/bash # Description: Verify the functionality of the `onCanvasPositionSelected` method in the codebase. # Test: Search for the definition of `onCanvasPositionSelected` method. rg 'onCanvasPositionSelected' -t ts -A 5 # Test: Search for the usage of `onCanvasPositionSelected` method. rg 'onCanvasPositionSelected' -t tsx -A 5Length of output: 2565
Script:
#!/bin/bash # Description: Verify the functionality of the `onCanvasPositionSelected` method in the codebase. # Test: Search for the usage of `onCanvasPositionSelected` method in TypeScript files. rg 'onCanvasPositionSelected' -t ts -A 5 # Test: Search for the usage of `onCanvasPositionSelected` method in JSX files. rg 'onCanvasPositionSelected' -t jsx -A 5Length of output: 2565
Script:
#!/bin/bash # Description: Verify the functionality of the `onCanvasPositionSelected` method in the codebase. # Test: Search for the usage of `onCanvasPositionSelected` method in TypeScript files. rg 'onCanvasPositionSelected' -t ts -A 5 # Test: Search for the usage of `onCanvasPositionSelected` method in JavaScript files. rg 'onCanvasPositionSelected' -t js -A 5Length of output: 2536
cvat-canvas/src/typescript/canvasView.ts (1)
1878-1878: LGTM! Verify the impact of the cursor reset.The change to reset the cursor style when
regionSelector.select(false)is called is approved.However, ensure that this change does not introduce any unintended side effects in other parts of the application.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #8236 +/- ##
===========================================
- Coverage 83.38% 83.37% -0.02%
===========================================
Files 389 389
Lines 41526 41524 -2
Branches 3856 3857 +1
===========================================
- Hits 34626 34619 -7
- Misses 6900 6905 +5
|
|
Motivation and context
Resolved #8147
There are two problems:
And small enhancement:
How has this been tested?
Checklist
developbranch[ ] I have updated the documentation accordingly(cvat-canvas,
cvat-core,
cvat-data and
cvat-ui)
License
Feel free to contact the maintainers if that's a concern.
Summary by CodeRabbit
Bug Fixes
New Features
CreateIssueDialogto trigger external actions upon successful issue creation.User Interface Enhancements