Skip to content

feat(webui): Enforce consistent border radius by constraining overflow in Monaco editor; Improve vertical text alignment. #1393

Merged
davemarco merged 7 commits intoy-scope:mainfrom
davemarco:mona2
Oct 9, 2025
Merged

Conversation

@davemarco
Copy link
Contributor

@davemarco davemarco commented Oct 7, 2025

Description

Previously border radius only applied to outer border, but monaco had additional elements that had no radius, which looked a bit weird on rendering. Also, the text in the single-line editor alignment was off, this PR aligns closer to the centre.

Checklist

  • The PR satisfies the contribution guidelines.
  • This is a breaking change and that has been indicated in the PR title, OR this isn't a
    breaking change.
  • Necessary docs have been updated, OR no docs need to be updated.

Validation performed

Both issues fixed

Summary by CodeRabbit

  • Style
    • SQL editor content is now clipped to its bounds to prevent overflow artefacts and align with the design system.
    • Increased top padding in the SQL input editor for improved vertical spacing and readability.

@davemarco davemarco requested a review from a team as a code owner October 7, 2025 13:37
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 7, 2025

Walkthrough

Adds overflow: hidden to the Monaco editor container in SqlEditor and increases the Monaco editor padding.top in SqlInput from 4 to 6.

Changes

Cohort / File(s) Summary
SqlEditor styling
components/webui/client/src/components/SqlEditor/index.module.css
Added overflow: hidden to the .editor :global(.monaco-editor) rule (keeps existing border-radius and other properties).
SqlInput editor options
components/webui/client/src/components/SqlInput/index.tsx
Changed Monaco editor option padding.top from 4 to 6.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title succinctly captures the two primary changes—adding border radius to internal Monaco editor elements and improving vertical text alignment—and aligns directly with the stated PR objectives, making it clear and specific without extraneous detail.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d3de48f and 4b34e8b.

📒 Files selected for processing (1)
  • components/webui/client/src/components/SqlEditor/index.module.css (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
  • GitHub Check: package-image
  • GitHub Check: rust-checks (ubuntu-22.04)
  • GitHub Check: rust-checks (ubuntu-24.04)
  • GitHub Check: rust-checks (macos-15)
  • GitHub Check: lint-check (macos-15)
  • GitHub Check: lint-check (ubuntu-24.04)

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@davemarco davemarco requested a review from hoophalab October 7, 2025 13:38
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between ba34c27 and 7987bef.

📒 Files selected for processing (2)
  • components/webui/client/src/components/SqlEditor/index.module.css (1 hunks)
  • components/webui/client/src/components/SqlInput/index.tsx (1 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/SqlInput/index.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
  • GitHub Check: package-image
  • GitHub Check: lint-check (ubuntu-24.04)
  • GitHub Check: lint-check (macos-15)
  • GitHub Check: rust-checks (ubuntu-24.04)
  • GitHub Check: rust-checks (ubuntu-22.04)
  • GitHub Check: rust-checks (macos-15)
🔇 Additional comments (2)
components/webui/client/src/components/SqlEditor/index.module.css (1)

9-13: LGTM! Border radius and overflow handling are appropriate.

The CSS rule correctly targets Monaco editor internal elements to apply consistent border radius and clip overflow. The use of !important is justified here to override third-party library styles.

components/webui/client/src/components/SqlInput/index.tsx (1)

49-52: LGTM! Verify the vertical alignment improvement.

The padding adjustment should improve vertical text centering in the single-line editor. Since the PR description indicates validation is pending, please ensure the visual appearance is as intended across different browser configurations.

hoophalab
hoophalab previously approved these changes Oct 7, 2025
Copy link
Contributor

@hoophalab hoophalab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm. The border looks correct now. I only have one style comment.

Co-authored-by: hoophalab <200652805+hoophalab@users.noreply.github.com>
@davemarco davemarco requested a review from hoophalab October 8, 2025 17:29
Copy link
Member

@junhaoliao junhaoliao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

deferring to @hoophalab 's review

@hoophalab
Copy link
Contributor

hoophalab commented Oct 8, 2025

We probably need to change the title. How about this title:

feat(webui): Apply `overflow: hidden` to `SqlEditor` to fix border radius. Improve vertical text alignment.

Does Apply `overflow: hidden` to `SqlEditor` to fix border radius. sound a bit weird? better suggestions?

@davemarco davemarco changed the title feat(webui): Apply border radius to internal monaco editor elements; Improve vertical text alignment. feat(webui): Enforce consistent border radius by constraining overflow in Monaco editor; Improve vertical text alignment. Oct 9, 2025
@davemarco davemarco merged commit dcdf5f8 into y-scope:main Oct 9, 2025
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants