-
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
Do not adjust opacity level each frame if there are masks #8149
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 updates ensure that the opacity level for annotations, specifically those with masks, is maintained consistently across frame switches. This enhancement modifies opacity handling logic in both the front-end code and test scripts, ensuring a more robust and user-friendly experience. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI
participant Backend
participant Annotations
User->>UI: Requests frame switch
UI->>Annotations: Check annotationsIncludeMasks
alt Mask Present
Annotations->>UI: Maintain current opacity
else No Mask
Annotations->>UI: Reset opacity to default
end
UI->>Backend: Fetch new frame data
Backend->>UI: Return frame data
UI->>User: Display frame with appropriate opacity
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 Configration 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: 0
Outside diff range, codebase verification and nitpick comments (1)
cvat-ui/src/utils/clamp-opacity.ts (1)
Line range hint
8-25
:
Consider replacing magic numbers with named constants.The use of magic numbers (30 and 60) in the function can reduce readability. Consider defining these values as named constants for better clarity.
const ENHANCED_DEFAULT_OPACITY = 30; const ENHANCED_DEFAULT_SELECTED_OPACITY = 60;
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (7)
- changelog.d/20240711_094827_boris_do_not_reset_opacity.md (1 hunks)
- cvat-ui/src/actions/annotation-actions.ts (3 hunks)
- cvat-ui/src/reducers/settings-reducer.ts (2 hunks)
- cvat-ui/src/utils/clamp-opacity.ts (2 hunks)
- tests/cypress/e2e/actions_objects/regression_tests.js (1 hunks)
- tests/cypress/e2e/issues_prs/issue_7176_opacity_reset.js (5 hunks)
- tests/cypress/support/commands.js (1 hunks)
Files skipped from review due to trivial changes (2)
- changelog.d/20240711_094827_boris_do_not_reset_opacity.md
- tests/cypress/support/commands.js
Additional comments not posted (7)
tests/cypress/e2e/actions_objects/regression_tests.js (1)
47-47
: Verify the correctness ofheadlessCreateObjects
usage.Ensure that the
headlessCreateObjects
function is correctly implemented and used to create objects as intended.Verification successful
The
headlessCreateObjects
function is correctly implemented and used in the test setup. The function properly creates objects and ensures they are saved as intended.
tests/cypress/support/commands.js
: Implementation ofheadlessCreateObjects
.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation of `headlessCreateObjects`. # Test: Search for the function implementation. Expect: Correct logic for creating objects. rg --type javascript 'headlessCreateObjects'Length of output: 79
Script:
#!/bin/bash # Description: Verify the implementation of `headlessCreateObjects`. # Test: Search for the function implementation. Expect: Correct logic for creating objects. rg 'headlessCreateObjects'Length of output: 344
Script:
#!/bin/bash # Description: Extract the implementation of `headlessCreateObjects` from `tests/cypress/support/commands.js`. # Test: Extract function implementation. Expect: Correct logic for creating objects. rg -A 20 'Cypress.Commands.add\(\'headlessCreateObjects\'' tests/cypress/support/commands.jsLength of output: 93
Script:
#!/bin/bash # Description: Extract the implementation of `headlessCreateObjects` from `tests/cypress/support/commands.js`. # Test: Extract function implementation. Expect: Correct logic for creating objects. rg -A 20 "Cypress.Commands.add('headlessCreateObjects'" tests/cypress/support/commands.jsLength of output: 213
Script:
#!/bin/bash # Description: Extract the implementation of `headlessCreateObjects` from `tests/cypress/support/commands.js`. # Test: Extract function implementation. Expect: Correct logic for creating objects. rg -A 20 "Cypress.Commands.add\('headlessCreateObjects'" tests/cypress/support/commands.jsLength of output: 978
tests/cypress/e2e/issues_prs/issue_7176_opacity_reset.js (3)
94-96
: LGTM!The test case correctly checks if the annotation view opens with increased opacity levels when masks are present.
Line range hint
98-113
:
LGTM!The test cases correctly set lower opacity values and verify that they do not reset when switching frames.
65-72
: Verify the correctness ofheadlessCreateObjects
usage.Ensure that the
headlessCreateObjects
function is correctly implemented and used to create objects with a mask shape as intended.cvat-ui/src/reducers/settings-reducer.ts (1)
444-451
: LGTM!The changes correctly use the
clampOpacity
function with theannotationsIncludeMasks
parameter to clamp the opacity values.cvat-ui/src/actions/annotation-actions.ts (2)
982-982
: Verify the correct usage of theannotationsIncludeMasks
flag.The
annotationsIncludeMasks
flag has been added to the payload of theGET_JOB_SUCCESS
action. Ensure that this flag is correctly set and utilized in the application.Verification successful
The
annotationsIncludeMasks
flag is correctly set and utilized in the application.
- The flag is set based on the presence of mask shapes in
cvat-ui/src/actions/annotation-actions.ts
.- It is passed to the
clampOpacity
function incvat-ui/src/reducers/settings-reducer.ts
.- Within
clampOpacity
incvat-ui/src/utils/clamp-opacity.ts
, it is used to determine opacity values.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correct usage of the `annotationsIncludeMasks` flag in the codebase. # Test: Search for the usage of `annotationsIncludeMasks`. Expect: Correct usage in relevant files. rg --type typescript 'annotationsIncludeMasks'Length of output: 81
Script:
#!/bin/bash # Description: Verify the correct usage of the `annotationsIncludeMasks` flag in the codebase. # Test: Search for the usage of `annotationsIncludeMasks`. Expect: Correct usage in relevant files. rg --type ts 'annotationsIncludeMasks'Length of output: 556
Script:
#!/bin/bash # Description: Retrieve the context around the usage of `annotationsIncludeMasks` in the identified files. # Test: Get 5 lines of context around each match to understand the usage. rg --type ts 'annotationsIncludeMasks' -A 5 -B 5Length of output: 3847
941-941
: Verify the impact of removingshowAllInterpolationTracks
.The
showAllInterpolationTracks
setting has been removed from thegetJobAsync
function. Ensure that this removal does not affect other parts of the application that rely on this setting.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #8149 +/- ##
===========================================
- Coverage 83.39% 83.39% -0.01%
===========================================
Files 389 388 -1
Lines 41296 41272 -24
Branches 3858 3852 -6
===========================================
- Hits 34440 34418 -22
+ Misses 6856 6854 -2
|
Quality Gate passedIssues Measures |
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
Refactor
Tests