-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Fixed tooltips with undefined shortcuts #8578
Conversation
WalkthroughThis update addresses multiple modifications across various components in the application. It resolves an issue with undefined shortcuts in tooltips and introduces changes to shortcut key mappings for drawing and object removal functionalities. Notably, the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AnnotationTopBar
participant DrawShapePopover
participant SetupTagPopover
participant ObjectItem
User->>AnnotationTopBar: Search Annotations
AnnotationTopBar->>AnnotationTopBar: onSearchAnnotations()
User->>DrawShapePopover: Draw Shape (Shortcut)
DrawShapePopover->>DrawShapePopover: Handle SWITCH_DRAW_MODE_STANDARD_CONTROLS
User->>SetupTagPopover: Setup Tag (Shortcut)
SetupTagPopover->>SetupTagPopover: Handle SWITCH_DRAW_MODE_STANDARD_CONTROLS
User->>ObjectItem: Delete Object (Shortcut)
ObjectItem->>ObjectItem: Handle DELETE_OBJECT_STANDARD_WORKSPACE
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
cvat-ui/src/containers/annotation-page/top-bar/top-bar.tsx (1)
680-680
: New onSearchAnnotations prop addedA new prop
onSearchAnnotations
has been added to theAnnotationTopBarComponent
. This suggests the introduction of a new feature for searching annotations.Consider adding a comment or documentation to explain the purpose and expected behavior of this new search functionality. This will help other developers understand how to use this feature and its impact on the component.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
- changelog.d/20241022_121246_sekachev.bs_fixed_undefined_shortcuts.md (1 hunks)
- cvat-ui/src/components/annotation-page/standard-workspace/objects-side-bar/object-item.tsx (1 hunks)
- cvat-ui/src/containers/annotation-page/standard-workspace/controls-side-bar/draw-shape-popover.tsx (1 hunks)
- cvat-ui/src/containers/annotation-page/standard-workspace/controls-side-bar/setup-tag-popover.tsx (1 hunks)
- cvat-ui/src/containers/annotation-page/top-bar/top-bar.tsx (2 hunks)
✅ Files skipped from review due to trivial changes (2)
- changelog.d/20241022_121246_sekachev.bs_fixed_undefined_shortcuts.md
- cvat-ui/src/containers/annotation-page/standard-workspace/controls-side-bar/draw-shape-popover.tsx
🧰 Additional context used
🔇 Additional comments (4)
cvat-ui/src/containers/annotation-page/standard-workspace/controls-side-bar/setup-tag-popover.tsx (1)
157-157
: LGTM! Verify consistency across the codebase.The change from
SWITCH_DRAW_MODE
toSWITCH_DRAW_MODE_STANDARD_CONTROLS
aligns with the PR objective of fixing undefined shortcuts in tooltips. This update is consistent with similar changes in other components, as mentioned in the AI-generated summary.To ensure consistency across the codebase, run the following script:
This script will help ensure that:
- There are no remaining instances of the old
SWITCH_DRAW_MODE
(without the_STANDARD_CONTROLS
suffix).- The new
SWITCH_DRAW_MODE_STANDARD_CONTROLS
is used consistently.- There are no lingering TODO comments related to this change.
Please review the results and make any necessary adjustments to ensure consistency across the codebase.
✅ Verification successful
Re-running verification scripts to ensure consistency across the codebase.
Please execute the following updated scripts:
Verification successful:
SWITCH_DRAW_MODE_STANDARD_CONTROLS
is consistently used across the codebase, and no instances ofSWITCH_DRAW_MODE
or related TODO comments were found.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of SWITCH_DRAW_MODE_STANDARD_CONTROLS across the codebase # Test 1: Check for any remaining usage of SWITCH_DRAW_MODE echo "Checking for any remaining usage of SWITCH_DRAW_MODE:" rg "SWITCH_DRAW_MODE[^_]" --type ts --type tsx # Test 2: Verify the usage of SWITCH_DRAW_MODE_STANDARD_CONTROLS echo "Verifying the usage of SWITCH_DRAW_MODE_STANDARD_CONTROLS:" rg "SWITCH_DRAW_MODE_STANDARD_CONTROLS" --type ts --type tsx # Test 3: Check for any TODO comments related to this change echo "Checking for any TODO comments related to this change:" rg "TODO.*SWITCH_DRAW_MODE" --type ts --type tsxLength of output: 607
Script:
#!/bin/bash # Description: Verify the usage of SWITCH_DRAW_MODE_STANDARD_CONTROLS across the codebase # Test 1: Check for any remaining usage of SWITCH_DRAW_MODE echo "Checking for any remaining usage of SWITCH_DRAW_MODE:" rg "SWITCH_DRAW_MODE[^_]" --glob "*.ts" --glob "*.tsx" # Test 2: Verify the usage of SWITCH_DRAW_MODE_STANDARD_CONTROLS echo "Verifying the usage of SWITCH_DRAW_MODE_STANDARD_CONTROLS:" rg "SWITCH_DRAW_MODE_STANDARD_CONTROLS" --glob "*.ts" --glob "*.tsx" # Test 3: Check for any TODO comments related to this change echo "Checking for any TODO comments related to this change:" rg "TODO.*SWITCH_DRAW_MODE" --glob "*.ts" --glob "*.tsx"Length of output: 1567
cvat-ui/src/components/annotation-page/standard-workspace/objects-side-bar/object-item.tsx (1)
121-121
: LGTM. Verify consistency and update documentation if needed.The change from
normalizedKeyMap.DELETE_OBJECT
tonormalizedKeyMap.DELETE_OBJECT_STANDARD_WORKSPACE
looks good and aligns with the PR objectives of fixing undefined shortcuts.To ensure consistency across the codebase, please run the following script:
Consider updating the documentation to reflect this change in shortcut naming if necessary.
✅ Verification successful
Verification Successful
All instances of
DELETE_OBJECT
have been successfully replaced withDELETE_OBJECT_STANDARD_WORKSPACE
. No remaining occurrences found.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining instances of DELETE_OBJECT and compare with DELETE_OBJECT_STANDARD_WORKSPACE echo "Occurrences of DELETE_OBJECT:" rg "DELETE_OBJECT[^_]" --type ts --type tsx echo "\nOccurrences of DELETE_OBJECT_STANDARD_WORKSPACE:" rg "DELETE_OBJECT_STANDARD_WORKSPACE" --type ts --type tsxLength of output: 340
Script:
#!/bin/bash # Description: Check for any remaining instances of DELETE_OBJECT and compare with DELETE_OBJECT_STANDARD_WORKSPACE in .ts and .tsx files echo "Occurrences of DELETE_OBJECT:" rg "DELETE_OBJECT[^_]" -g "*.ts" -g "*.tsx" echo "\nOccurrences of DELETE_OBJECT_STANDARD_WORKSPACE:" rg "DELETE_OBJECT_STANDARD_WORKSPACE" -g "*.ts" -g "*.tsx"Length of output: 772
cvat-ui/src/containers/annotation-page/top-bar/top-bar.tsx (2)
Line range hint
1-720
: Summary of changesThe modifications in this file include:
- Updating the
drawShortcut
prop to use a new keyboard mapping.- Adding a new
onSearchAnnotations
prop for search functionality.- An incomplete line of code that needs attention.
These changes suggest improvements to the user interface and the addition of new features. The updated keyboard shortcut may provide a more standardized control scheme, while the new search functionality could enhance the user's ability to navigate and find specific annotations.
To ensure the effectiveness of these changes:
- Complete or remove the incomplete line of code.
- Add documentation for the new search functionality.
- Verify that the keyboard shortcut change is consistent across the codebase.
- Consider adding tests for the new search feature to ensure its reliability.
701-701
: Update to drawShortcut propThe
drawShortcut
prop has been updated to usenormalizedKeyMap.SWITCH_DRAW_MODE_STANDARD_CONTROLS
instead ofnormalizedKeyMap.SWITCH_DRAW_MODE
. This change suggests a modification in the keyboard shortcut mapping for the draw mode.To ensure this change is consistent across the codebase, please run the following script:
✅ Verification successful
Verification Successful
The
drawShortcut
prop has been consistently updated across the codebase to usenormalizedKeyMap.SWITCH_DRAW_MODE_STANDARD_CONTROLS
. No remaining instances ofSWITCH_DRAW_MODE
were found.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining usage of SWITCH_DRAW_MODE and compare with SWITCH_DRAW_MODE_STANDARD_CONTROLS echo "Occurrences of SWITCH_DRAW_MODE:" rg "SWITCH_DRAW_MODE[^_]" --type ts --type tsx echo "\nOccurrences of SWITCH_DRAW_MODE_STANDARD_CONTROLS:" rg "SWITCH_DRAW_MODE_STANDARD_CONTROLS" --type ts --type tsxLength of output: 355
Script:
#!/bin/bash # Description: Check for any remaining usage of SWITCH_DRAW_MODE and compare with SWITCH_DRAW_MODE_STANDARD_CONTROLS echo "Occurrences of SWITCH_DRAW_MODE:" rg "SWITCH_DRAW_MODE[^_]" -g "*.ts" -g "*.tsx" echo "\nOccurrences of SWITCH_DRAW_MODE_STANDARD_CONTROLS:" rg "SWITCH_DRAW_MODE_STANDARD_CONTROLS" -g "*.ts" -g "*.tsx"Length of output: 1319
Quality Gate passedIssues Measures |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #8578 +/- ##
===========================================
+ Coverage 74.24% 74.28% +0.04%
===========================================
Files 403 403
Lines 43287 43287
Branches 3914 3914
===========================================
+ Hits 32137 32156 +19
+ Misses 11150 11131 -19
|
Motivation and context
How has this been tested?
Checklist
develop
branch(cvat-canvas,
cvat-core,
cvat-data and
cvat-ui)
License
Feel free to contact the maintainers if that's a concern.
Summary by CodeRabbit
New Features
Bug Fixes
Improvements