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
davemarco 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 23 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

@coderabbitai coderabbitai Bot Sep 12, 2025

Copy link
Copy Markdown
Contributor

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;

@junhaoliao junhaoliao Sep 16, 2025

Copy link
Copy Markdown
Member

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;

@coderabbitai coderabbitai Bot Sep 12, 2025

Copy link
Copy Markdown
Contributor

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.

}
17 changes: 9 additions & 8 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,28 @@ 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

@coderabbitai coderabbitai Bot Sep 12, 2025

Copy link
Copy Markdown
Contributor

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"] || ""}

@coderabbitai coderabbitai Bot Sep 12, 2025

Copy link
Copy Markdown
Contributor

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.

@coderabbitai coderabbitai Bot Sep 12, 2025

Copy link
Copy Markdown
Contributor

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,
backgroundColor: token.colorBorder,
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

@coderabbitai coderabbitai Bot Sep 12, 2025

Copy link
Copy Markdown
Contributor

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

@coderabbitai coderabbitai Bot Sep 12, 2025

Copy link
Copy Markdown
Contributor

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.

Comment on lines +18 to 33

@coderabbitai coderabbitai Bot Sep 17, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

🧩 Analysis chain

Verify SqlInput/SqlEditor id forwarding to enable htmlFor usage

Ensure the id passed to SqlInput lands on the actual DOM node for the label to target.

🏁 Script executed:

#!/bin/bash
# Verify id is forwarded in SqlInput/SqlEditor (read-only)
rg -n -C2 'export default function SqlInput|const SqlInput' components/webui/client/src/components/SqlInput
rg -n -C2 '\bid\b|forwardRef|\.\.\.props' components/webui/client/src/components/SqlInput components/webui/client/src/components/SqlEditor

Length of output: 931

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== SqlInput ==="
nl -ba components/webui/client/src/components/SqlInput/index.tsx | sed -n '1,240p' || true

echo
echo "=== Search for SqlEditor occurrences ==="
rg -n --hidden --no-ignore -g 'components/webui/client/src/components/**' 'SqlEditor' || true

echo
echo "=== SqlEditor source (if present) ==="
if [ -f components/webui/client/src/components/SqlEditor/index.tsx ]; then
  nl -ba components/webui/client/src/components/SqlEditor/index.tsx | sed -n '1,400p'
else
  rg -n --hidden --no-ignore 'export default function SqlEditor|const SqlEditor|forwardRef' components/webui/client/src/components || true
fi

Length of output: 1630

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== SqlInput (first 240 lines) ==="
sed -n '1,240p' components/webui/client/src/components/SqlInput/index.tsx || true

echo
echo "=== SqlEditor (first 400 lines) ==="
sed -n '1,400p' components/webui/client/src/components/SqlEditor/index.tsx || true

echo
echo "=== rg searches in SqlEditor ==="
rg -n -C3 'id\b|id=|wrapperProps|forwardRef|forwardRef\(|useRef\(|\bref\b|onEditorReady|editorDidMount|monaco|createEditor' components/webui/client/src/components/SqlEditor/index.tsx || true

Length of output: 7111

Forward id to Editor wrapper so htmlFor works

SqlInput forwards props → SqlEditor, and SqlEditor spreads the remaining props into ; @monaco-editor/react applies DOM attributes via EditorProps.wrapperProps (not a top-level id) and SqlEditor’s outer wrapper currently does not receive an id. (app.unpkg.com)

  • Change: add id?: string to SqlEditorProps (and accept it in SqlInput) and forward it into Editor via wrapperProps (merge with any existing wrapperProps).
  • Files: components/webui/client/src/components/SqlEditor/index.tsx (where is rendered) and components/webui/client/src/components/SqlInput/index.tsx (prop passthrough).
🤖 Prompt for AI Agents 
In components/webui/client/src/components/SqlEditor/index.tsx and
components/webui/client/src/components/SqlInput/index.tsx (changes around where
props are defined and where <Editor/> is rendered), add id?: string to the
SqlEditorProps type and accept id in SqlInput props, then forward that id into
the monaco <Editor/> via wrapperProps: merge the incoming id with any existing
wrapperProps object (preserving other wrapperProps keys) so the outer DOM
wrapper receives the id; ensure types reflect wrapperProps possibly being
undefined and do not overwrite existing wrapperProps when merging.

};

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

@junhaoliao junhaoliao Sep 16, 2025

Copy link
Copy Markdown
Member

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?

@davemarco davemarco Sep 16, 2025

Copy link
Copy Markdown
Contributor Author

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

@coderabbitai coderabbitai Bot Sep 12, 2025

Copy link
Copy Markdown
Contributor

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.

@coderabbitai coderabbitai Bot Sep 12, 2025

Copy link
Copy Markdown
Contributor

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.

@junhaoliao junhaoliao Sep 16, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
}
}

6 changes: 6 additions & 0 deletions components/webui/client/src/components/SqlEditor/index.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
.editor {
height: 100%;
min-height: 0;
min-width: 0;
width: 100%;
}
5 changes: 5 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 @@ -12,13 +12,16 @@ import {theme} from "antd";
import color from "color";
import * as monaco from "monaco-editor/esm/vs/editor/editor.api.js";

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

import "./monaco-loader";


type SqlEditorType = monaco.editor.IStandaloneCodeEditor;

type SqlEditorProps = Omit<EditorProps, "language"> & {
disabled: boolean;
className?: string;

/** Callback when the editor is mounted and ref is ready to use. */
onEditorReady?: (editor: SqlEditorType) => void;
Expand Down Expand Up @@ -75,6 +78,8 @@ const SqlEditor = (props: SqlEditorProps) => {

return (
<div
className={[styles["editor"],
props.className].filter(Boolean).join(" ")}
style={{
border: `1px solid ${token.colorBorder}`,
borderRadius: token.borderRadius,
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.
4 changes: 2 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 @@ -6,9 +6,9 @@ import {
Select,
} 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 Down Expand Up @@ -79,7 +79,7 @@ const Dataset = () => {
return (
<div className={styles["datasetContainer"]}>
{contextHolder}
<DatasetLabel/>
<InputLabel>Dataset</InputLabel>
<Select
Comment on lines +82 to 83

@coderabbitai coderabbitai Bot Sep 12, 2025

Copy link
Copy Markdown
Contributor

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"] || ""}

@coderabbitai coderabbitai Bot Sep 12, 2025

Copy link
Copy Markdown
Contributor

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.

@coderabbitai coderabbitai Bot Sep 12, 2025

Copy link
Copy Markdown
Contributor

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.

20 changes: 20 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,20 @@
import InputLabel from "../../../../../components/InputLabel";
import SqlInput from "../../../../../components/SqlInput";
import guidedGrid from "./index.module.css";
Comment on lines +1 to +3

@coderabbitai coderabbitai Bot Sep 12, 2025

Copy link
Copy Markdown
Contributor

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

@coderabbitai coderabbitai Bot Sep 17, 2025

Copy link
Copy Markdown
Contributor

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>

@junhaoliao junhaoliao Sep 16, 2025

Copy link
Copy Markdown
Member

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
className={guidedGrid["noLeftBorderRadius"] || ""}
disabled={false}/>
</div>
Comment on lines +12 to +17

@coderabbitai coderabbitai Bot Sep 12, 2025

Copy link
Copy Markdown
Contributor

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

@coderabbitai coderabbitai Bot Sep 12, 2025

Copy link
Copy Markdown
Contributor

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;
33 changes: 33 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,33 @@
import {Select} from "antd";

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

@coderabbitai coderabbitai Bot Sep 17, 2025

Copy link
Copy Markdown
Contributor

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>
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved

@junhaoliao junhaoliao Sep 16, 2025

Copy link
Copy Markdown
Member

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>

<Select
defaultValue={LIMIT_OPTIONS[0]?.value}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
options={LIMIT_OPTIONS}
className={
`${guidedGrid["noLeftBorderRadiusSelect"]} ${
guidedGrid["widthSelect"]}`
}/>
</div>
Comment on lines +21 to +30

@coderabbitai coderabbitai Bot Sep 12, 2025

Copy link
Copy Markdown
Contributor

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

@coderabbitai coderabbitai Bot Sep 12, 2025

Copy link
Copy Markdown
Contributor

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;
20 changes: 20 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,20 @@
import InputLabel from "../../../../../components/InputLabel";
import SqlInput from "../../../../../components/SqlInput";
import guidedGrid from "./index.module.css";
Comment on lines +1 to +3

@coderabbitai coderabbitai Bot Sep 17, 2025

Copy link
Copy Markdown
Contributor

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

@coderabbitai coderabbitai Bot Sep 17, 2025

Copy link
Copy Markdown
Contributor

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>

@coderabbitai coderabbitai Bot Sep 12, 2025

Copy link
Copy Markdown
Contributor

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.

@junhaoliao junhaoliao Sep 16, 2025

Copy link
Copy Markdown
Member

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
className={guidedGrid["noLeftBorderRadius"] || ""}
disabled={false}/>
</div>
Comment on lines +12 to +17

@coderabbitai coderabbitai Bot Sep 12, 2025

Copy link
Copy Markdown
Contributor

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.

Comment on lines +11 to +17

@coderabbitai coderabbitai Bot Sep 17, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

🧩 Analysis chain

Derive disabled state from store; clean className; add a11y label link.

Hard-coding disabled={false} allows edits while querying. Also remove redundant fallback and use dot-notation.

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

Note: If SqlInput doesn’t forward aria-* props to its wrapper/editable element, update it (or SqlEditor) accordingly.

🏁 Script executed:

#!/bin/bash
# Does SqlInput forward arbitrary props (e.g., aria-*)?
fd -a 'components/webui/client/src/components/SqlInput' | xargs -I{} sed -n '1,240p' {}
rg -n '...props|{[^}]*\.\.\.' components/webui/client/src/components/SqlInput

Length of output: 599

🏁 Script executed:

#!/bin/bash
set -euo pipefail
echo "Searching repo for occurrences of 'SqlInput'..."
rg -n --hidden -S '\bSqlInput\b' || true

echo
echo "Collecting unique files that reference 'SqlInput'..."
rg -n --hidden -S '\bSqlInput\b' | awk -F: '{print $1}' | sort -u > /tmp/sql_files || true

if [ ! -s /tmp/sql_files ]; then
  echo "No files found containing 'SqlInput'."
  exit 0
fi

while IFS= read -r f; do
  echo
  echo "---- FILE: $f ----"
  sed -n '1,400p' "$f" || true
  echo
  echo "Matches for prop-spread or aria usage in this file:"
  rg -n --hidden -S '\.\.\.[A-Za-z_][A-Za-z0-9_]*|aria-' "$f" || true
done < /tmp/sql_files

Length of output: 6188

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Searching for SqlEditor definitions and related files ==="
rg -n --hidden -S '\bSqlEditor\b' || true

echo
echo "=== Locating files that define SqlEditor ==="
sql_editor_defs=$(rg -n --hidden -S 'export default .*SqlEditor|const SqlEditor|function SqlEditor|class SqlEditor' -g 'components/**' -l || true)
echo "Found: $sql_editor_defs"

if [ -n "$sql_editor_defs" ]; then
  for f in $sql_editor_defs; do
    echo
    echo "---- FILE: $f ----"
    sed -n '1,400p' "$f" || true
    echo
    echo "Prop-spread or aria- usage in this file:"
    rg -n --hidden -S '\.\.\.[A-Za-z_][A-Za-z0-9_]*|aria-' "$f" || true
  done
else
  echo "No direct SqlEditor definition file found by those patterns; listing files that mention SqlEditor:"
  rg -n --hidden -S '\bSqlEditor\b' -g 'components/**' || true
  files=$(rg -n --hidden -S '\bSqlEditor\b' -g 'components/**' -l || true)
  for f in $files; do
    echo
    echo "---- FILE: $f ----"
    sed -n '1,400p' "$f" || true
    echo
    rg -n --hidden -S '\.\.\.[A-Za-z_][A-Za-z0-9_]*|aria-' "$f" || true
  done
fi

echo
echo "=== Inspecting SqlInput (already found earlier) to confirm prop spread ==="
if [ -f components/webui/client/src/components/SqlInput/index.tsx ]; then
  sed -n '1,240p' components/webui/client/src/components/SqlInput/index.tsx || true
  echo
  rg -n --hidden -S '\.\.\.[A-Za-z_][A-Za-z0-9_]*|aria-' components/webui/client/src/components/SqlInput/index.tsx || true
else
  echo "SqlInput file not found at expected path."
fi

echo
echo "=== Finding InputLabel implementation ==="
rg -n --hidden -S '\bInputLabel\b' || true
input_label_files=$(rg -n --hidden -S '\bInputLabel\b' -g 'components/**' -l || true)
echo "InputLabel files: $input_label_files"
for f in $input_label_files; do
  echo
  echo "---- FILE: $f ----"
  sed -n '1,240p' "$f" || true
  echo
  rg -n --hidden -S 'id=|htmlFor|htmlfor|for=|props|...props' "$f" || true
done

echo
echo "=== Locating search state (useSearchStore / SEARCH_UI_STATE) ==="
rg -n --hidden -S 'useSearchStore' || true
rg -n --hidden -S 'SEARCH_UI_STATE' || true
state_files=$(rg -n --hidden -S 'useSearchStore|SEARCH_UI_STATE' -g 'components/**' -l || true)
echo "State-related files: $state_files"
for f in $state_files; do
  echo
  echo "---- FILE: $f ----"
  sed -n '1,240p' "$f" || true
done

echo
echo "=== Inspecting GuidedControls CSS module(s) ==="
rg -n --hidden -S 'components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/.+index.module.css' || true
css_files=$(rg -n --hidden -S 'index.module.css' components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls -g 'components/**' -l || true)
echo "CSS module files: $css_files"
for f in $css_files; do
  echo
  echo "---- FILE: $f ----"
  sed -n '1,240p' "$f" || true
done

echo
echo "=== Also listing the GuidedControls components that use SqlInput (Select/From/Where/OrderBy) ==="
for f in components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/*.{tsx,ts}; do
  if [ -f "$f" ]; then
    echo
    echo "---- FILE: $f ----"
    sed -n '1,240p' "$f" || true
    rg -n --hidden -S '\.\.\.[A-Za-z_][A-Za-z0-9_]*|aria-|disabled=' "$f" || true
  fi
done

Length of output: 107374

Derive disabled state from store; clean className; add accessible label & forward aria props.

  • Replace hard-coded disabled={false} with a derived value from useSearchStore() (SEARCH_UI_STATE.QUERY_ID_PENDING / QUERYING) in guided inputs: components/.../GuidedControls/{Select.tsx,From.tsx,Where.tsx,OrderBy.tsx}. Also switch CSS access to dot-notation (guidedGrid.noLeftBorderRadius / guidedGrid.order) and remove the redundant || "" fallback.
  • Accessibility: link label → input by giving the label an id and the input aria-labelledby. Update InputLabel (components/webui/client/src/components/InputLabel/index.tsx) to accept an optional id?: string and forward it to the rendered (id={id}).
  • Forward aria-* props: SqlInput already spreads {...props} into SqlEditor (components/webui/client/src/components/SqlInput/index.tsx), but SqlEditor (components/webui/client/src/components/SqlEditor/index.tsx) only spreads remaining props into and does not apply them to the outer wrapper div. Either spread editorProps onto the wrapper div or explicitly forward aria-* attributes to that wrapper so aria-labelledby is present on the visible/editable element.
  • SqlInput change not required beyond passing aria-labelledby once InputLabel and SqlEditor are updated.
🤖 Prompt for AI Agents 
In
components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/OrderBy.tsx
around lines 11-17, replace the hard-coded disabled={false} with a derived value
from useSearchStore() that checks for SEARCH_UI_STATE.QUERY_ID_PENDING or
SEARCH_UI_STATE.QUERYING, change guidedGrid["order"] and
guidedGrid["noLeftBorderRadius"] to dot-notation (guidedGrid.order /
guidedGrid.noLeftBorderRadius) and remove the redundant || "" fallback, pass an
id prop to InputLabel and set aria-labelledby on SqlInput to that id; update
InputLabel (components/.../InputLabel/index.tsx) to accept an optional id?:
string and forward it to the rendered <Text> as id={id}, and update SqlEditor
(components/.../SqlEditor/index.tsx) to either spread editorProps (or the aria-*
attributes) onto the outer wrapper div so aria-labelledby is present on the
visible editable element (no changes needed to SqlInput beyond passing
aria-labelledby).

);

export default OrderBy;
20 changes: 20 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,20 @@
import InputLabel from "../../../../../components/InputLabel";
import SqlInput from "../../../../../components/SqlInput";
import guidedGrid from "./index.module.css";
Comment on lines +1 to +3

@coderabbitai coderabbitai Bot Sep 12, 2025

Copy link
Copy Markdown
Contributor

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>

@junhaoliao junhaoliao Sep 16, 2025

Copy link
Copy Markdown
Member

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
className={guidedGrid["noLeftBorderRadius"] || ""}
disabled={false}/>
</div>
Comment on lines +12 to +17

@coderabbitai coderabbitai Bot Sep 12, 2025

Copy link
Copy Markdown
Contributor

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

@coderabbitai coderabbitai Bot Sep 12, 2025

Copy link
Copy Markdown
Contributor

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.

Comment on lines +11 to +17

@coderabbitai coderabbitai Bot Sep 17, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

🧩 Analysis chain

Derive disabled state; clean className; add a11y linkage.

Match behaviour with other controls and tidy props.

-import InputLabel from "../../../../../components/InputLabel";
-import SqlInput from "../../../../../components/SqlInput";
-import guidedGrid from "./index.module.css";
+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 Select = () => (
-    <div className={guidedGrid["select"]}>
-        <InputLabel>SELECT</InputLabel>
-        <SqlInput
-            className={guidedGrid["noLeftBorderRadius"] || ""}
-            disabled={false}/>
-    </div>
-);
+const Select = () => {
+    const searchUiState = useSearchStore((s) => s.searchUiState);
+    const disabled =
+        searchUiState === SEARCH_UI_STATE.QUERY_ID_PENDING ||
+        searchUiState === SEARCH_UI_STATE.QUERYING;
+    return (
+        <div className={guidedGrid.select}>
+            <InputLabel id="select-label">SELECT</InputLabel>
+            <SqlInput
+                aria-labelledby="select-label"
+                className={guidedGrid.noLeftBorderRadius}
+                disabled={disabled}
+            />
+        </div>
+    );
+};

If SqlInput doesn’t forward aria-*, adjust SqlEditor wrapper to accept aria-labelledby and apply it to the container div.

🏁 Script executed:

#!/bin/bash
# Check SqlEditor props for wrapper-level aria support
sed -n '1,160p' components/webui/client/src/components/SqlEditor/index.tsx
rg -n 'aria-' components/webui/client/src/components/SqlEditor/index.tsx

Length of output: 3029

🏁 Script executed:

#!/bin/bash
set -euo pipefail

file="components/webui/client/src/components/SqlInput/index.tsx"

if [ -f "$file" ]; then
  sed -n '1,240p' "$file" || true
  echo "----"
  rg -n "aria-" "$file" || true
  echo "----"
  rg -n "SqlEditor" "$file" || true
else
  echo "MISSING: $file"
  rg -n --hidden --no-ignore -S "SqlInput" components/webui || true
fi

Length of output: 2330

Derive disabled state; clean className; add a11y linkage.

Compute disabled from the search UI state, use guidedGrid.select / guidedGrid.noLeftBorderRadius, add an id on InputLabel and pass aria-labelledby on SqlInput (Select file: components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/Select.tsx). SqlInput correctly forwards props to SqlEditor, but components/webui/client/src/components/SqlEditor/index.tsx currently only forwards unknown props into and does not apply aria-* to its outer wrapper — update SqlEditor to accept and apply aria-labelledby (or spread React.AriaAttributes) onto the container div so the label properly targets the editor container.

🤖 Prompt for AI Agents 
In
components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/Select.tsx
around lines 11-17, derive the disabled value from the search UI state
(import/use the relevant hook/selectors), pass the proper className using
guidedGrid.select and guidedGrid.noLeftBorderRadius (not the || ""), add an id
on the InputLabel (e.g. id="select-label") and pass
aria-labelledby="select-label" to SqlInput; also update
components/webui/client/src/components/SqlEditor/index.tsx to accept and apply
React.AriaAttributes (at least aria-labelledby) by spreading aria props onto the
outer container div so the label correctly targets the editor container.

);

export default Select;
20 changes: 20 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,20 @@
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>

@junhaoliao junhaoliao Sep 16, 2025

Copy link
Copy Markdown
Member

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
className={guidedGrid["noLeftBorderRadius"] || ""}
disabled={false}/>
</div>
Comment on lines +11 to +17

@coderabbitai coderabbitai Bot Sep 12, 2025

Copy link
Copy Markdown
Contributor

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

@coderabbitai coderabbitai Bot Sep 12, 2025

Copy link
Copy Markdown
Contributor

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

@coderabbitai coderabbitai Bot Sep 12, 2025

Copy link
Copy Markdown
Contributor

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.

Comment on lines +11 to +17

@coderabbitai coderabbitai Bot Sep 17, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Derive disabled state; clean className; add a11y linkage.

Same concerns as SELECT/ORDER BY.

-import InputLabel from "../../../../../components/InputLabel";
-import SqlInput from "../../../../../components/SqlInput";
-import guidedGrid from "./index.module.css";
+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 Where = () => (
-    <div className={guidedGrid["where"]}>
-        <InputLabel>WHERE</InputLabel>
-        <SqlInput
-            className={guidedGrid["noLeftBorderRadius"] || ""}
-            disabled={false}/>
-    </div>
-);
+const Where = () => {
+    const searchUiState = useSearchStore((s) => s.searchUiState);
+    const disabled =
+        searchUiState === SEARCH_UI_STATE.QUERY_ID_PENDING ||
+        searchUiState === SEARCH_UI_STATE.QUERYING;
+    return (
+        <div className={guidedGrid.where}>
+            <InputLabel id="where-label">WHERE</InputLabel>
+            <SqlInput
+                aria-labelledby="where-label"
+                className={guidedGrid.noLeftBorderRadius}
+                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
const Where = () => (
<div className={guidedGrid["where"]}>
<InputLabel>WHERE</InputLabel>
<SqlInput
className={guidedGrid["noLeftBorderRadius"] || ""}
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 Where = () => {
const searchUiState = useSearchStore((s) => s.searchUiState);
const disabled =
searchUiState === SEARCH_UI_STATE.QUERY_ID_PENDING ||
searchUiState === SEARCH_UI_STATE.QUERYING;
return (
<div className={guidedGrid.where}>
<InputLabel id="where-label">WHERE</InputLabel>
<SqlInput
aria-labelledby="where-label"
className={guidedGrid.noLeftBorderRadius}
disabled={disabled}
/>
</div>
);
};
🤖 Prompt for AI Agents 
In
components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/Where.tsx
around lines 11-17, derive the SqlInput disabled state from the component's
props/context instead of hardcoding false, clean up the className by using a
proper conditional join (or clsx) rather than using "|| ''", and add proper
accessibility linkage by giving the SqlInput a stable id and setting
InputLabel's htmlFor to that id (mirror the same approach used for SELECT/ORDER
BY); update the component to accept/obtain the disabled flag, compute className
cleanly, generate or pass an id, and wire htmlFor to that id.

);

export default Where;
45 changes: 45 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,45 @@
.gridContainer {
display: grid;
grid-template-columns: repeat(auto-fit, minmax(200px, 1fr));
grid-template-rows: 32px;
gap: 5px;
}
Comment on lines +1 to +6

@coderabbitai coderabbitai Bot Sep 12, 2025

Copy link
Copy Markdown
Contributor

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

@coderabbitai coderabbitai Bot Sep 12, 2025

Copy link
Copy Markdown
Contributor

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

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

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

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

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

.widthSelect {
width: 100%;
}

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

.noLeftBorderRadius {
border-top-left-radius: 0!important;
border-bottom-left-radius: 0!important;
}
Comment on lines +37 to +45

@coderabbitai coderabbitai Bot Sep 17, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick

Confirm left-radius overrides apply to actual controls

For non-AntD inputs (e.g., Monaco), .noLeftBorderRadius may not affect inner wrappers. If SqlInput wraps Monaco, consider a :global() rule targeting the editor container.

I can propose precise selectors after confirming SqlInput/SqlEditor DOM structure.

🤖 Prompt for AI Agents 
In
components/webui/client/src/pages/SearchPage/SearchControls/Presto/GuidedControls/index.module.css
around lines 37–45, the .noLeftBorderRadius rule may not reach inner editor
wrappers (e.g., Monaco) used by SqlInput/SqlEditor, so add a :global() selector
that targets the editor's container/selector (for example the Monaco editor
wrapper or the specific SqlEditor inner wrapper) and apply the same
border-top-left-radius and border-bottom-left-radius: 0!important overrides
there; ensure the selector matches the actual DOM class names used by
SqlInput/SqlEditor and include both the AntD .ant-select-selector rule and the
editor-specific :global(...) rule so the left-radius is removed for both AntD
and non-AntD inputs.

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

@junhaoliao junhaoliao Sep 16, 2025

Copy link
Copy Markdown
Member

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

@coderabbitai coderabbitai Bot Sep 12, 2025

Copy link
Copy Markdown
Contributor

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 "@webui/common/utility-types";
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
Loading