-
-
Notifications
You must be signed in to change notification settings - Fork 18
Workspace top-right controls => react #7555
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
base: master
Are you sure you want to change the base?
Conversation
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
WalkthroughThis PR modernizes the workspace top-right UI by replacing Windows Forms controls with React components. New React components for language selection, help menu, and zoom control are introduced, backed by new C# API endpoints and a ZoomModel class. The existing ZoomControl WinForms component is removed and replaced, with WorkspaceView significantly refactored to support the new architecture and coordinate with the React-based UI. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant React as React UI<br/>(WorkspaceTopRightControls)
participant API as C# Backend<br/>(WorkspaceApi)
participant Model as ZoomModel/<br/>Language State
participant WS as WebSocket/<br/>UI Updates
User->>React: Click Language Menu
React->>API: POST workspace/topRight/openLanguageMenu
API->>API: ShowUiLanguageMenu()
API-->>React: Display Context Menu
User->>React: Select Language
React->>API: Language selection triggers<br/>SetUiLanguage(langTag)
API->>Model: Update UI Language State
API->>WS: Emit uiLanguageChanged event
WS-->>React: WebSocket notification
React->>React: Re-render on language change
User->>React: Click Zoom +/−
React->>API: POST workspace/topRight/zoom<br/>(with delta)
API->>Model: Update ZoomModel.Zoom<br/>(clamped [30, 300])
API->>WS: Broadcast zoom update
WS-->>React: Zoom state updated
React->>React: Re-render with new zoom
Note over React,Model: BackColor changes propagated<br/>via OnBackColorChanged
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
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: 2
🧹 Nitpick comments (2)
src/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/TopRightMenuButton.tsx (1)
13-15: Consider whether!importantis necessary.The
!importantflag in the font-size rule may indicate a specificity conflict. If possible, resolving the underlying specificity issue would be preferable to using!important, as it can make future CSS maintenance more challenging.src/BloomExe/Workspace/WorkspaceView.cs (1)
516-535: Consider extracting magic number 125 to a named constant.The width adjustment logic works correctly, but the magic number 125 could be more self-documenting as a named constant (e.g.,
kMinTopRightPanelWidth). The inline comment explains the purpose, which helps.
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
src/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/HelpMenu.tsxsrc/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/TopRightMenuButton.tsxsrc/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/UiLanguageMenu.tsxsrc/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/WorkspaceTopRightControls.tsxsrc/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/ZoomControl.tsxsrc/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/workspaceTopRightControls.entry.tsxsrc/BloomBrowserUI/vite.config.mtssrc/BloomExe/CollectionChoosing/OpenCreateCloneControl.cssrc/BloomExe/Edit/EditingView.cssrc/BloomExe/Wizard/WinForms/WizardPage.cssrc/BloomExe/Workspace/WorkspaceView.Designer.cssrc/BloomExe/Workspace/WorkspaceView.cssrc/BloomExe/Workspace/ZoomControl.Designer.cssrc/BloomExe/Workspace/ZoomControl.cssrc/BloomExe/Workspace/ZoomControl.resxsrc/BloomExe/Workspace/ZoomModel.cssrc/BloomExe/web/ReactControl.cssrc/BloomExe/web/controllers/WorkspaceApi.cs
💤 Files with no reviewable changes (3)
- src/BloomExe/Workspace/ZoomControl.resx
- src/BloomExe/Workspace/ZoomControl.cs
- src/BloomExe/Workspace/ZoomControl.Designer.cs
🧰 Additional context used
📓 Path-based instructions (2)
src/BloomBrowserUI/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/BloomBrowserUI/**/*.{ts,tsx}: Always use arrow functions and function components in React
Do not destructure props in React components
Do not define a props data type unless it is huge; use inline prop types only when necessary
Style elements using the css macro from @emotion/react directly on the element being styled, using the css prop (e.g.,<div css={csscolor:red}>)
src/BloomBrowserUI/**/*.{ts,tsx}: Use arrow functions and function components in React
Style elements using the css macro from @emotion/react directly on the element using the css prop
Preferundefinedovernullin TypeScript for values that have not been set
Do not destructure props; useprops.foofor clarity
Avoid removing existing comments
Avoid adding comments like '// add this line'
Use useEffect only when synchronizing with external systems (subscriptions, timers, event listeners, API calls, or updates outside React control)
Avoid useEffect when data can be derived or handled internally; compute values during render instead
Always include a comment before a useEffect explaining what it does and why it is necessary
Files:
src/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/HelpMenu.tsxsrc/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/UiLanguageMenu.tsxsrc/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/WorkspaceTopRightControls.tsxsrc/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/TopRightMenuButton.tsxsrc/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/workspaceTopRightControls.entry.tsxsrc/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/ZoomControl.tsx
src/BloomBrowserUI/**/*.{ts,tsx,html}
📄 CodeRabbit inference engine (src/BloomBrowserUI/AGENTS.md)
Never use CDNs; this is an offline app
Files:
src/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/HelpMenu.tsxsrc/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/UiLanguageMenu.tsxsrc/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/WorkspaceTopRightControls.tsxsrc/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/TopRightMenuButton.tsxsrc/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/workspaceTopRightControls.entry.tsxsrc/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/ZoomControl.tsx
🧠 Learnings (17)
📓 Common learnings
Learnt from: CR
Repo: BloomBooks/BloomDesktop PR: 0
File: src/BloomBrowserUI/AGENTS.md:0-0
Timestamp: 2025-11-26T22:49:09.917Z
Learning: Stack uses TypeScript, React, MUI, Emotion, and WebView2 112
📚 Learning: 2025-11-26T22:49:09.917Z
Learnt from: CR
Repo: BloomBooks/BloomDesktop PR: 0
File: src/BloomBrowserUI/AGENTS.md:0-0
Timestamp: 2025-11-26T22:49:09.917Z
Learning: Applies to src/BloomBrowserUI/**/*.{ts,tsx} : Use arrow functions and function components in React
Applied to files:
src/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/HelpMenu.tsxsrc/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/UiLanguageMenu.tsxsrc/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/WorkspaceTopRightControls.tsxsrc/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/TopRightMenuButton.tsxsrc/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/workspaceTopRightControls.entry.tsxsrc/BloomExe/web/ReactControl.cssrc/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/ZoomControl.tsx
📚 Learning: 2025-11-26T22:48:57.452Z
Learnt from: CR
Repo: BloomBooks/BloomDesktop PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T22:48:57.452Z
Learning: Applies to src/BloomBrowserUI/**/*.{ts,tsx} : Always use arrow functions and function components in React
Applied to files:
src/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/HelpMenu.tsxsrc/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/UiLanguageMenu.tsxsrc/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/WorkspaceTopRightControls.tsxsrc/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/TopRightMenuButton.tsxsrc/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/workspaceTopRightControls.entry.tsxsrc/BloomExe/web/ReactControl.cssrc/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/ZoomControl.tsx
📚 Learning: 2025-11-26T22:48:57.452Z
Learnt from: CR
Repo: BloomBooks/BloomDesktop PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T22:48:57.452Z
Learning: For TypeScript coding style, refer to ./src/BloomBrowserUI/AGENTS.md
Applied to files:
src/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/HelpMenu.tsxsrc/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/UiLanguageMenu.tsxsrc/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/WorkspaceTopRightControls.tsxsrc/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/TopRightMenuButton.tsxsrc/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/workspaceTopRightControls.entry.tsxsrc/BloomBrowserUI/vite.config.mtssrc/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/ZoomControl.tsx
📚 Learning: 2025-11-26T22:49:09.917Z
Learnt from: CR
Repo: BloomBooks/BloomDesktop PR: 0
File: src/BloomBrowserUI/AGENTS.md:0-0
Timestamp: 2025-11-26T22:49:09.917Z
Learning: Read the AGENTS.md file at the root of this workspace and the AGENTS.md file under src/BloomBrowserUI/react_components for additional guidelines
Applied to files:
src/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/HelpMenu.tsxsrc/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/WorkspaceTopRightControls.tsxsrc/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/workspaceTopRightControls.entry.tsxsrc/BloomExe/web/ReactControl.cs
📚 Learning: 2025-11-26T22:48:57.452Z
Learnt from: CR
Repo: BloomBooks/BloomDesktop PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T22:48:57.452Z
Learning: Applies to src/BloomBrowserUI/**/*.{ts,tsx} : Do not destructure props in React components
Applied to files:
src/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/HelpMenu.tsxsrc/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/UiLanguageMenu.tsxsrc/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/WorkspaceTopRightControls.tsxsrc/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/TopRightMenuButton.tsxsrc/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/workspaceTopRightControls.entry.tsxsrc/BloomExe/web/ReactControl.cssrc/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/ZoomControl.tsx
📚 Learning: 2025-11-26T22:49:09.917Z
Learnt from: CR
Repo: BloomBooks/BloomDesktop PR: 0
File: src/BloomBrowserUI/AGENTS.md:0-0
Timestamp: 2025-11-26T22:49:09.917Z
Learning: Applies to src/BloomBrowserUI/**/*.{ts,tsx} : Always include a comment before a useEffect explaining what it does and why it is necessary
Applied to files:
src/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/HelpMenu.tsxsrc/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/TopRightMenuButton.tsxsrc/BloomExe/web/ReactControl.cssrc/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/ZoomControl.tsx
📚 Learning: 2025-11-26T22:49:09.917Z
Learnt from: CR
Repo: BloomBooks/BloomDesktop PR: 0
File: src/BloomBrowserUI/AGENTS.md:0-0
Timestamp: 2025-11-26T22:49:09.917Z
Learning: Applies to src/BloomBrowserUI/**/*.{ts,tsx} : Do not destructure props; use `props.foo` for clarity
Applied to files:
src/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/HelpMenu.tsxsrc/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/UiLanguageMenu.tsxsrc/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/TopRightMenuButton.tsxsrc/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/workspaceTopRightControls.entry.tsx
📚 Learning: 2025-11-26T22:49:09.917Z
Learnt from: CR
Repo: BloomBooks/BloomDesktop PR: 0
File: src/BloomBrowserUI/AGENTS.md:0-0
Timestamp: 2025-11-26T22:49:09.917Z
Learning: Applies to src/BloomBrowserUI/**/*.{ts,tsx} : Avoid adding comments like '// add this line'
Applied to files:
src/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/HelpMenu.tsxsrc/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/UiLanguageMenu.tsxsrc/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/WorkspaceTopRightControls.tsxsrc/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/TopRightMenuButton.tsxsrc/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/workspaceTopRightControls.entry.tsxsrc/BloomBrowserUI/vite.config.mtssrc/BloomExe/web/ReactControl.cs
📚 Learning: 2025-11-26T22:49:09.917Z
Learnt from: CR
Repo: BloomBooks/BloomDesktop PR: 0
File: src/BloomBrowserUI/AGENTS.md:0-0
Timestamp: 2025-11-26T22:49:09.917Z
Learning: Applies to src/BloomBrowserUI/**/*.uitest.ts : Use Playwright for UI tests; tests for components under /react_components use '*.uitest.ts' files
Applied to files:
src/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/UiLanguageMenu.tsxsrc/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/workspaceTopRightControls.entry.tsxsrc/BloomBrowserUI/vite.config.mts
📚 Learning: 2025-11-26T22:49:13.864Z
Learnt from: CR
Repo: BloomBooks/BloomDesktop PR: 0
File: src/BloomBrowserUI/react_components/AGENTS.md:0-0
Timestamp: 2025-11-26T22:49:13.864Z
Learning: Applies to src/BloomBrowserUI/react_components/**/*.uitest.ts : Tests for components under /react_components must use a Playwright test system with files following the '*.uitest.ts' naming convention. See src/BloomBrowserUI/react_components/component-tester/README.md for implementation details.
Applied to files:
src/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/UiLanguageMenu.tsxsrc/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/WorkspaceTopRightControls.tsxsrc/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/TopRightMenuButton.tsxsrc/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/workspaceTopRightControls.entry.tsxsrc/BloomBrowserUI/vite.config.mts
📚 Learning: 2025-11-26T22:49:09.917Z
Learnt from: CR
Repo: BloomBooks/BloomDesktop PR: 0
File: src/BloomBrowserUI/AGENTS.md:0-0
Timestamp: 2025-11-26T22:49:09.917Z
Learning: Stack uses TypeScript, React, MUI, Emotion, and WebView2 112
Applied to files:
src/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/WorkspaceTopRightControls.tsx
📚 Learning: 2025-11-26T22:49:09.917Z
Learnt from: CR
Repo: BloomBooks/BloomDesktop PR: 0
File: src/BloomBrowserUI/AGENTS.md:0-0
Timestamp: 2025-11-26T22:49:09.917Z
Learning: Applies to src/BloomBrowserUI/**/*.{ts,tsx} : Avoid removing existing comments
Applied to files:
src/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/WorkspaceTopRightControls.tsxsrc/BloomExe/web/ReactControl.cssrc/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/ZoomControl.tsx
📚 Learning: 2025-11-26T22:49:09.917Z
Learnt from: CR
Repo: BloomBooks/BloomDesktop PR: 0
File: src/BloomBrowserUI/AGENTS.md:0-0
Timestamp: 2025-11-26T22:49:09.917Z
Learning: Applies to src/BloomBrowserUI/**/*.{ts,tsx} : Style elements using the css macro from emotion/react directly on the element using the css prop
Applied to files:
src/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/WorkspaceTopRightControls.tsxsrc/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/TopRightMenuButton.tsx
📚 Learning: 2025-11-26T22:48:57.452Z
Learnt from: CR
Repo: BloomBooks/BloomDesktop PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T22:48:57.452Z
Learning: Applies to src/BloomBrowserUI/**/*.{ts,tsx} : Style elements using the css macro from emotion/react directly on the element being styled, using the css prop (e.g., `<div css={css`color:red`}>`)
Applied to files:
src/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/WorkspaceTopRightControls.tsxsrc/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/TopRightMenuButton.tsx
📚 Learning: 2025-11-26T22:49:09.917Z
Learnt from: CR
Repo: BloomBooks/BloomDesktop PR: 0
File: src/BloomBrowserUI/AGENTS.md:0-0
Timestamp: 2025-11-26T22:49:09.917Z
Learning: Applies to src/BloomBrowserUI/**/*.{ts,tsx} : Use useEffect only when synchronizing with external systems (subscriptions, timers, event listeners, API calls, or updates outside React control)
Applied to files:
src/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/workspaceTopRightControls.entry.tsxsrc/BloomExe/web/ReactControl.cssrc/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/ZoomControl.tsx
📚 Learning: 2025-11-26T22:49:09.917Z
Learnt from: CR
Repo: BloomBooks/BloomDesktop PR: 0
File: src/BloomBrowserUI/AGENTS.md:0-0
Timestamp: 2025-11-26T22:49:09.917Z
Learning: Applies to src/BloomBrowserUI/**/*.{ts,tsx} : Avoid useEffect when data can be derived or handled internally; compute values during render instead
Applied to files:
src/BloomExe/web/ReactControl.cssrc/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/ZoomControl.tsx
🧬 Code graph analysis (11)
src/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/HelpMenu.tsx (4)
src/BloomBrowserUI/react_components/l10nHooks.ts (1)
useL10n(14-39)src/BloomBrowserUI/utils/bloomApi.ts (2)
useApiString(336-338)postJson(674-692)src/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/TopRightMenuButton.tsx (2)
TopRightMenuButton(17-44)topRightMenuArrowCss(13-15)src/BloomBrowserUI/react_components/BloomToolTip.tsx (1)
BloomTooltip(41-81)
src/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/UiLanguageMenu.tsx (2)
src/BloomBrowserUI/utils/bloomApi.ts (2)
useApiString(336-338)postJson(674-692)src/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/TopRightMenuButton.tsx (2)
TopRightMenuButton(17-44)topRightMenuArrowCss(13-15)
src/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/WorkspaceTopRightControls.tsx (6)
src/BloomBrowserUI/bloomMaterialUITheme.ts (2)
lightTheme(62-143)kTextOnPurple(13-13)src/BloomBrowserUI/utils/WebSocketManager.ts (1)
useSubscribeToWebSocketForStringMessage(63-75)src/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/UiLanguageMenu.tsx (1)
UiLanguageMenu(6-20)src/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/HelpMenu.tsx (1)
HelpMenu(8-39)src/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/ZoomControl.tsx (1)
ZoomControl(15-68)src/BloomBrowserUI/utils/WireUpWinform.tsx (1)
WireUpForWinforms(14-28)
src/BloomExe/Edit/EditingView.cs (1)
src/BloomExe/Workspace/ZoomModel.cs (1)
ZoomModel(5-26)
src/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/workspaceTopRightControls.entry.tsx (2)
src/BloomBrowserUI/utils/entryPointBootstrap.tsx (1)
bootstrapReactComponent(34-61)src/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/WorkspaceTopRightControls.tsx (1)
WorkspaceTopRightControls(14-84)
src/BloomExe/Workspace/WorkspaceView.Designer.cs (1)
src/BloomExe/Workspace/WorkspaceView.cs (3)
NestedDockedChildPanel(1771-1789)buildingReaderTemplatesMenuItem_Click(1578-1581)usingReaderTemplatesMenuItem_Click(1583-1586)
src/BloomExe/web/ReactControl.cs (1)
src/BloomExe/Utils/MiscUtils.cs (2)
MiscUtils(32-918)ColorToHtmlCode(282-286)
src/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/ZoomControl.tsx (2)
src/BloomBrowserUI/utils/WebSocketManager.ts (1)
useSubscribeToWebSocketForObject(78-94)src/BloomBrowserUI/utils/bloomApi.ts (1)
postJson(674-692)
src/BloomExe/CollectionChoosing/OpenCreateCloneControl.cs (1)
src/BloomExe/Workspace/WorkspaceView.cs (3)
WorkspaceView(38-1669)WorkspaceView(78-227)GetShortenedLanguageName(1020-1037)
src/BloomExe/web/controllers/WorkspaceApi.cs (1)
src/BloomExe/Workspace/WorkspaceView.cs (6)
WorkspaceView(38-1669)WorkspaceView(78-227)GetCurrentUiLanguageLabel(550-557)ShowUiLanguageMenu(586-590)ShowHelpMenu(592-596)SetZoom(501-507)
src/BloomExe/Workspace/WorkspaceView.cs (2)
src/BloomExe/Workspace/ZoomModel.cs (1)
ZoomModel(5-26)src/BloomExe/Edit/EditingView.cs (2)
SetZoomModel(1889-1892)SetZoom(1863-1874)
🔇 Additional comments (24)
src/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/workspaceTopRightControls.entry.tsx (1)
1-4: LGTM!The entry point correctly bootstraps the WorkspaceTopRightControls component using the established pattern.
src/BloomExe/CollectionChoosing/OpenCreateCloneControl.cs (1)
337-349: LGTM!The refactor from
UpdateMenuTextToShorterNameOfSelectionto directly usingGetShortenedLanguageNameis a cleaner approach that moves from mutation to retrieval-based pattern. The logic remains equivalent and aligns with the broader language handling improvements in the PR.src/BloomExe/Wizard/WinForms/WizardPage.cs (1)
162-181: LGTM!The type change from
ToolStripDropDownItemtoToolStripItemcorrectly aligns with the actual element type in theDropDownItemscollection. This improves type accuracy without altering behavior.src/BloomBrowserUI/vite.config.mts (1)
558-559: LGTM!The new
workspaceTopRightControlsBundleentry point follows the established pattern and correctly points to the new entry file. The integration is clean and consistent with the existing configuration.src/BloomExe/web/controllers/WorkspaceApi.cs (2)
34-49: LGTM!The new endpoint registrations follow the established pattern and properly route to their respective handlers. The endpoint paths are well-structured under the
workspace/topRight/namespace.
76-91: LGTM!The handlers for UI language label and menu operations are straightforward and correctly delegate to
WorkspaceViewmethods. Response handling is appropriate for each endpoint type.src/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/HelpMenu.tsx (1)
8-39: Consider clarifying the icon-only display logic.Lines 13-14 determine when to show icon-only mode based on two conditions:
helpText === "?"(untranslated)- Language is in
["en", "fr", "de", "es"]This logic suggests that even if translations exist for English, French, German, and Spanish, the component will still show icon-only. Is this intentional? If so, a brief comment explaining the design rationale would help future maintainers understand why these specific languages prefer the icon-only display.
If this logic is correct and intentional, the implementation follows all coding guidelines correctly:
- ✓ Arrow function component
- ✓ Proper use of localization hooks
- ✓ Clean conditional rendering
src/BloomExe/web/ReactControl.cs (2)
58-65: LGTM!The
OnBackColorChangedoverride correctly synchronizes the control's background color with the browser content. The implementation properly:
- Calls the base method to preserve inherited behavior
- Uses null-safe navigation for the browser instance
- Generates a valid HTML color code (confirmed safe from the
ColorToHtmlCodeimplementation)
231-234: LGTM!The new bundle mapping for
workspaceTopRightControlsBundlefollows the established pattern and correctly points to the entry file for the new React-based top-right controls.src/BloomExe/Workspace/ZoomModel.cs (1)
5-26: LGTM!Clean, well-structured model class with proper clamping logic and change notification. The implementation correctly prevents unnecessary event firing when the clamped value matches the current value.
src/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/UiLanguageMenu.tsx (1)
6-19: LGTM!Component correctly uses arrow function syntax, avoids props destructuring, and properly delegates to the shared
TopRightMenuButtoncomponent. The API integration pattern is clean.src/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/WorkspaceTopRightControls.tsx (1)
14-84: LGTM!Well-structured component that correctly:
- Uses arrow function syntax and avoids props destructuring
- Applies Emotion's css macro for inline styling
- Memoizes the theme override appropriately
- Documents the language change subscription with a clear comment (Line 39)
- Uses the generation key pattern to force re-renders on language changes
The composition of
UiLanguageMenu,HelpMenu, andZoomControlis clean.src/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/ZoomControl.tsx (2)
15-68: LGTM!The component correctly:
- Uses arrow function syntax throughout
- Includes explanatory comments before both
useEffectcalls (Lines 18 and 25) as per guidelines- Uses the css macro from Emotion for styling
- Implements optimistic UI updates for responsive zoom changes
- Properly guards against undefined state with early null return
75-96: LGTM!
PlusMinusButtoncorrectly usesprops.onClickandprops.labelwithout destructuring, following the coding guidelines.src/BloomExe/Edit/EditingView.cs (2)
52-52: LGTM!Clean field rename from
_zoomControlto_zoomModelto reflect the new model-based approach.
1876-1892: LGTM!
AdjustPageZoomandSetZoomModelcorrectly useZoomModelconstants and properties. The boundary checks inAdjustPageZoomproperly prevent zoom adjustments beyond the allowed range before delegating to the model's clamping logic.src/BloomExe/Workspace/WorkspaceView.Designer.cs (2)
43-43: LGTM!Appropriate rename from
_toolSpecificPanelto_toolSpecificTopBarPanelfor clarity about the control's purpose.
64-64: LGTM!New
NestedDockedChildPanelcontainer for the React control with appropriate anchoring and transparent background to blend with the tab bar.src/BloomExe/Workspace/WorkspaceView.cs (6)
492-514: LGTM!
SetupZoomModel,SetZoom, andOnZoomChangedcorrectly wire up the zoom model with event-based propagation to both the current tab view and React UI via websocket.
537-548: LGTM!
GetZoomInfocorrectly builds a dynamic object with zoom state for the React control, including bounds fromZoomModelconstants.
624-646: LGTM!
ShowContextMenuuses a timer pattern to ensure the menu appears after the click event completes, preventing race conditions. This is consistent with the same pattern inEditingView.ShowContextMenu. The right-edge alignment calculation is a good UX improvement for ensuring the menu stays on the same monitor.
804-831: Well-refactored reusable helper.
AddUiLanguageMenuItemsis a good extraction that allows both the workspace context menu and the staticSetupUiLanguageMenuCommonto share menu population logic, reducing code duplication.
951-955: LGTM!
SetUiLanguageprovides a clean public API for the React UI to trigger language changes, properly delegating toApplyUiLanguageChangeandFinishUiLanguageMenuItemClick.
1220-1256: LGTM!
SelectTab(renamed fromSelectPage) correctly updates panel references to use the new naming (_toolSpecificTopBarPanel,_panelHoldingTopRightReactControl,_topRightReactControl) and properly synchronizes zoom state when switching tabs.
| export const TopRightMenuButton: React.FunctionComponent< | ||
| TopRightMenuButtonProps | ||
| > = (props) => { | ||
| return ( | ||
| <BloomButton | ||
| l10nKey="" | ||
| alreadyLocalized={true} | ||
| enabled={true} | ||
| onClick={props.onClick} | ||
| startIcon={props.startIcon} | ||
| endIcon={props.endIcon} | ||
| hasText={props.hasText === undefined ? true : props.hasText} | ||
| variant="text" | ||
| css={css` | ||
| font-size: 12px; | ||
| padding-top: 0px; | ||
| padding-bottom: 0px; | ||
| text-transform: none; | ||
| display: inline-flex; | ||
| align-items: center; | ||
| justify-content: end; | ||
| width: 100%; | ||
| `} | ||
| > | ||
| {props.text} | ||
| </BloomButton> | ||
| ); | ||
| }; |
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.
Use flex-end instead of end for broader compatibility.
Line 37 uses justify-content: end, but the standard flexbox value is flex-end. While modern browsers support end, using flex-end ensures compatibility with older browsers.
🔎 Proposed fix
display: inline-flex;
align-items: center;
- justify-content: end;
+ justify-content: flex-end;
width: 100%;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const TopRightMenuButton: React.FunctionComponent< | |
| TopRightMenuButtonProps | |
| > = (props) => { | |
| return ( | |
| <BloomButton | |
| l10nKey="" | |
| alreadyLocalized={true} | |
| enabled={true} | |
| onClick={props.onClick} | |
| startIcon={props.startIcon} | |
| endIcon={props.endIcon} | |
| hasText={props.hasText === undefined ? true : props.hasText} | |
| variant="text" | |
| css={css` | |
| font-size: 12px; | |
| padding-top: 0px; | |
| padding-bottom: 0px; | |
| text-transform: none; | |
| display: inline-flex; | |
| align-items: center; | |
| justify-content: end; | |
| width: 100%; | |
| `} | |
| > | |
| {props.text} | |
| </BloomButton> | |
| ); | |
| }; | |
| export const TopRightMenuButton: React.FunctionComponent< | |
| TopRightMenuButtonProps | |
| > = (props) => { | |
| return ( | |
| <BloomButton | |
| l10nKey="" | |
| alreadyLocalized={true} | |
| enabled={true} | |
| onClick={props.onClick} | |
| startIcon={props.startIcon} | |
| endIcon={props.endIcon} | |
| hasText={props.hasText === undefined ? true : props.hasText} | |
| variant="text" | |
| css={css` | |
| font-size: 12px; | |
| padding-top: 0px; | |
| padding-bottom: 0px; | |
| text-transform: none; | |
| display: inline-flex; | |
| align-items: center; | |
| justify-content: flex-end; | |
| width: 100%; | |
| `} | |
| > | |
| {props.text} | |
| </BloomButton> | |
| ); | |
| }; |
🤖 Prompt for AI Agents
In
src/BloomBrowserUI/react_components/TopBar/workspaceTopRightControls/TopRightMenuButton.tsx
around lines 17 to 44, the CSS uses justify-content: end which is less
compatible; change that declaration to justify-content: flex-end to use the
standard flexbox value for broader browser support and keep behavior identical.
| private void HandleZoom(ApiRequest request) | ||
| { | ||
| if (request.HttpMethod == HttpMethods.Get) | ||
| { | ||
| request.ReplyWithJson(WorkspaceView.GetZoomInfo()); | ||
| return; | ||
| } | ||
|
|
||
| var data = request.RequiredPostDynamic(); | ||
| int zoom = Convert.ToInt32(data.zoom); | ||
| WorkspaceView.SetZoom(zoom); | ||
| request.PostSucceeded(); | ||
| } |
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.
Add error handling for zoom value conversion.
Line 102 uses Convert.ToInt32(data.zoom) without error handling. If the client sends a non-integer value or the zoom field is missing from the post data, this will throw an exception that could crash the request handler.
🔎 Proposed fix
private void HandleZoom(ApiRequest request)
{
if (request.HttpMethod == HttpMethods.Get)
{
request.ReplyWithJson(WorkspaceView.GetZoomInfo());
return;
}
var data = request.RequiredPostDynamic();
- int zoom = Convert.ToInt32(data.zoom);
+ if (data.zoom == null || !int.TryParse(data.zoom.ToString(), out int zoom))
+ {
+ request.Failed("Invalid zoom value");
+ return;
+ }
WorkspaceView.SetZoom(zoom);
request.PostSucceeded();
}🤖 Prompt for AI Agents
In src/BloomExe/web/controllers/WorkspaceApi.cs around lines 93 to 105, the
conversion Convert.ToInt32(data.zoom) can throw if zoom is missing or not an
integer; replace it with a safe parse/validation: check that data contains a
zoom value, attempt to parse it with int.TryParse (or convert safely from
dynamic), and if parsing fails reply with an error (e.g., request.ReplyWithError
or a BadRequest response) rather than letting an exception propagate; only call
WorkspaceView.SetZoom(zoom) when parsing succeeds and ensure PostSucceeded is
only called on success.
This change is
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.