Fixed couple of issues with copy/paste a mask #8728
Conversation
WalkthroughThe changes introduced in this pull request focus on fixing issues related to the copy and paste functionality of masks within a video annotation application. The updates ensure that attributes associated with masks are correctly copied during paste operations. Additionally, modifications were made to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Canvas
participant MasksHandler
participant State
User->>Canvas: Draw Mask
Canvas->>MasksHandler: onDrawDone
MasksHandler->>State: Update with attributes (occluded, color, etc.)
State-->>Canvas: Reflect changes
User->>Canvas: Copy Mask
Canvas->>MasksHandler: Copy attributes
MasksHandler-->>Canvas: Paste Mask with attributes
Canvas->>User: Display pasted Mask
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 (
|
|
Please retry analysis of this Pull-Request directly on SonarQube Cloud |
|
![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
🧹 Outside diff range and nitpick comments (2)
cvat-canvas/src/typescript/masksHandler.ts (1)
407-410: LGTM! Consider adding type safety.The changes correctly implement the copying of mask properties during paste operations, ensuring that attributes, occluded state, color, and object type are preserved from the initial state.
Consider adding type safety by explicitly defining the interface for
initialState:interface MaskInitialState { occluded: boolean; attributes: Record<string, unknown>; color: string; objectType: string; label: unknown; }This would help catch potential type-related issues early in development.
cvat-ui/src/components/annotation-page/canvas/views/canvas2d/canvas-wrapper.tsx (1)
656-657: LGTM! Consider improving readability.The logic for assigning objectType is correct - forcing ObjectType.SHAPE for masks while falling back to activeObjectType for other shapes.
Consider improving readability by extracting the condition into a more descriptive variable:
-state.objectType = state.shapeType === ShapeType.MASK ? - ObjectType.SHAPE : state.objectType ?? activeObjectType; +const isMaskShape = state.shapeType === ShapeType.MASK; +state.objectType = isMaskShape ? ObjectType.SHAPE : state.objectType ?? activeObjectType;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
changelog.d/20241121_013447_sekachev.bs.md(1 hunks)changelog.d/20241121_013934_sekachev.bs.md(1 hunks)cvat-canvas/src/typescript/masksHandler.ts(1 hunks)cvat-ui/src/components/annotation-page/canvas/views/canvas2d/canvas-wrapper.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- changelog.d/20241121_013447_sekachev.bs.md
- changelog.d/20241121_013934_sekachev.bs.md
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #8728 +/- ##
========================================
Coverage 74.18% 74.18%
========================================
Files 401 401
Lines 43510 43513 +3
Branches 3950 3953 +3
========================================
+ Hits 32278 32281 +3
Misses 11232 11232
|
Motivation and context
How has this been tested?
Checklist
developbranch(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
Improvements