Skip to content

feat(webui): Add SQL input boxes to guided presto UI. #1300

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.

Sign up for GitHub

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

Merged
merged 32 commits into from
Sep 22, 2025
Merged

feat(webui): Add SQL input boxes to guided presto UI. #1300

Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
004a12d
latest
Sep 2, 2025
7194d78
latest
Sep 4, 2025
d5fc91f
Merge branch 'main' into inputMona
Sep 4, 2025
dc79169
latest
Sep 4, 2025
52e83a3
latest
Sep 8, 2025
630a360
Merge branch 'main' into inputMona
davemarco Sep 8, 2025
aa3968c
latest
Sep 9, 2025
fb61505
latest
Sep 10, 2025
af4a2b0
latest
Sep 11, 2025
202d002
latest
Sep 11, 2025
6aa31be
latest
Sep 11, 2025
2cc9f2f
merge
Sep 11, 2025
e30211e
latest
Sep 11, 2025
a8daeb2
latest
Sep 11, 2025
148379c
latest
Sep 11, 2025
d8ec46c
latest
Sep 11, 2025
84a5840
latest
Sep 12, 2025
6c3a065
Merge branch 'main' into playout
davemarco Sep 12, 2025
8da8133
lint
Sep 12, 2025
cfae7cc
Merge branch 'playout' of https://github.com/davemarco/clp into playout
Sep 12, 2025
7a3b7f9
latest
Sep 12, 2025
8d41f91
latest
Sep 17, 2025
10f9a2d
latest
Sep 17, 2025
1f2a97d
Merge branch 'main' into playout
Sep 17, 2025
96b46a1
latest
Sep 17, 2025
7eb61ac
Merge branch 'main' into playout
davemarco Sep 17, 2025
847da48
latest
Sep 17, 2025
0631199
latest
Sep 17, 2025
96e8965
Merge branch 'playout' of https://github.com/davemarco/clp into playout
Sep 17, 2025
0db25b1
latest
Sep 17, 2025
a18abc3
Merge branch 'main' into playout
davemarco Sep 17, 2025
c5d1256
Merge branch 'main' into playout
davemarco Sep 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions ...ols/Dataset/DatasetLabel/index.module.css → ...rc/components/InputLabel/index.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
.datasetLabel {
min-width: 80px;
.label {
height: 100%;
display: flex;
padding-left: 10px;
padding-right: 10px;
Comment on lines +4 to +5
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 12, 2025

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Minor: use padding shorthand

Same effect, simpler.

-    padding-left: 10px;
-    padding-right: 10px;
+    padding: 0 10px;

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In components/webui/client/src/components/InputLabel/index.module.css around
lines 4 to 5, the two separate declarations padding-left and padding-right
should be replaced with a single padding shorthand to simplify the CSS; update
the declarations to use something like padding: 0 10px; (keeping vertical
padding as needed) so the left/right padding remains 10px while reducing
verbosity.

align-items: center;
justify-content: center;
border-top-right-radius: 0px;
border-bottom-right-radius: 0px;
Copy link
Member

@junhaoliao junhaoliao Sep 16, 2025

Choose a reason for hiding this comment

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

unrelated to the PR, but since we modified a few attributes in this class

Suggested change
border-top-right-radius: 0px;
border-bottom-right-radius: 0px;
border-top-right-radius: 0;
border-bottom-right-radius: 0;

white-space: nowrap;
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 12, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Prevent label overflow on narrow layouts

white-space: nowrap can cause overflow/clipping. Add ellipsis and hide overflow.

 .label {
@@
-    white-space: nowrap;
+    white-space: nowrap;
+    overflow: hidden;
+    text-overflow: ellipsis;
 }
📝 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.

Suggested change
white-space: nowrap;
.label {
white-space: nowrap;
overflow: hidden;
text-overflow: ellipsis;
}
🤖 Prompt for AI Agents 
In components/webui/client/src/components/InputLabel/index.module.css around
line 10, the rule using "white-space: nowrap" can cause label overflow/clipping
on narrow layouts; update the CSS to prevent overflow by adding "overflow:
hidden;" and "text-overflow: ellipsis;" alongside or replacing the existing
property so long labels are truncated with an ellipsis instead of overflowing.

}
14 changes: 8 additions & 6 deletions ...chControls/Dataset/DatasetLabel/index.tsx → ...lient/src/components/InputLabel/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,27 +9,29 @@ import styles from "./index.module.css";
const {Text} = Typography;

/**
* Renders a label for the dataset selector.
* Renders a label for an input field.
*
* @param props
* @param props.children The label text to display.
* @return
Comment on lines 11 to 16
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 12, 2025

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Document a11y props.

Add docs for as and htmlFor to clarify usage as a real label.

 /**
- * Renders a label for an input field.
+ * Renders a label for an input field.
  *
- * @param props
- * @param props.children The label text to display.
+ * @param props
+ * @param props.children The label text to display.
+ * @param props.as Render as a native "label" or as a "span" (Typography.Text). Default: "span".
+ * @param props.htmlFor When as="label", associates the label to a control id.
📝 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.

Suggested change
/**
* Renders a label for the dataset selector.
* Renders a label for an input field.
*
* @param props
* @param props.children The label text to display.
* @return
/**
* Renders a label for an input field.
*
* @param props
* @param props.children The label text to display.
* @param props.as Render as a native "label" or as a "span" (Typography.Text). Default: "span".
* @param props.htmlFor When as="label", associates the label to a control id.
* @return
*/
🤖 Prompt for AI Agents 
In components/webui/client/src/components/InputLabel/index.tsx around lines 11
to 16, the JSDoc block for the component is missing documentation for the
accessible props `as` and `htmlFor`; update the comment to describe that `as`
lets callers render a custom element type (e.g., 'label' or 'div') and that
`htmlFor` should be used when rendering a real <label> to associate the label
with an input by id, including expected types and accessibility notes (e.g.,
when to pass `htmlFor` and that it must match the input id).

*/
const DatasetLabel = () => {
const InputLabel = ({children}: {children: React.ReactNode}) => {
const {token} = theme.useToken();

return (
<Text
className={styles["datasetLabel"] || ""}
className={styles["label"] || ""}
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 12, 2025

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Remove || "" fallback.

CSS modules always yield a string; the fallback is unnecessary and was removed in the diff above.

🤖 Prompt for AI Agents 
In components/webui/client/src/components/InputLabel/index.tsx around line 23,
remove the unnecessary fallback "|| ''" from the className assignment so it uses
the CSS module value directly (e.g., className={styles["label"]}); update the
line to stop providing the empty-string fallback since CSS modules always return
a string.

Copy link
Contributor

@coderabbitai coderabbitai bot Sep 12, 2025

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Drop redundant fallback

className accepts undefined; || "" is unnecessary.

-            className={styles["label"] || ""}
+            className={styles.label}
📝 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.

Suggested change
className={styles["label"] || ""}
className={styles.label}
🤖 Prompt for AI Agents 
In components/webui/client/src/components/InputLabel/index.tsx around line 23,
remove the redundant fallback "|| ''" from the className assignment because
React's className accepts undefined; change the line to use the styles lookup
directly (i.e., className={styles["label"]}) so the property can be undefined
instead of forcing an empty string.

style={{
backgroundColor: token.colorFillTertiary,
borderBottomLeftRadius: `${token.borderRadius}px`,
borderColor: token.colorBorder,
borderTopLeftRadius: `${token.borderRadius}px`,
fontSize: token.fontSizeLG,
fontSize: token.fontSize,
}}
>
Dataset
{children}
</Text>
);
Comment on lines 18 to 33
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 12, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enable semantic rendering and clean className handling.

This improves accessibility and removes the unnecessary || "".

-const InputLabel = ({children}: {children: React.ReactNode}) => {
+type InputLabelProps = {
+    children: React.ReactNode;
+    as?: "span" | "label";
+    htmlFor?: string;
+    id?: string;
+    className?: string;
+};
+
+const InputLabel = ({children, as = "span", htmlFor, id, className}: InputLabelProps) => {
     const {token} = theme.useToken();
 
-    return (
-        <Text
-            className={styles["label"] || ""}
-            style={{
-                backgroundColor: token.colorFillTertiary,
-                borderBottomLeftRadius: `${token.borderRadius}px`,
-                borderColor: token.colorBorder,
-                borderTopLeftRadius: `${token.borderRadius}px`,
-                fontSize: token.fontSize,
-            }}
-        >
-            {children}
-        </Text>
-    );
+    const commonProps = {
+        id,
+        className: `${styles.label} ${className ?? ""}`.trim(),
+        style: {
+            backgroundColor: token.colorFillTertiary,
+            borderBottomLeftRadius: `${token.borderRadius}px`,
+            borderColor: token.colorBorder,
+            borderTopLeftRadius: `${token.borderRadius}px`,
+            fontSize: token.fontSize,
+        } as React.CSSProperties,
+    };
+
+    return as === "label" ? (
+        <label {...commonProps} htmlFor={htmlFor}>
+            {children}
+        </label>
+    ) : (
+        <Text {...commonProps}>
+            {children}
+        </Text>
+    );
📝 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.

Suggested change
const InputLabel = ({children}: {children: React.ReactNode}) => {
const {token} = theme.useToken();
return (
<Text
className={styles["datasetLabel"] || ""}
className={styles["label"] || ""}
style={{
backgroundColor: token.colorFillTertiary,
borderBottomLeftRadius: `${token.borderRadius}px`,
borderColor: token.colorBorder,
borderTopLeftRadius: `${token.borderRadius}px`,
fontSize: token.fontSizeLG,
fontSize: token.fontSize,
}}
>
Dataset
{children}
</Text>
);
type InputLabelProps = {
children: React.ReactNode;
as?: "span" | "label";
htmlFor?: string;
id?: string;
className?: string;
};
const InputLabel = ({children, as = "span", htmlFor, id, className}: InputLabelProps) => {
const {token} = theme.useToken();
const commonProps = {
id,
className: `${styles.label} ${className ?? ""}`.trim(),
style: {
backgroundColor: token.colorFillTertiary,
borderBottomLeftRadius: `${token.borderRadius}px`,
borderColor: token.colorBorder,
borderTopLeftRadius: `${token.borderRadius}px`,
fontSize: token.fontSize,
} as React.CSSProperties,
};
return as === "label" ? (
<label {...commonProps} htmlFor={htmlFor}>
{children}
</label>
) : (
<Text {...commonProps}>
{children}
</Text>
);
};
🤖 Prompt for AI Agents 
In components/webui/client/src/components/InputLabel/index.tsx around lines 18
to 34, replace the non-semantic Text element with a semantic label element and
remove the unnecessary fallback for className; specifically render a native
<label> (or a Text/typography component configured to render as a label) so the
component is accessible, pass through the same inline styles, and use
className={styles["label"]} (no "|| ''" fallback). If the label needs to target
an input, accept and forward an optional htmlFor prop and apply it to the label.

Comment on lines 21 to 33
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 12, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Use semantic and add a visible border

Improves accessibility (associable via htmlFor) and ensures the border actually renders (colour without width/style is a no-op).

-const InputLabel = ({children}: {children: React.ReactNode}) => {
+type InputLabelProps = {
+    children: React.ReactNode;
+    htmlFor?: string;
+    className?: string;
+};
+
+const InputLabel = ({children, htmlFor, className}: InputLabelProps) => {
     const {token} = theme.useToken();
 
     return (
-        <Text
-            className={styles["label"] || ""}
+        <label
+            htmlFor={htmlFor}
+            className={`${styles.label} ${className ?? ""}`}
             style={{
                 backgroundColor: token.colorFillTertiary,
                 borderBottomLeftRadius: `${token.borderRadius}px`,
-                borderColor: token.colorBorder,
+                border: `1px solid ${token.colorBorder}`,
                 borderTopLeftRadius: `${token.borderRadius}px`,
                 fontSize: token.fontSize,
             }}
         >
             {children}
-        </Text>
+        </label>
     );
 };
📝 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.

Suggested change
return (
<Text
className={styles["datasetLabel"] || ""}
className={styles["label"] || ""}
style={{
backgroundColor: token.colorFillTertiary,
borderBottomLeftRadius: `${token.borderRadius}px`,
borderColor: token.colorBorder,
borderTopLeftRadius: `${token.borderRadius}px`,
fontSize: token.fontSizeLG,
fontSize: token.fontSize,
}}
>
Dataset
{children}
</Text>
);
type InputLabelProps = {
children: React.ReactNode;
htmlFor?: string;
className?: string;
};
const InputLabel = ({children, htmlFor, className}: InputLabelProps) => {
const {token} = theme.useToken();
return (
<label
htmlFor={htmlFor}
className={`${styles.label} ${className ?? ""}`}
style={{
backgroundColor: token.colorFillTertiary,
borderBottomLeftRadius: `${token.borderRadius}px`,
border: `1px solid ${token.colorBorder}`,
borderTopLeftRadius: `${token.borderRadius}px`,
fontSize: token.fontSize,
}}
>
{children}
</label>
);
};
🤖 Prompt for AI Agents 
In components/webui/client/src/components/InputLabel/index.tsx around lines
21-34, replace the non-semantic Text wrapper with a semantic <label> element (or
render Text as a label) so the component can accept and forward an htmlFor prop
for accessibility, and change the border-only color style to a concrete border
(e.g. borderWidth and borderStyle or a full border string like "1px solid
{token.colorBorder}") so the border actually renders; update the component
props/type to include htmlFor (and forward it to the rendered label), keep
existing className/style and token-based values, and ensure children remain
inside the label.

};

export default DatasetLabel;
export default InputLabel;
3 changes: 3 additions & 0 deletions ...onents/webui/client/src/components/QueryBox/InputWithCaseSensitive/CaseSenstiveToggle.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import {
Tooltip,
} from "antd";

import styles from "./index.module.css";


interface CaseSensitiveToggleProps {
disabled: boolean;
Expand Down Expand Up @@ -51,6 +53,7 @@ const CaseSensitiveToggle = ({
title={"Match case"}
>
<Button
className={styles["customButton"] || ""}
disabled={disabled}
size={"small"}
variant={buttonVariant}
Expand Down
3 changes: 3 additions & 0 deletions components/webui/client/src/components/QueryBox/InputWithCaseSensitive/index.module.css
View file
Open in desktop
Copy link
Member

@junhaoliao junhaoliao Sep 16, 2025

Choose a reason for hiding this comment

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

is this change directly related to the current PR?

Copy link
Contributor Author

@davemarco davemarco Sep 16, 2025

Choose a reason for hiding this comment

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

Indirectly related but needed. Kirk decided he prefers the smaller 14 font. So i'm moving both UI to the smaller font, which will simplify code and components reuse. Unfortunately, this icon is too big for the smaller input, so it needed to be a bit smaller.

Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
.customButton {
height: 22px;
}
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 12, 2025

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Avoid text clipping on small buttons.

Hard-coding only height can cause vertical misalignment/clipping across fonts/OS. Set line-height to match.

Apply:

 .customButton {
-  height: 22px;
+  height: 22px;
+  line-height: 22px;
 }
📝 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.

Suggested change
.customButton {
height: 22px;
}
.customButton {
height: 22px;
line-height: 22px;
}
🤖 Prompt for AI Agents 
In
components/webui/client/src/components/QueryBox/InputWithCaseSensitive/index.module.css
around lines 1-3, the .customButton rule only sets height which can cause text
clipping on some fonts/OS; add line-height: 22px to match the height so text
vertically centers and doesn't clip, and optionally ensure display is
inline-block (or block) and vertical-align: middle if needed for alignment.

Copy link
Contributor

@coderabbitai coderabbitai bot Sep 12, 2025

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Avoid fixed pixel height; use Ant Design token to match theme.

Use the small control height variable so this stays in sync across themes.

-.customButton {
-  height: 22px;
-}
+.customButton {
+  height: var(--ant-control-height-sm);
+  line-height: var(--ant-control-height-sm);
+}
📝 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.

Suggested change
.customButton {
height: 22px;
}
.customButton {
height: var(--ant-control-height-sm);
line-height: var(--ant-control-height-sm);
}
🤖 Prompt for AI Agents 
In
components/webui/client/src/components/QueryBox/InputWithCaseSensitive/index.module.css
around lines 1–3, the .customButton rule uses a fixed height: 22px; replace this
with the Ant Design small control height token so the button follows theme
changes (e.g., use the ANTD token/variable for control height small via CSS
variable or Less variable such as the small control height token), ensure the
selector preserves specificity and add any necessary line-height/vertical-align
adjustments so the button content remains centered.

Copy link
Member

@junhaoliao junhaoliao Sep 16, 2025

Choose a reason for hiding this comment

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

Suggested change
}
}

4 changes: 4 additions & 0 deletions components/webui/client/src/components/SqlEditor/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,13 @@ const SqlEditor = (props: SqlEditorProps) => {
style={{
border: `1px solid ${token.colorBorder}`,
borderRadius: token.borderRadius,
height: "100%",
minHeight: 0,
minWidth: 0,
Copy link
Member

@junhaoliao junhaoliao Sep 16, 2025

Choose a reason for hiding this comment

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

possible to create css classes for these static attributes?

Copy link
Contributor Author

@davemarco davemarco Sep 16, 2025

Choose a reason for hiding this comment

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

I did something

pointerEvents: disabled ?
"none" :
"auto",
width: "100%",
}}
Comment on lines 86 to 89
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 12, 2025

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Prefer read-only over pointer-events: none to keep copy/select enabled.

Disabling pointer events prevents users from selecting/copying SQL when disabled. Suggest using Monaco’s readOnly instead and leaving pointer events on.

Proposed change:

-                pointerEvents: disabled ?
-                    "none" :
-                    "auto",
+                // Keep interactions for selection/copy; enforce readOnly via editor options.
+                pointerEvents: "auto",

And merge readOnly into options:

             <Editor
               language={"sql"}
+              options={{
+                ...(editorProps.options ?? {}),
+                readOnly: disabled,
+              }}
               loading={
                 <div

Also applies to: 90-105

🤖 Prompt for AI Agents 
In components/webui/client/src/components/SqlEditor/index.tsx around lines 84-88
(and similarly lines 90-105), the component currently disables user interaction
by setting CSS pointerEvents to "none", which prevents text selection and
copying; replace this approach by keeping pointerEvents as normal and setting
the Monaco editor option readOnly: disabled (merge this flag into the existing
editor options object) so the editor is non-editable but still
selectable/copyable; update all occurrences in the noted ranges to remove
pointerEvents toggling and add readOnly to the options passed to Monaco.

>
<Editor
Expand Down
2 changes: 1 addition & 1 deletion components/webui/client/src/components/SqlInput/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ const SqlInput = (props: SqlEditorProps) => {

return (
<SqlEditor
height={30}
options={{
automaticLayout: true,
find: {
addExtraSpaceOnTop: false,
autoFindInSelection: "never",
Expand Down
Empty file added components/webui/client/src/components/TimeRangeInput/index.tsx
View file
Open in desktop
Empty file.
6 changes: 4 additions & 2 deletions components/webui/client/src/pages/SearchPage/SearchControls/Dataset/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,12 @@ import {useQuery} from "@tanstack/react-query";
import {
message,
Select,
theme,
} from "antd";

import InputLabel from "../../../../components/InputLabel";
import useSearchStore from "../../SearchState/index";
import {SEARCH_UI_STATE} from "../../SearchState/typings";
import DatasetLabel from "./DatasetLabel";
import styles from "./index.module.css";
import {fetchDatasetNames} from "./sql";

Expand All @@ -22,6 +23,7 @@ const Dataset = () => {
const dataset = useSearchStore((state) => state.selectDataset);
const updateDataset = useSearchStore((state) => state.updateSelectDataset);
const searchUiState = useSearchStore((state) => state.searchUiState);
const {token} = theme.useToken();

const [messageApi, contextHolder] = message.useMessage();

Expand Down Expand Up @@ -79,7 +81,7 @@ const Dataset = () => {
return (
<div className={styles["datasetContainer"]}>
{contextHolder}
<DatasetLabel/>
<InputLabel>Dataset</InputLabel>
<Select
Comment on lines +82 to 83
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 12, 2025

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Wire up accessible labelling between label and Select.

Expose the label via id and reference it from Select for screen readers.

Apply:

-            <InputLabel>Dataset</InputLabel>
+            <InputLabel id="dataset-label">Dataset</InputLabel>
             <Select
+                aria-labelledby="dataset-label"

Please confirm InputLabel forwards the id prop to a semantic element and that no conflicting ids exist on the page.

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

Suggested change
<InputLabel>Dataset</InputLabel>
<Select
<InputLabel id="dataset-label">Dataset</InputLabel>
<Select
aria-labelledby="dataset-label"

className={styles["select"] || ""}
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 12, 2025

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Remove redundant fallback in className.

styles["select"] is already a string; the || "" is unnecessary.

-                className={styles["select"] || ""}
+                className={styles["select"]}
📝 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.

Suggested change
className={styles["select"] || ""}
className={styles["select"]}
🤖 Prompt for AI Agents 
In components/webui/client/src/pages/SearchPage/SearchControls/Dataset/index.tsx
around line 84, remove the redundant fallback in the className prop —
styles["select"] is already a string so drop the "|| ''" and set className to
styles["select"] directly to simplify the JSX.

Copy link
Contributor

@coderabbitai coderabbitai bot Sep 12, 2025

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Drop redundant fallback in className.

CSS modules yield a string; the || "" is unnecessary.

Apply:

-                className={styles["select"] || ""}
+                className={styles.select}
📝 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.

Suggested change
className={styles["select"] || ""}
className={styles.select}
🤖 Prompt for AI Agents 
In components/webui/client/src/pages/SearchPage/SearchControls/Dataset/index.tsx
around line 84, the className uses a redundant fallback (`|| ""`) which is
unnecessary because CSS modules always return a string; remove the `|| ""`
fallback so the className is simply styles["select"] (or the equivalent property
access used elsewhere) to clean up the code.

loading={isPending}
Expand Down
28 changes: 0 additions & 28 deletions components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls.tsx
View file
Open in desktop

This file was deleted.

18 changes: 18 additions & 0 deletions components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/From.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import InputLabel from "../../../../../components/InputLabel";
import SqlInput from "../../../../../components/SqlInput";
import guidedGrid from "./index.module.css";
Comment on lines +1 to +3
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 12, 2025

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Prefer import aliases.

Same rationale as above.

🤖 Prompt for AI Agents 
In
components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/From.tsx
lines 1-3 the file uses deep relative imports for InputLabel, SqlInput and the
CSS module; replace these with the project's configured path aliases (for
example @components/InputLabel, @components/SqlInput and a shorter alias for the
local CSS module) so imports are shorter and resilient to file moves — update
the three import statements to use the alias paths and run the build/tsc to
verify the aliases resolve.



/**
* Renders the FROM SQL input field.
*
* @return
*/
Comment on lines +6 to +10
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 17, 2025

Choose a reason for hiding this comment

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

🧹 Nitpick

Complete the JSDoc and align with return type.

Document the return type; optionally annotate the function for stronger typing.

 /**
  * Renders the FROM SQL input field.
  *
- * @return
+ * @return JSX.Element
  */
📝 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.

Suggested change
/**
* Renders the FROM SQL input field.
*
* @return
*/
/**
* Renders the FROM SQL input field.
*
* @return JSX.Element
*/
🤖 Prompt for AI Agents 
In
components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/From.tsx
around lines 6 to 10, the JSDoc header is incomplete and doesn't state the
return type; update the JSDoc to include a descriptive summary and an explicit
@returns tag (e.g., @returns {JSX.Element}) and, optionally, annotate the
function signature with an explicit React return type (e.g., : JSX.Element or
use React.FC) to align doc and TypeScript typings; ensure imports/types are
available and the doc matches the actual return value.

const From = () => (
<div className={guidedGrid["from"]}>
<InputLabel> FROM </InputLabel>
Copy link
Member

@junhaoliao junhaoliao Sep 16, 2025

Choose a reason for hiding this comment

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

are the spaces intended?

Suggested change
<InputLabel> FROM </InputLabel>
<InputLabel>FROM</InputLabel>

<SqlInput disabled={false}/>
</div>
Comment on lines 12 to 17
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 12, 2025

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Same cleanups: no whitespace padding, no redundant prop, dot-notation.

-    <div className={guidedGrid["from"]}>
-        <InputLabel> FROM </InputLabel>
-        <SqlInput disabled={false}/>
+    <div className={guidedGrid.from}>
+        <InputLabel>FROM</InputLabel>
+        <SqlInput />
📝 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.

Suggested change
<div className={guidedGrid["from"]}>
<InputLabel> FROM </InputLabel>
<SqlInput disabled={false}/>
</div>
<div className={guidedGrid.from}>
<InputLabel>FROM</InputLabel>
<SqlInput />
</div>
🤖 Prompt for AI Agents 
In
components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/From.tsx
around lines 12 to 15, clean up the JSX by removing the extra whitespace inside
the InputLabel so it reads "FROM" (no surrounding spaces), drop the redundant
disabled={false} prop on SqlInput (it's the default), and use dot-notation for
the className: change guidedGrid["from"] to guidedGrid.from; make those three
minimal edits in place.

Comment on lines 12 to 17
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 12, 2025

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Add accessible association between label and editor

Apply the same id/aria-labelledby pattern as suggested for Select.

🤖 Prompt for AI Agents 
In
components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/From.tsx
around lines 12-15, the InputLabel is not programmatically associated with the
SqlInput; add an explicit id/label association by generating or passing a stable
id (e.g., via React's useId or a prop), set that id on the SqlInput, and set the
InputLabel to reference it (use htmlFor on InputLabel or aria-labelledby
pointing to the label id) so assistive tech can correctly associate the label
with the editor.

);

export default From;
30 changes: 30 additions & 0 deletions components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/Limit.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import {Select as AntdSelect} from "antd";
Copy link
Member

@junhaoliao junhaoliao Sep 16, 2025

Choose a reason for hiding this comment

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

do you think it would be better to change the SQL clause components as SelectBox / SelectInput for example, so that their purposes (to accept user inputs rather than displaying some output) is more obvious? Then we don't have to do this rename here as well

that said, the rename here is fine if you want to keep

Copy link
Contributor Author

@davemarco davemarco Sep 16, 2025

Choose a reason for hiding this comment

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

I think i can remove the rename but leave the name the same. I had it in a different file before thats why was renamed but not dont actually need to rename.


import InputLabel from "../../../../../components/InputLabel";
import guidedGrid from "./index.module.css";


const LIMIT_OPTIONS = [
{value: 10, label: "10"},
{value: 50, label: "50"},
{value: 100, label: "100"},
{value: 500, label: "500"},
{value: 1000, label: "1000"},
];
Comment on lines +7 to +13
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 17, 2025

Choose a reason for hiding this comment

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

🧹 Nitpick

Optional: expose and reuse a DEFAULT_LIMIT constant.

Reduces magic numbers and keeps builders/tests consistent.

 const LIMIT_OPTIONS: { value: number; label: string }[] = [
     {value: 10, label: "10"},
@@
 ];
+
+export const DEFAULT_LIMIT = 10;

And use it:

-        <Select<number>
+        <Select<number>
             aria-labelledby="limit-label"
-            value={value ?? LIMIT_OPTIONS[0].value}
+            value={value ?? DEFAULT_LIMIT}

Also applies to: 21-31

🤖 Prompt for AI Agents 
In
components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/Limit.tsx
around lines 7 to 13 (and similarly lines 21 to 31), extract a DEFAULT_LIMIT
constant (e.g., numeric default like 100) at the top of the module and replace
the hard-coded magic numbers in LIMIT_OPTIONS and any other usages with that
constant; update builders/tests to import/use DEFAULT_LIMIT so the default is
centralized and consistent across the file.


/**
* Renders the LIMIT SQL input field.
*
* @return
*/
const Limit = () => (
<div className={guidedGrid["limit"]}>
<InputLabel> LIMIT </InputLabel>
Copy link
Member

@junhaoliao junhaoliao Sep 16, 2025

Choose a reason for hiding this comment

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

ditto: if the spaces are not intended

Suggested change
<InputLabel> LIMIT </InputLabel>
<InputLabel>LIMIT</InputLabel>

<AntdSelect
Copy link
Member

@junhaoliao junhaoliao Sep 16, 2025

Choose a reason for hiding this comment

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

can we remove the left border radius? e.g.,

.noLeftBorderRadius :global(.ant-select-selector) {
    border-top-left-radius: 0!important;
    border-bottom-left-radius: 0!important;
}

defaultValue={LIMIT_OPTIONS[0]?.value}
options={LIMIT_OPTIONS}
style={{width: "100%"}}/>
Copy link
Member

@junhaoliao junhaoliao Sep 16, 2025

Choose a reason for hiding this comment

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

possible to create a class for this?

</div>
Comment on lines 21 to 30
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 12, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Persist and disable the control appropriately.

Currently uncontrolled and always enabled. Derive disabled from search state, and (optionally) expose the selected value to whatever composes the SQL.

 import InputLabel from "../../../../../components/InputLabel";
 import guidedGrid from "./index.module.css";
+import useSearchStore from "../../../SearchState/index";
+import {SEARCH_UI_STATE} from "../../../SearchState/typings";
 
 const LIMIT_OPTIONS = [
@@
-const Limit = () => (
-    <div className={guidedGrid["limit"]}>
-        <InputLabel> LIMIT </InputLabel>
-        <AntdSelect
-            defaultValue={LIMIT_OPTIONS[0]?.value}
-            options={LIMIT_OPTIONS}
-            style={{width: "100%"}}/>
-    </div>
-);
+const Limit = () => {
+    const searchUiState = useSearchStore((state) => state.searchUiState);
+    const disabled =
+        searchUiState === SEARCH_UI_STATE.QUERY_ID_PENDING ||
+        searchUiState === SEARCH_UI_STATE.QUERYING;
+    return (
+        <div className={guidedGrid["limit"]}>
+            <InputLabel>LIMIT</InputLabel>
+            <AntdSelect
+                defaultValue={LIMIT_OPTIONS[0]?.value}
+                options={LIMIT_OPTIONS}
+                size={"middle"}
+                disabled={disabled}
+                style={{width: "100%"}}/>
+        </div>
+    );
+};
📝 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.

Suggested change
<div className={guidedGrid["limit"]}>
<InputLabel> LIMIT </InputLabel>
<AntdSelect
defaultValue={LIMIT_OPTIONS[0]?.value}
options={LIMIT_OPTIONS}
style={{width: "100%"}}/>
</div>
import InputLabel from "../../../../../components/InputLabel";
import guidedGrid from "./index.module.css";
import useSearchStore from "../../../SearchState/index";
import {SEARCH_UI_STATE} from "../../../SearchState/typings";
const Limit = () => {
const searchUiState = useSearchStore((state) => state.searchUiState);
const disabled =
searchUiState === SEARCH_UI_STATE.QUERY_ID_PENDING ||
searchUiState === SEARCH_UI_STATE.QUERYING;
return (
<div className={guidedGrid["limit"]}>
<InputLabel>LIMIT</InputLabel>
<AntdSelect
defaultValue={LIMIT_OPTIONS[0]?.value}
options={LIMIT_OPTIONS}
size={"middle"}
disabled={disabled}
style={{width: "100%"}}/>
</div>
);
};

Comment on lines 21 to 30
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 12, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Make LIMIT selectable value controllable + align height with grid

Expose value/onChange to parent and align control height with grid; also add an accessible label.

-const Limit = () => (
-    <div className={guidedGrid["limit"]}>
-        <InputLabel> LIMIT </InputLabel>
-        <AntdSelect
-            defaultValue={LIMIT_OPTIONS[0]?.value}
-            options={LIMIT_OPTIONS}
-            style={{width: "100%"}}/>
-    </div>
-);
+type LimitProps = {
+    value?: number;
+    onChange?: (v: number) => void;
+};
+
+const Limit = ({value, onChange}: LimitProps) => (
+    <div className={guidedGrid["limit"]}>
+        <InputLabel>LIMIT</InputLabel>
+        <AntdSelect<number>
+            aria-label="LIMIT"
+            value={value}
+            onChange={onChange}
+            options={LIMIT_OPTIONS}
+            size="middle"
+            style={{width: "100%"}} />
+    </div>
+);
📝 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.

Suggested change
<div className={guidedGrid["limit"]}>
<InputLabel> LIMIT </InputLabel>
<AntdSelect
defaultValue={LIMIT_OPTIONS[0]?.value}
options={LIMIT_OPTIONS}
style={{width: "100%"}}/>
</div>
type LimitProps = {
value?: number;
onChange?: (v: number) => void;
};
const Limit = ({value, onChange}: LimitProps) => (
<div className={guidedGrid["limit"]}>
<InputLabel>LIMIT</InputLabel>
<AntdSelect<number>
aria-label="LIMIT"
value={value}
onChange={onChange}
options={LIMIT_OPTIONS}
size="middle"
style={{width: "100%"}} />
</div>
);
🤖 Prompt for AI Agents 
In
components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/Limit.tsx
around lines 21-27, the LIMIT select is currently uncontrolled, lacks an
accessible label association, and its visual height doesn't match the
surrounding grid; change the component to accept value and onChange props from
the parent (add to the props/interface: value?: string|number, onChange?: (v:
string|number) => void), remove the unconditional defaultValue use and pass the
controlled value prop into AntdSelect, wire AntdSelect's onChange to call
props.onChange, add an id on the select and set InputLabel's htmlFor to that id
for accessibility, and make the select fill the grid cell height (e.g., style={{
width: '100%', height: '100%' }} or apply the component/grid CSS class to align
heights).

);

export default Limit;
18 changes: 18 additions & 0 deletions ...onents/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/OrderBy.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import InputLabel from "../../../../../components/InputLabel";
import SqlInput from "../../../../../components/SqlInput";
import guidedGrid from "./index.module.css";
Comment on lines +1 to +3
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 17, 2025

Choose a reason for hiding this comment

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

🧹 Nitpick

Switch deep relative imports to path aliases

Use the repo’s configured TS path aliases for resilience to moves and shorter imports.

What alias should these resolve to in this project (e.g., @components/InputLabel)? I can provide a committable diff once confirmed.

🤖 Prompt for AI Agents 
In
components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/OrderBy.tsx
lines 1-3, replace the deep relative imports with TS path aliases: import
InputLabel from "@components/InputLabel" and import SqlInput from
"@components/SqlInput"; also replace the CSS import with the page-scoped alias
import guidedGrid from
"@pages/SearchPage/SearchControls/Presto/GuidedControls/index.module.css" so
imports are resilient to file moves and shorter.



/**
* Renders the ORDER BY SQL input field.
*
* @return
*/
Comment on lines +6 to +10
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 17, 2025

Choose a reason for hiding this comment

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

🧹 Nitpick

Tighten JSDoc: specify return type.

-/**
- * Renders the ORDER BY SQL input field.
- *
- * @return
- */
+/**
+ * Renders the ORDER BY SQL input field.
+ *
+ * @returns JSX.Element
+ */
📝 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.

Suggested change
/**
* Renders the ORDER BY SQL input field.
*
* @return
*/
/**
* Renders the ORDER BY SQL input field.
*
* @returns JSX.Element
*/
🤖 Prompt for AI Agents 
In
components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/OrderBy.tsx
around lines 6 to 10, the JSDoc block for the component's return value is too
vague; update the JSDoc to include a specific return type (for example "@returns
{JSX.Element}" or "@returns {React.ReactElement}") and a short phrase describing
what is returned (e.g., "the ORDER BY SQL input field JSX element"), keeping the
rest of the doc intact.

const OrderBy = () => (
<div className={guidedGrid["order"]}>
<InputLabel> ORDER BY </InputLabel>
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 12, 2025

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Trim label padding; use CSS for spacing.

Avoid hard-coded spaces inside label text. Style spacing via CSS for consistency and i18n.

-        <InputLabel> ORDER BY </InputLabel>
+        <InputLabel>ORDER BY</InputLabel>
📝 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.

Suggested change
<InputLabel> ORDER BY </InputLabel>
<InputLabel>ORDER BY</InputLabel>
🤖 Prompt for AI Agents 
In
components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/OrderBy.tsx
around line 13, the InputLabel contains hard-coded leading/trailing spaces ("
ORDER BY "); remove those extra spaces and adjust the component to apply spacing
via CSS (e.g., margin/padding or a className on the label or surrounding
container) so the visual gap is controlled by styles and remains i18n-safe.

Copy link
Member

@junhaoliao junhaoliao Sep 16, 2025

Choose a reason for hiding this comment

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

Suggested change
<InputLabel> ORDER BY </InputLabel>
<InputLabel>ORDER BY</InputLabel>

<SqlInput disabled={false}/>
</div>
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 12, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Wire disabled state to search UI state.

Hard-coding disabled={false} can allow edits during QUERYING. Recommend deriving disabled from the store for consistency with other controls.

Apply this diff:

 import InputLabel from "../../../../../components/InputLabel";
 import SqlInput from "../../../../../components/SqlInput";
 import guidedGrid from "./index.module.css";
+import useSearchStore from "../../../SearchState/index";
+import {SEARCH_UI_STATE} from "../../../SearchState/typings";
 
-const OrderBy = () => (
-    <div className={guidedGrid["order"]}>
-        <InputLabel> ORDER BY </InputLabel>
-        <SqlInput disabled={false}/>
-    </div>
-);
+const OrderBy = () => {
+    const searchUiState = useSearchStore((state) => state.searchUiState);
+    const disabled =
+        searchUiState === SEARCH_UI_STATE.QUERY_ID_PENDING ||
+        searchUiState === SEARCH_UI_STATE.QUERYING;
+    return (
+        <div className={guidedGrid["order"]}>
+            <InputLabel>ORDER BY</InputLabel>
+            <SqlInput disabled={disabled}/>
+        </div>
+    );
+};
📝 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.

Suggested change
<InputLabel> ORDER BY </InputLabel>
<SqlInput disabled={false}/>
</div>
import InputLabel from "../../../../../components/InputLabel";
import SqlInput from "../../../../../components/SqlInput";
import guidedGrid from "./index.module.css";
import useSearchStore from "../../../SearchState/index";
import {SEARCH_UI_STATE} from "../../../SearchState/typings";
const OrderBy = () => {
const searchUiState = useSearchStore((state) => state.searchUiState);
const disabled =
searchUiState === SEARCH_UI_STATE.QUERY_ID_PENDING ||
searchUiState === SEARCH_UI_STATE.QUERYING;
return (
<div className={guidedGrid["order"]}>
<InputLabel>ORDER BY</InputLabel>
<SqlInput disabled={disabled}/>
</div>
);
};
🤖 Prompt for AI Agents 
In
components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/OrderBy.tsx
around lines 13-15, the SqlInput is hard-coded with disabled={false}; instead,
read the disabled state from the search UI store (e.g.,
isQuerying/controlsDisabled via the existing selector or hook used by other
controls) and pass that value into SqlInput so the control is disabled during
querying; ensure you import and use the same selector/hook pattern as the other
controls for consistency.

Comment on lines 12 to 17
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 12, 2025

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Provide ARIA linkage between label and editor

Same a11y guidance as in Select: id on InputLabel + aria-labelledby on SqlInput (or extend SqlInput to accept aria props).

🤖 Prompt for AI Agents 
In
components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/OrderBy.tsx
around lines 12 to 15, the InputLabel is not programmatically linked to the
SqlInput for accessibility; add an id on the InputLabel (e.g., generated via
useId or a unique constant) and set the SqlInput to reference that id via
aria-labelledby (or update SqlInput to accept and forward aria props and pass
aria-labelledby). Ensure the id is unique in the DOM and that SqlInput receives
the aria attribute so screen readers associate the label with the editor.

Copy link
Contributor

@coderabbitai coderabbitai bot Sep 12, 2025

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Trim spaces and remove redundant prop

Consistent labelling; fewer needless props.

-        <InputLabel> ORDER BY </InputLabel>
-        <SqlInput disabled={false}/>
+        <InputLabel>ORDER BY</InputLabel>
+        <SqlInput/>
📝 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.

Suggested change
<InputLabel> ORDER BY </InputLabel>
<SqlInput disabled={false}/>
</div>
<InputLabel>ORDER BY</InputLabel>
<SqlInput/>
</div>
🤖 Prompt for AI Agents 
In
components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/OrderBy.tsx
around lines 13 to 15, the ORDER BY label contains extra surrounding spaces and
the SqlInput includes a redundant disabled={false} prop; trim the label text to
"ORDER BY" (no leading/trailing spaces) and remove the disabled={false} prop
from the SqlInput so the markup is consistent and free of needless props.

);

export default OrderBy;
18 changes: 18 additions & 0 deletions components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/Select.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import InputLabel from "../../../../../components/InputLabel";
import SqlInput from "../../../../../components/SqlInput";
import guidedGrid from "./index.module.css";
Comment on lines +1 to +3
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 12, 2025

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Prefer import aliases.

Same rationale as in Where.tsx. See tsconfig check script there.

🤖 Prompt for AI Agents 
In
components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/Select.tsx
lines 1-3, the three deep relative imports should be converted to the project
path-alias style used in Where.tsx; replace the "../../../.." relative paths
with the same alias-based imports (matching the aliases defined in tsconfig and
the imports in Where.tsx) for InputLabel, SqlInput and the CSS module, and then
run the tsconfig check script to confirm the aliases resolve.



/**
* Renders the SELECT SQL input field.
*
* @return
*/
const Select = () => (
<div className={guidedGrid["select"]}>
<InputLabel> SELECT </InputLabel>
Copy link
Member

@junhaoliao junhaoliao Sep 16, 2025

Choose a reason for hiding this comment

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

ditto: if the spaces are not intended

Suggested change
<InputLabel> SELECT </InputLabel>
<InputLabel>SELECT</InputLabel>

<SqlInput disabled={false}/>
</div>
Comment on lines 12 to 17
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 12, 2025

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Same cleanups as WHERE: remove padding spaces, redundant prop, and use dot-notation.

-    <div className={guidedGrid["select"]}>
-        <InputLabel> SELECT </InputLabel>
-        <SqlInput disabled={false}/>
+    <div className={guidedGrid.select}>
+        <InputLabel>SELECT</InputLabel>
+        <SqlInput />
📝 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.

Suggested change
<div className={guidedGrid["select"]}>
<InputLabel> SELECT </InputLabel>
<SqlInput disabled={false}/>
</div>
<div className={guidedGrid.select}>
<InputLabel>SELECT</InputLabel>
<SqlInput />
</div>
🤖 Prompt for AI Agents 
In
components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/Select.tsx
around lines 12 to 15, clean up the JSX: remove the extra spaces inside the
InputLabel text (change " SELECT " to "SELECT"), remove the redundant prop
disabled={false} from SqlInput, and replace bracket notation
guidedGrid["select"] with dot-notation guidedGrid.select; make these minimal
edits in-place.

Comment on lines 12 to 17
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 12, 2025

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Add accessible labelling for the editor

If SqlInput forwards ARIA props, link the label and input (e.g., id on InputLabel + aria-labelledby on SqlInput). Otherwise, consider extending SqlInput to accept aria-label/aria-labelledby.

🤖 Prompt for AI Agents 
In
components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/Select.tsx
around lines 12–15, the label and the SqlInput are not programmatically
associated; update the markup so the InputLabel has a unique id and the SqlInput
is given an accessible reference (either aria-labelledby pointing to that id or
an aria-label). If SqlInput currently doesn't forward ARIA props, modify the
SqlInput component to accept and forward aria-label/aria-labelledby (and id if
needed) to the underlying editable element, then wire InputLabel's id and
SqlInput's aria-labelledby in this file.

Copy link
Contributor

@coderabbitai coderabbitai bot Sep 12, 2025

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Trim extra spaces in label and drop redundant disabled prop

Removes noise; behaviour unchanged.

-        <InputLabel> SELECT </InputLabel>
-        <SqlInput disabled={false}/>
+        <InputLabel>SELECT</InputLabel>
+        <SqlInput/>
📝 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.

Suggested change
<InputLabel> SELECT </InputLabel>
<SqlInput disabled={false}/>
</div>
<InputLabel>SELECT</InputLabel>
<SqlInput/>
</div>
🤖 Prompt for AI Agents 
In
components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/Select.tsx
around lines 13 to 15, the InputLabel contains extra surrounding spaces ("
SELECT ") and SqlInput includes a redundant disabled={false} prop; remove the
extra spaces so the label reads "SELECT" and delete the disabled prop to rely on
the default behaviour.

);

export default Select;
18 changes: 18 additions & 0 deletions components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/Where.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import InputLabel from "../../../../../components/InputLabel";
import SqlInput from "../../../../../components/SqlInput";
import guidedGrid from "./index.module.css";


/**
* Renders the WHERE SQL input field.
*
* @return
*/
const Where = () => (
<div className={guidedGrid["where"]}>
<InputLabel> WHERE </InputLabel>
Copy link
Member

@junhaoliao junhaoliao Sep 16, 2025

Choose a reason for hiding this comment

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

ditto: if the spaces are not intended

Suggested change
<InputLabel> WHERE </InputLabel>
<InputLabel>WHERE</InputLabel>

<SqlInput disabled={false}/>
</div>
Comment on lines 11 to 17
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 12, 2025

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

A11y: make the label programmatic and associate it to the input.

Once InputLabel supports rendering a native label (see comment in InputLabel file), associate them via htmlFor/id.

-        <InputLabel>WHERE</InputLabel>
-        <SqlInput />
+        <InputLabel as="label" htmlFor="presto-where">WHERE</InputLabel>
+        <SqlInput id="presto-where" />

Verify SqlInput forwards id to the focusable element:

🏁 Script executed:

#!/bin/bash
# Check SqlInput props and id forwarding
rg -n -C3 'export default function SqlInput|function SqlInput|const SqlInput' components/webui/client/src/components/SqlInput
rg -n -C2 '\bid\b' components/webui/client/src/components/SqlInput

Length of output: 687

🏁 Script executed:

#!/bin/bash
set -euo pipefail

SQL_INPUT="components/webui/client/src/components/SqlInput/index.tsx"
WHERE_FILE="components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/Where.tsx"

echo "== SqlInput file check =="
if [ -f "$SQL_INPUT" ]; then
  echo "file: $SQL_INPUT"
  nl -ba "$SQL_INPUT" | sed -n '1,240p'
else
  echo "NOT FOUND: $SQL_INPUT"
fi

echo
echo "== look for 'id' occurrences inside SqlInput =="
if [ -f "$SQL_INPUT" ]; then
  rg -n --hidden --no-ignore-vcs '\bid\b' "$SQL_INPUT" || echo "no 'id' matches in $SQL_INPUT"
fi

echo
echo "== look for props spread / rest forwarding in SqlInput =="
if [ -f "$SQL_INPUT" ]; then
  rg -n --hidden --no-ignore-vcs '\.\.\.props|\.\.\.rest|\{[^}]*\.\.\.rest' "$SQL_INPUT" || echo "no obvious props spread patterns in $SQL_INPUT"
  rg -n --hidden --no-ignore-vcs 'forwardRef|React\.forwardRef' "$SQL_INPUT" || true
fi

echo
echo "== locate SqlEditorProps type/interface definition in repo =="
rg -n --hidden --no-ignore-vcs 'SqlEditorProps' || echo "SqlEditorProps not found in repo search"

echo
echo "== show Where.tsx referenced in review =="
if [ -f "$WHERE_FILE" ]; then
  nl -ba "$WHERE_FILE" | sed -n '1,240p'
else
  echo "NOT FOUND: $WHERE_FILE"
fi

Length of output: 131

🏁 Script executed:

#!/bin/bash
set -euo pipefail

SQL_INPUT="components/webui/client/src/components/SqlInput/index.tsx"
WHERE_FILE="components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/Where.tsx"

echo "== SqlInput file existence =="
if [ -f "$SQL_INPUT" ]; then echo "FOUND: $SQL_INPUT"; else echo "NOT FOUND: $SQL_INPUT"; fi

echo
echo "== SqlInput (first 400 lines) =="
if [ -f "$SQL_INPUT" ]; then sed -n '1,400p' "$SQL_INPUT"; fi

echo
echo "== 'id' occurrences inside SqlInput =="
if [ -f "$SQL_INPUT" ]; then rg -n '\bid\b' "$SQL_INPUT" || echo "no 'id' matches"; fi

echo
echo "== props spread / forwardRef patterns in SqlInput =="
if [ -f "$SQL_INPUT" ]; then
  rg -n '\.\.\.props|\.\.\.rest|{\s*\.\.\.rest|forwardRef|React\.forwardRef' "$SQL_INPUT" || true
fi

echo
echo "== Find SqlEditorProps definition in repo =="
rg -n --hidden --no-ignore-vcs 'type SqlEditorProps|interface SqlEditorProps|SqlEditorProps =' || echo "SqlEditorProps not found by simple pattern"

echo
echo "== Underlying editor / focusable element references in SqlInput =="
if [ -f "$SQL_INPUT" ]; then
  rg -n 'SqlEditor|Monaco|Ace|CodeMirror|textarea|contentEditable|codemirror|editorRef|getEditor' "$SQL_INPUT" || true
fi

echo
echo "== Where.tsx (context) =="
if [ -f "$WHERE_FILE" ]; then sed -n '1,240p' "$WHERE_FILE"; else echo "NOT FOUND: $WHERE_FILE"; fi

echo
echo "== Locate InputLabel component(s) =="
rg -n --hidden --no-ignore-vcs 'function InputLabel|const InputLabel|export .*InputLabel' components/webui/client/src || true

echo
echo "== Print candidate InputLabel files (first 240 lines) =="
rg -n --hidden --no-ignore-vcs 'function InputLabel|const InputLabel|export .*InputLabel' components/webui/client/src | cut -d: -f1 | sort -u | while read -r f; do
  [ -f "$f" ] && { echo "== FILE: $f =="; sed -n '1,240p' "$f"; } || echo "SKIP: $f (not a file)";
done

Length of output: 4610

🏁 Script executed:

#!/bin/bash
set -euo pipefail

SE_FILE="components/webui/client/src/components/SqlEditor/index.tsx"
WHERE_FILE="components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/Where.tsx"
INPUT_LABEL="components/webui/client/src/components/InputLabel/index.tsx"

echo "== SqlEditor exists? =="
[ -f "$SE_FILE" ] && echo "FOUND: $SE_FILE" || { echo "NOT FOUND: $SE_FILE"; exit 0; }

echo
echo "== SqlEditor (first 400 lines) =="
sed -n '1,400p' "$SE_FILE"

echo
echo "== 'id' occurrences in SqlEditor =="
rg -n --hidden --no-ignore-vcs '\bid\b|id\s*=' "$SE_FILE" || echo "no 'id' occurrences"

echo
echo "== props spread / forwardRef patterns in SqlEditor =="
rg -n --hidden --no-ignore-vcs '\.\.\.props|{\s*\.\.\.rest|forwardRef|React\.forwardRef' "$SE_FILE" || true

echo
echo "== underlying editor / container references in SqlEditor =="
rg -n --hidden --no-ignore-vcs 'Monaco|monaco|monaco-editor|monacoRef|editorRef|onMount|onEditorMount|textarea|contentEditable|divRef' "$SE_FILE" || true

echo
echo "== Where.tsx (for context) =="
sed -n '1,240p' "$WHERE_FILE" || true

echo
echo "== InputLabel (for context) =="
sed -n '1,240p' "$INPUT_LABEL" || true

Length of output: 5334

A11y: associate label with the SQL input — implement in InputLabel + SqlEditor first

InputLabel currently renders antd Typography.Text (no htmlFor/as props) and SqlInput simply spreads props to SqlEditor → Editor; @monaco-editor/react does not expose a way to set an HTML id on the editor DOM (see issue). (github.com)

  • Add htmlFor/as support to InputLabel so it can render a native . — components/webui/client/src/components/InputLabel/index.tsx
  • Ensure the editor wrapper receives the id: modify components/webui/client/src/components/SqlEditor/index.tsx to apply the incoming id (or htmlId) to the outer
    . SqlInput already forwards props to SqlEditor (so no change needed in SqlInput).

After those two changes, apply this Where.tsx update:

-        <InputLabel>WHERE</InputLabel>
-        <SqlInput />
+        <InputLabel as="label" htmlFor="presto-where">WHERE</InputLabel>
+        <SqlInput id="presto-where" />

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In
components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/Where.tsx
around lines 11-15, the InputLabel is not associated with the SQL editor because
InputLabel currently renders Typography.Text and SqlEditor does not expose an id
on its DOM; fix by (1) updating
components/webui/client/src/components/InputLabel/index.tsx to accept htmlFor
(or an "as" prop) and render a native <label> when htmlFor is provided (preserve
existing styling and typings), (2) updating
components/webui/client/src/components/SqlEditor/index.tsx to accept an id or
htmlId prop and apply that id to the outer wrapper <div> that contains the
monaco editor so the label can target it, and then (3) change this Where.tsx to
pass a matching id/htmlId to SqlInput (e.g., <InputLabel htmlFor="where-sql">
WHERE </InputLabel> and <SqlInput id="where-sql" />) so the label is
programmatically associated with the editor.

Comment on lines 12 to 17
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 12, 2025

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Trim label whitespace, drop redundant prop, prefer dot-notation.

  • Avoid relying on literal spaces for layout; let CSS handle padding.
  • disabled={false} is redundant.
  • guidedGrid.where reads cleaner than bracket access.
-    <div className={guidedGrid["where"]}>
-        <InputLabel> WHERE </InputLabel>
-        <SqlInput disabled={false}/>
+    <div className={guidedGrid.where}>
+        <InputLabel>WHERE</InputLabel>
+        <SqlInput />
📝 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.

Suggested change
<div className={guidedGrid["where"]}>
<InputLabel> WHERE </InputLabel>
<SqlInput disabled={false}/>
</div>
<div className={guidedGrid.where}>
<InputLabel>WHERE</InputLabel>
<SqlInput />
</div>
🤖 Prompt for AI Agents 
In
components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/Where.tsx
around lines 12 to 15, trim the literal spaces inside the InputLabel, remove the
redundant disabled={false} prop from SqlInput, and use dot-notation for the
className (guidedGrid.where) instead of bracket access; rely on CSS for any
needed padding/spacing rather than literal spaces in the label.

Comment on lines 12 to 17
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 12, 2025

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Apply a11y linkage for the editor

Use id on InputLabel and aria-labelledby (or aria-label) on SqlInput to make the control discoverable to assistive tech.

🤖 Prompt for AI Agents 
In
components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/Where.tsx
around lines 12 to 15, the InputLabel and SqlInput are not linked for
accessibility; add a unique id to the InputLabel (e.g. id="where-label" or
generated unique id) and set the SqlInput to reference it via
aria-labelledby="where-label" (or aria-label with same text if label reference
isn’t possible). Ensure the id is stable/unique in this component, update any
props passed to SqlInput to accept aria attributes, and verify the label text
matches the referenced id.

Copy link
Contributor

@coderabbitai coderabbitai bot Sep 12, 2025

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Tidy label and remove redundant prop

Same cleanup as other inputs.

-        <InputLabel> WHERE </InputLabel>
-        <SqlInput disabled={false}/>
+        <InputLabel>WHERE</InputLabel>
+        <SqlInput/>
📝 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.

Suggested change
<InputLabel> WHERE </InputLabel>
<SqlInput disabled={false}/>
</div>
<InputLabel>WHERE</InputLabel>
<SqlInput/>
</div>
🤖 Prompt for AI Agents 
In
components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/Where.tsx
around lines 13-15, tidy the label by removing extra surrounding spaces and
match casing used by other inputs (e.g., change "<InputLabel> WHERE
</InputLabel>" to "<InputLabel>Where</InputLabel>"), and remove the redundant
disabled={false} prop from the SqlInput so it relies on the default enabled
state; keep formatting consistent with the other input components.

);

export default Where;
34 changes: 34 additions & 0 deletions ...s/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/index.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
.gridContainer {
display: grid;
grid-template-columns: repeat(auto-fit, minmax(200px, 1fr));
grid-template-rows: repeat(auto-fit, 30px);
Copy link
Member

@junhaoliao junhaoliao Sep 16, 2025

Choose a reason for hiding this comment

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

on a narrower display (you may increase the zoom-level to 200% to simulate), the height of the input boxes are inconsistent. e.g.,
image

can we change this to:

Suggested change
grid-template-rows: repeat(auto-fit, 30px);
grid-auto-rows: 32px;

so that it matches antd 's default input box height? (even better, we can use some antd var instead of the hardcoded 32px)

max-width: 1250px;
Copy link
Member

@junhaoliao junhaoliao Sep 16, 2025

Choose a reason for hiding this comment

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

if not strictly needed, i think it's better to remove the width limit. since all boxes are stacked together, it should be fine even if all of them show up together on a very long line

Suggested change
max-width: 1250px;

gap: 5px;
}
Comment on lines 1 to 6
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 12, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Row sizing bug: use grid-auto-rows instead of template rows.

grid-template-rows: repeat(auto-fit, 30px) depends on container height and won’t size implicit rows. Use grid-auto-rows for consistent 30px rows; consider dense packing.

Apply:

 .gridContainer {
     display: grid;
     grid-template-columns: repeat(auto-fit, minmax(200px, 1fr));
-    grid-template-rows: repeat(auto-fit, 30px);
+    grid-auto-rows: 30px;
+    grid-auto-flow: row dense;
     max-width: 1250px;
     gap: 5px;
 }
📝 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.

Suggested change
.gridContainer {
display: grid;
grid-template-columns: repeat(auto-fit, minmax(200px, 1fr));
grid-template-rows: repeat(auto-fit, 30px);
max-width: 1250px;
gap: 5px;
}
.gridContainer {
display: grid;
grid-template-columns: repeat(auto-fit, minmax(200px, 1fr));
grid-auto-rows: 30px;
grid-auto-flow: row dense;
max-width: 1250px;
gap: 5px;
}
🤖 Prompt for AI Agents 
In
components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/index.module.css
around lines 1-7, the use of grid-template-rows: repeat(auto-fit, 30px) is
incorrect for implicit row sizing; replace that line with grid-auto-rows: 30px
so each implicit row is consistently 30px high, and optionally add
grid-auto-flow: dense if you want denser packing of items; keep the existing
grid-template-columns, max-width and gap as-is.

Comment on lines 3 to 6
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 12, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix row sizing: use auto rows instead of templated rows

grid-template-rows: repeat(auto-fit, 30px) won’t behave as intended; use grid-auto-rows so rows are created as needed. Also make the grid fill its container and pack items better.

 .gridContainer {
     display: grid;
     grid-template-columns: repeat(auto-fit, minmax(200px, 1fr));
-    grid-template-rows: repeat(auto-fit, 30px);
-    max-width: 1250px;
+    grid-auto-rows: 32px; /* consider theming to match control height */
+    max-width: 1250px;
+    width: 100%;
     gap: 5px;
+    grid-auto-flow: dense;
 }

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In
components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/index.module.css
around lines 3 to 7, replace the incorrect use of grid-template-rows:
repeat(auto-fit, 30px) with grid-auto-rows: 30px so rows are created as needed,
and ensure the grid fills its container and packs items better by setting the
container to width: 100% (or max-width as needed) and adding alignment
properties such as align-content: start (or center) and justify-items: stretch
to control item packing.


.select {
grid-column: span 3;
display: flex;
min-width: 200px;
}

.from {
grid-column: span 1;
display: flex;
min-width: 200px;
}

.where {
grid-column: span 2;
display: flex;
}

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

.limit {
grid-column: span 1;
display: flex;
}
39 changes: 39 additions & 0 deletions components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import searchStyles from "../../index.module.css";
import QueryStatus from "../../QueryStatus";
import SqlInterfaceButton from "../SqlInterfaceButton";
import SqlSearchButton from "../SqlSearchButton";
import From from "./From";
import guidedGrid from "./index.module.css";
import Limit from "./Limit";
import OrderBy from "./OrderBy";
import Select from "./Select";
import Where from "./Where";


/**
* Renders controls and status for guided sql.
*
* @return
*/
const GuidedControls = () => (
<div className={searchStyles["searchControlsContainer"]}>
<div className={guidedGrid["gridContainer"]}>
<Select/>
<From/>
<Where/>
<OrderBy/>
Comment on lines +21 to +24
Copy link
Member

@junhaoliao junhaoliao Sep 16, 2025

Choose a reason for hiding this comment

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

can we remove the left border radius? e.g.,

{
    border-top-left-radius: 0!important;
    border-bottom-left-radius: 0!important;
}

<Limit/>
</div>
Comment on lines +20 to +26
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 12, 2025

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Grid + Monaco: verify track sizing to avoid overflow and layout thrash

Ensure the grid container and items establish explicit heights (e.g., via grid-auto-rows or minmax) and set min-height: 0 on flex/grid wrappers so Monaco’s automaticLayout works reliably on resize.

🤖 Prompt for AI Agents 
In
components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/index.tsx
around lines 20-26, the grid wrapper and child panels can cause Monaco editor to
overflow or thrash on resize because explicit track sizing and min-height
constraints are missing; update the grid container CSS to define grid-auto-rows
(or use grid-template-rows with minmax) so rows have explicit heights, and add
min-height: 0 (and height: 100% where applicable) to the grid/flex wrappers and
to any intermediate container that hosts the Monaco editor so Monaco’s
automaticLayout can calculate sizes reliably during resize. Ensure the Monaco
editor wrapper itself stretches to the available cell (height: 100%) so it
doesn’t overflow.

<div className={searchStyles["statusAndButtonsRow"]}>
<div className={searchStyles["status"]}>
<QueryStatus/>
</div>
<div className={searchStyles["buttons"]}>
<SqlInterfaceButton/>
<SqlSearchButton/>
</div>
</div>
</div>
);

export default GuidedControls;
2 changes: 1 addition & 1 deletion components/webui/client/src/pages/SearchPage/SearchControls/Presto/SqlQueryInput/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@ import {
} from "react";

import SqlEditor, {SqlEditorType} from "../../../../../components/SqlEditor";
import {Nullable} from "../../../../../typings/common";
Copy link
Member

@junhaoliao junhaoliao Sep 16, 2025

Choose a reason for hiding this comment

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

if not a result of linter run, let's revert this

import useSearchStore from "../../../SearchState/index";
import {SEARCH_UI_STATE} from "../../../SearchState/typings";
import styles from "./index.module.css";
import {Nullable} from "../../../../../typings/common";


/**
Expand Down
2 changes: 1 addition & 1 deletion components/webui/client/src/pages/SearchPage/SearchControls/QueryInput/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ const QueryInput = () => {
placeholder={"Enter your query"}
progress={pseudoProgress}
ref={inputRef}
size={"large"}
size={"middle"}
value={queryString}
disabled={
searchUiState === SEARCH_UI_STATE.QUERY_ID_PENDING ||
Expand Down
2 changes: 1 addition & 1 deletion components/webui/client/src/pages/SearchPage/SearchControls/SearchButton/CancelButton.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ const CancelButton = () => {
danger={true}
htmlType={"button"}
icon={<CloseOutlined/>}
size={"large"}
size={"middle"}
type={"primary"}
onClick={handleCancelButtonClick}
>
Expand Down
2 changes: 1 addition & 1 deletion ...ents/webui/client/src/pages/SearchPage/SearchControls/SearchButton/SubmitButton/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ const SubmitButton = () => {
className={styles["gradientButton"] || ""}
htmlType={"submit"}
icon={<SearchOutlined/>}
size={"large"}
size={"middle"}
type={"primary"}
disabled={isQueryStringEmpty || isNoDatasetsAndClpS ||
searchUiState === SEARCH_UI_STATE.QUERY_ID_PENDING}
Expand Down
4 changes: 2 additions & 2 deletions components/webui/client/src/pages/SearchPage/SearchControls/TimeRangeInput/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ const TimeRangeInput = () => {
listHeight={400}
options={TIME_RANGE_OPTION_NAMES.map((option) => ({label: option, value: option}))}
popupMatchSelectWidth={false}
size={"large"}
size={"middle"}
value={timeRangeOption}
variant={"filled"}
className={timeRangeOption === TIME_RANGE_OPTION.CUSTOM ?
Expand All @@ -75,7 +75,7 @@ const TimeRangeInput = () => {
allowClear={true}
className={styles["rangePicker"] || ""}
showTime={true}
size={"large"}
size={"middle"}
value={timeRange}
disabled={searchUiState === SEARCH_UI_STATE.QUERY_ID_PENDING ||
searchUiState === SEARCH_UI_STATE.QUERYING}
Expand Down
Loading
Loading