Misc fixes for toolkit UI and tool cycling #8658
Conversation
📝 WalkthroughWalkthroughThis update introduces stricter enforcement of read-only mode by preventing activation of unavailable tools and toolkits. It centralizes tool availability logic, adjusts reducers and sagas to respect mode restrictions, and refines UI components to hide toolkits when editing is disallowed. Minor UI improvements for segment ID display are also included. Changes
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
![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
🧹 Nitpick comments (1)
frontend/javascripts/viewer/model/sagas/settings_saga.ts (1)
7-11: Optimize imports by usingimport typefor type-only imports.The static analysis correctly identifies that
SetViewModeActionandUpdateUserSettingActionare only used as type annotations and should be imported as types to avoid loading unnecessary modules at runtime.Apply this diff to optimize the imports:
import { - SetViewModeAction, - type UpdateUserSettingAction, + type SetViewModeAction, + type UpdateUserSettingAction, updateUserSettingAction, } from "viewer/model/actions/settings_actions";🧰 Tools
🪛 Biome (1.9.4)
[error] 7-11: Some named imports are only used as types.
This import is only used as a type.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Add inline type keywords.(lint/style/useImportType)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
frontend/javascripts/viewer/model/actions/settings_actions.ts(1 hunks)frontend/javascripts/viewer/model/sagas/settings_saga.ts(3 hunks)frontend/javascripts/viewer/view/action_bar_view.tsx(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- frontend/javascripts/viewer/model/actions/settings_actions.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/javascripts/viewer/view/action_bar_view.tsx
🧰 Additional context used
🧬 Code Graph Analysis (1)
frontend/javascripts/viewer/model/sagas/settings_saga.ts (1)
frontend/javascripts/viewer/model/actions/settings_actions.ts (2)
updateUserSettingAction(55-63)SetViewModeAction(19-19)
🪛 Biome (1.9.4)
frontend/javascripts/viewer/model/sagas/settings_saga.ts
[error] 7-11: Some named imports are only used as types.
This import is only used as a type.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Add inline type keywords.
(lint/style/useImportType)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: backend-tests
- GitHub Check: build-smoketest-push
🔇 Additional comments (2)
frontend/javascripts/viewer/model/sagas/settings_saga.ts (2)
118-137: Well-implemented saga for enforcing toolkit restrictions in read-only mode.The
ensureValidToolkitsaga properly addresses the PR objective of preventing activation of unavailable tools in read-only scenarios. The implementation correctly:
- Waits for system readiness before checking conditions
- Checks both view mode and annotation update restrictions
- Defaults to
ALL_TOOLStoolkit when editing is restricted- Continuously monitors view mode changes and resets toolkit when needed
The logic handles the edge case where users might switch from orthogonal to other view modes, ensuring toolkit consistency across different viewing contexts.
154-154: Proper integration of the new saga into the main watcher.The
ensureValidToolkitsaga is correctly integrated using thecalleffect, ensuring it starts when the settings system initializes and continues running to monitor state changes.
* hide toolkit switcher in read-only and fix+refactor tool setting/cycling * switch to the default toolkit if the current annotation was opened in read-only mode * add padding to the segment id in the list if a segment name exists * fix import * update changelog * also hide toolkit switcher in arbitrary modes * also switch to default toolkit if oblique or flight mode is activated * lint
Additionally, I added some margin to the segment id in the segment list (only relevant when a segment name exists).
URL of deployed dev instance (used for testing):
Steps to test:
Issues: