-
Notifications
You must be signed in to change notification settings - Fork 82
feat(webui): Add focus and hover to Monaco editor to mimic Antd styles. #1374
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: main
Are you sure you want to change the base?
Conversation
WalkthroughAdds focus and hover state handling to SqlEditor: introduces React state to track focus/hover, updates container style (dynamic border colour, box shadow, pointer events, tabIndex), and adds corresponding event handlers. CSS gains transitions for border-colour and box-shadow. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant SE as SqlEditor (React)
participant S as State (isFocused, isHovered)
participant V as DOM Styles
U->>SE: Focus
SE->>S: set isFocused = true
S-->>SE: state updated
SE->>V: apply borderColor=colorPrimary, boxShadow, pointerEvents
U->>SE: Hover enter/leave
SE->>S: set isHovered = true/false
S-->>SE: state updated
SE->>V: apply borderColor=colorPrimaryHover/Border
U->>SE: Blur
SE->>S: set isFocused = false
S-->>SE: state updated
SE->>V: remove boxShadow, restore borderColor
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Pre-merge checks and finishing touches✅ Passed checks (3 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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
components/webui/client/src/components/SqlEditor/index.module.css
(1 hunks)components/webui/client/src/components/SqlEditor/index.tsx
(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}
⚙️ CodeRabbit configuration file
- Prefer
false == <expression>
rather than!<expression>
.
Files:
components/webui/client/src/components/SqlEditor/index.tsx
🔇 Additional comments (8)
components/webui/client/src/components/SqlEditor/index.module.css (1)
6-6
: LGTM! Transition properties align with dynamic styling.The CSS transition for
border-color
andbox-shadow
with a 0.2s duration properly supports the focus and hover state changes implemented in the TypeScript component.components/webui/client/src/components/SqlEditor/index.tsx (7)
4-4
: LGTM! Necessary import for state management.The
useState
import is correctly added to support focus and hover state tracking.
41-42
: LGTM! State initialization is appropriate.Both
isFocused
andisHovered
are correctly initialized tofalse
, representing the initial unfocused and unhovered states.
82-87
: LGTM! Border colour logic correctly prioritises states.The conditional logic appropriately prioritises focused state over hovered state, with a fallback to the default border colour.
93-95
: LGTM! Pointer events correctly disabled when component is disabled.The ternary operator appropriately sets
pointerEvents
to"none"
when disabled, preventing user interaction.
102-106
: LGTM! Dynamic styling correctly applied.The container style appropriately applies the computed
borderColor
,boxShadow
, andpointerEvents
values based on the component's focus, hover, and disabled states.
107-109
: LGTM! TabIndex correctly manages keyboard accessibility.Setting
tabIndex
to-1
when disabled appropriately removes the editor from the keyboard tab order, whilst0
includes it in the natural tab order when enabled.
110-121
: LGTM! Event handlers correctly manage focus and hover state.The
onBlur
,onFocus
,onMouseEnter
, andonMouseLeave
handlers appropriately update the component's focus and hover state, enabling the dynamic styling behaviour.
const boxShadow = isFocused ? | ||
"0 0 0 2px rgba(5, 172, 255, 0.06)" : | ||
"none"; |
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.
🧹 Nitpick | 🔵 Trivial
Consider using an Antd token for the box-shadow value.
The box-shadow logic is correct. However, since the PR objectives mention using Antd design tokens where possible, consider checking if Antd provides a token for focus box-shadow (e.g., token.controlOutline
or similar) instead of the hard-coded RGBA value.
🤖 Prompt for AI Agents
In components/webui/client/src/components/SqlEditor/index.tsx around lines 89 to
91, the boxShadow is hard-coded as an RGBA string when focused; replace this
with the appropriate Ant Design token (e.g., token.controlOutline or the
control/focus shadow token provided by theme.useToken) so the component uses
Antd design tokens instead of a literal value; import/use theme.useToken (or
accept the token from props/context), read the correct token
(controlOutline/focus shadow) and apply it to boxShadow when isFocused is true,
falling back to the current rgba value only if the token is undefined.
Description
Adds focus and hover to monaco editor component. Used antd tokens where possible.
I looked at doing it with css, but was more difficult to use the Antd token, and deal with multiple states. Using the state logic seemed cleaner.
There are still some issues with centering, and border radius. will look into in another PR
Checklist
breaking change.
Validation performed
Focus and hover work correctly.
Summary by CodeRabbit
New Features
Style