Fixed "Back" button redirection #8277
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 to the CVAT UI enhance navigation functionality, particularly for the Changes
Assessment against linked issues
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 (4)
- cvat-ui/src/components/analytics-page/analytics-page.tsx (2 hunks)
- cvat-ui/src/components/common/go-back-button.tsx (1 hunks)
- cvat-ui/src/components/cvat-app.tsx (4 hunks)
- cvat-ui/src/utils/hooks.ts (1 hunks)
Additional comments not posted (11)
cvat-ui/src/components/common/go-back-button.tsx (2)
11-13: LGTM: Props interface is well-defined.The
Propsinterface correctly defines an optionalbackLinkproperty, enhancing the flexibility of theGoBackButtoncomponent.
15-18: LGTM: GoBackButton function changes.The
GoBackButtonfunction now accepts abackLinkprop, enhancing navigation flexibility. The use ofReact.memois appropriate for performance optimization.Ensure that the
GoBackButtonis used correctly across the codebase with the newbackLinkprop.cvat-ui/src/utils/hooks.ts (1)
65-70: LGTM: useGoBack function changes.The
useGoBackfunction now accepts abackLinkparameter, enhancing navigation flexibility. The use ofuseCallbackis appropriate for performance optimization.Consider verifying the handling of invalid
backLinkvalues to ensure robust navigation.cvat-ui/src/components/analytics-page/analytics-page.tsx (2)
82-84: LGTM: Props interface is well-defined.The
Propsinterface correctly defines an optionalbackLinkproperty, enhancing the flexibility of theAnalyticsPagecomponent.
86-87: LGTM: AnalyticsPage function changes.The
AnalyticsPagefunction now accepts abackLinkprop, enhancing navigation flexibility. The use ofReact.memois appropriate for performance optimization.Ensure that the
AnalyticsPageis used correctly across the codebase with the newbackLinkprop.Verification successful
Verified: Correct usage of
AnalyticsPagewithbackLinkpropThe
AnalyticsPagecomponent is used correctly with thebackLinkprop incvat-ui/src/components/cvat-app.tsx, ensuring navigation flexibility is implemented as intended.
- Location:
cvat-ui/src/components/cvat-app.tsxScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `AnalyticsPage` with the new `backLink` prop. # Test: Search for the usage of `AnalyticsPage`. Expect: Usage with the `backLink` prop. rg --type tsx -A 5 $'<AnalyticsPage'Length of output: 65
Script:
#!/bin/bash # Description: Verify the usage of `AnalyticsPage` with the new `backLink` prop. # Test: Search for the usage of `AnalyticsPage`. Expect: Usage with the `backLink` prop. rg -A 5 '<AnalyticsPage'Length of output: 2170
cvat-ui/src/components/cvat-app.tsx (6)
131-131: Addition ofgoBackLinkis appropriate.The introduction of
goBackLinkin theCVATAppStateinterface is a suitable enhancement for managing navigation state.
139-140: Initialization ofgoBackLinkis appropriate.The constructor correctly initializes
goBackLinktoundefined, which is consistent with its intended use.
521-524: Render logic forAnalyticsPageis appropriate.Passing
goBackLinkas a prop toAnalyticsPageensures dynamic back navigation, aligning with the PR objectives.
531-535: Render logic for task analytics is appropriate.The update to pass
goBackLinkas a prop ensures consistent navigation behavior across analytics pages.
542-546: Render logic for job analytics is appropriate.Passing
goBackLinkas a prop ensures consistent navigation behavior across all analytics pages.
178-182: Logic for settinggoBackLinkis appropriate.The
history.listenupdate correctly captures the previous pathname for analytics page navigation, enhancing the back navigation feature.Ensure that the condition logic accurately reflects the intended navigation paths.
Verification successful
Let's refine our approach to verify the condition logic for setting
goBackLinkby correctly identifying JavaScript files and searching for analytics routes.
Let's broaden our search to ensure we capture any analytics-related routes or components by looking for more general routing patterns and keywords.
Logic for setting
goBackLinkis consistent with analytics route handling.The search results confirm that analytics routes are managed within the application, as indicated by the regex pattern in
webpack.config.js. This supports the logic implemented incvat-app.tsxfor setting thegoBackLinkwhen navigating to an analytics page.
- Relevant file:
cvat-ui/webpack.config.jscontains a regex pattern that includesanalytics.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of the condition logic for setting `goBackLink`. # Test: Search for analytics page routes and ensure the logic is consistent. rg --type javascript 'path=.*analytics'Length of output: 76
Script:
#!/bin/bash # Find all JavaScript files and search for analytics routes fd -e js -e jsx | xargs rg 'path=.*analytics'Length of output: 47
Script:
#!/bin/bash # Search for any route definitions or analytics-related keywords in JavaScript files fd -e js -e jsx | xargs rg -i 'route|analytics'Length of output: 2831
klakhov
changed the title
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #8277 +/- ##
===========================================
- Coverage 83.35% 83.34% -0.01%
===========================================
Files 391 393 +2
Lines 41556 41582 +26
Branches 3839 3844 +5
===========================================
+ Hits 34639 34658 +19
- Misses 6917 6924 +7
|
|
|
@bsekachev |
Motivation and context
Resolved #8257
The problem is in hashing system we use to save the opened tab. It clutters the history and we cant really go back using it. There are two ways to improve that. We eighter save the actual link to go back somewhere in our application or pass it as a state when moving to analytics page
history.push(/analytics, { from: somewhere}). From my perspective the first way is more elegantTODO:
How has this been tested?
Checklist
developbranch[ ] I have updated the documentation accordingly[ ] I have added tests to cover my changes(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
Documentation