feat(webui): Enforce 2-column layout for SQL input grid in guided Presto UI. by davemarco · Pull Request #1638 · y-scope/clp

davemarco · 2025-11-20T16:50:06Z

Description

Previous grid in presto UI used auto-fit to dynamically position sql fields. Therefore on a larger screen, the layout could potentially be on one row. We determined this is undesirable, and now it is always on one row.

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

always on 2 rows

Summary by CodeRabbit

Style
- Updated search filter layout to a fixed two-column grid with smaller column minima.
- Converted previously full-width controls to single-column grid items for consistent alignment.
- Removed multi-column spanning for several controls to tighten layout.
- Optimized element sizing for a more compact, uniform search interface presentation.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

To see the specific tasks where the Asana app for GitHub is being used, see below:
- https://app.asana.com/0/0/1212018015778917

coderabbitai · 2025-11-20T16:50:12Z

Walkthrough

Refactored guided controls CSS grid from a responsive auto-fit layout (min 200px) to a fixed two-column grid (min 100px) and replaced multi-column span selectors with a single-column .gridItem; corresponding components updated to use the new .gridItem class.

Changes

Cohort / File(s)	Summary
CSS Grid Layout Update `components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/index.module.css`	Replaced `grid-template-columns: repeat(auto-fit, minmax(200px, 1fr))` with `repeat(2, minmax(100px, 1fr))`; removed `.select`, `.from`, `.where`, `.order` classes that spanned 2 columns; added `.gridItem` with `grid-column: span 1; display: flex;`.
Component class updates `components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/From.tsx`, `.../OrderBy.tsx`, `.../Select.tsx`, `.../Where.tsx`	Each component's top-level wrapper class changed from the removed multi-column class (`from`, `order`, `select`, `where`) to the new `gridItem` class. No logic or behavior changes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5–15 minutes

Verify CSS selectors applied correctly and no leftover references to removed classes.
Visually check layout across breakpoints, especially narrow widths around the 100px minimum.
Confirm no unintended style regressions in the affected components.

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 accurately describes the main change: enforcing a fixed 2-column layout for the SQL input grid in the guided Presto UI, which aligns with the changeset modifications to grid layout and CSS class assignments.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

📝 Generate docstrings

🧪 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 3c5f15d and 94fce52.

📒 Files selected for processing (5)

components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/From.tsx (1 hunks)
components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/OrderBy.tsx (1 hunks)
components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/Select.tsx (1 hunks)
components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/Where.tsx (1 hunks)
components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/index.module.css (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/pages/SearchPage/SearchControls/Presto/GuidedControls/OrderBy.tsx
components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/Where.tsx
components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/From.tsx
components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/Select.tsx

🧠 Learnings (1)

📓 Common learnings

Learnt from: haiqi96
Repo: y-scope/clp PR: 0
File: :0-0
Timestamp: 2025-07-29T14:04:13.769Z
Learning: User haiqi96 requested creating a GitHub issue to document a bug fix from PR #1136, which addressed MySQL compatibility issues with invalid SQL CAST operations in the WebUI component.

⏰ 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). (3)

GitHub Check: package-image
GitHub Check: lint-check (macos-15)
GitHub Check: lint-check (ubuntu-24.04)

🔇 Additional comments (6)

components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/index.module.css (2)

8-11: LGTM! Clean consolidation of grid item styles.

The new .gridItem class effectively consolidates the previously separate classes (.select, .from, .where, .order) into a single, reusable style definition that enforces single-column spanning in the 2-column grid. This addresses the previous review feedback about reducing duplication.

3-3: Manual verification required: Confirm the minmax change and test mobile rendering.

The verification could not confirm whether the minimum column width was previously 200px—git history shows no record of this change. Additionally, whether 100px per column causes usability issues depends on the actual rendered dimensions of the Select, From, Where, and OrderBy components, which cannot be determined through static code analysis.

Please verify:

Confirm this change was actually made from 200px to 100px (check PR description or commit message)

Test the grid layout on mobile viewports to ensure SQL input fields and labels render properly with the new 100px minimum

components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/Where.tsx (1)

23-23: LGTM! Consistent with the new grid layout.

The wrapper class has been correctly updated to use the new .gridItem class, aligning with the 2-column grid layout changes in the CSS module.

components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/From.tsx (1)

13-13: LGTM! Consistent with the new grid layout.

The wrapper class has been correctly updated to use the new .gridItem class, aligning with the 2-column grid layout changes in the CSS module.

components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/OrderBy.tsx (1)

23-23: LGTM! Consistent with the new grid layout.

The wrapper class has been correctly updated to use the new .gridItem class, aligning with the 2-column grid layout changes in the CSS module.

components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/Select.tsx (1)

23-23: LGTM! Consistent with the new grid layout.

The wrapper class has been correctly updated to use the new .gridItem class, aligning with the 2-column grid layout changes in the CSS module.

Tip

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

Provide your own instructions using the high_level_summary_instructions setting.
Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

📝 Description — Summarize the main change in 50–60 words, explaining what was done.

📓 References — List relevant issues, discussions, documentation, or related PRs.

📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.

📊 Contributor Summary — Include a Markdown table showing contributions:
| Contributor | Lines Added | Lines Removed | Files Changed |

✔️ Additional Notes — Add any extra reviewer context.
Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.

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.}

junhaoliao

i use the browser debugger tab to simulate various screen resolutions whose widths range from 1000px - 2560px. the layout is consistently 2-column which matches the purpose of the change.

for the title, how about:

feat(webui): Enforce a fixed 2-column layout for guided Presto search controls.

note the time range selector may overflow the window when the window width is less than 1000px, though i think that's unrelated to the current PR. we should submit an issue to track the fix if the problem is confirmed.

junhaoliao · 2025-11-22T03:22:23Z

    gap: 5px;
 }

 .select {


kinda unrelated to the changes in this PR - would it be cleaner to just write

.select .from .where .order { grid-column: span 1; display: flex; }

to avoid repeating the style definitions? if in the future they will have different styles, we can always split the block?

I changed to one class

davemarco · 2025-11-25T16:57:39Z

@junhaoliao the overflow issue will be better after we merge the PR where the timekey selector is moved. There will be more space on the top row.

davemarco · 2025-11-25T17:06:55Z

@junhaoliao can you reapprove to due to merge conflicts

…sto UI. (y-scope#1638)

latest

ee89720

davemarco marked this pull request as ready for review November 20, 2025 16:50

davemarco requested a review from a team as a code owner November 20, 2025 16:50

Merge branch 'main' into 2column

3c5f15d

davemarco requested a review from hoophalab November 20, 2025 16:50

davemarco changed the title ~~feat(webui): Enforce 2-column layout for SQL input grid in Presto Guided UI.~~ feat(webui): Enforce 2-column layout for SQL input grid in guided Presto UI. Nov 20, 2025

junhaoliao self-requested a review November 20, 2025 21:03

Merge branch 'main' into 2column

8c76d2a

junhaoliao previously approved these changes Nov 22, 2025

View reviewed changes

latest

94fce52

davemarco dismissed junhaoliao’s stale review via 94fce52 November 25, 2025 16:56

davemarco requested a review from junhaoliao November 25, 2025 16:57

junhaoliao previously approved these changes Nov 25, 2025

View reviewed changes

Merge branch 'main' into 2column

7f5c624

davemarco dismissed junhaoliao’s stale review via 7f5c624 November 25, 2025 17:01

davemarco requested a review from junhaoliao November 25, 2025 17:06

junhaoliao approved these changes Nov 25, 2025

View reviewed changes

davemarco merged commit 21b8da6 into y-scope:main Nov 25, 2025
19 checks passed

junhaoliao pushed a commit to junhaoliao/clp that referenced this pull request May 17, 2026

feat(webui): Enforce 2-column layout for SQL input grid in guided Pre…

353923e

…sto UI. (y-scope#1638)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(webui): Enforce 2-column layout for SQL input grid in guided Presto UI.#1638

feat(webui): Enforce 2-column layout for SQL input grid in guided Presto UI.#1638
davemarco merged 5 commits into
y-scope:mainfrom
davemarco:2column

davemarco commented Nov 20, 2025 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Nov 20, 2025 •

edited

Loading

Uh oh!

junhaoliao left a comment •

edited

Loading

Uh oh!

junhaoliao Nov 22, 2025

Uh oh!

davemarco Nov 25, 2025

Uh oh!

davemarco commented Nov 25, 2025

Uh oh!

davemarco commented Nov 25, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

davemarco commented Nov 20, 2025 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Validation performed

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Nov 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Pre-merge checks and finishing touches

Uh oh!

junhaoliao left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

junhaoliao Nov 22, 2025

Choose a reason for hiding this comment

Uh oh!

davemarco Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

davemarco commented Nov 25, 2025

Uh oh!

davemarco commented Nov 25, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

davemarco commented Nov 20, 2025 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Nov 20, 2025 •

edited

Loading

junhaoliao left a comment •

edited

Loading