Skip to content
Merged
Show file tree
Hide file tree
Changes from 31 commits
Commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
.label {
height: 100%;
display: flex;
padding-left: 10px;
padding-right: 10px;
Comment on lines +4 to +5
Copy link
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: 0;
border-bottom-right-radius: 0;
white-space: nowrap;
Copy link
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.

}
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
Copy link
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"] || ""}
Copy link
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.

Copy link
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
Copy link
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;
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
Copy link
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?

Copy link
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;
}
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%;
}
Comment on lines +1 to +6
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick

Add box-sizing to prevent overflow when borders are applied.

With height/width at 100% and a 1px border added inline on the wrapper, content-box sizing can overflow its grid cell. Use border-box.

 .editor {
     height: 100%;
     min-height: 0;
     min-width: 0;
     width: 100%;
+    box-sizing: border-box;
 }
📝 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
.editor {
height: 100%;
min-height: 0;
min-width: 0;
width: 100%;
}
.editor {
height: 100%;
min-height: 0;
min-width: 0;
width: 100%;
box-sizing: border-box;
}
🤖 Prompt for AI Agents
In components/webui/client/src/components/SqlEditor/index.module.css around
lines 1 to 6, the .editor class uses 100% height/width and can overflow when a
1px border is applied because the default content-box sizing excludes borders;
change the box model to border-box by adding box-sizing: border-box to the
.editor selector so borders are included in the element's dimensions and prevent
grid-cell overflow.

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 All @@ -31,7 +34,7 @@ type SqlEditorProps = Omit<EditorProps, "language"> & {
* @return
*/
const SqlEditor = (props: SqlEditorProps) => {
const {disabled, onEditorReady, ...editorProps} = props;
const {disabled, onEditorReady, className, ...editorProps} = props;
const monacoEditor = useMonaco();
const {token} = theme.useToken();

Expand Down Expand Up @@ -75,6 +78,8 @@ const SqlEditor = (props: SqlEditorProps) => {

return (
<div
className={[styles["editor"],
className].filter(Boolean).join(" ")}
Comment on lines +81 to +82
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick

Optional: simplify class merge and prefer dot access.

If clsx (or classnames) is already in the repo, use it; otherwise dot access alone improves readability.

-            className={[styles["editor"],
-                className].filter(Boolean).join(" ")}
+            className={[styles.editor, className].filter(Boolean).join(" ")}
+            // or: className={clsx(styles.editor, className)}

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

🤖 Prompt for AI Agents
In components/webui/client/src/components/SqlEditor/index.tsx around lines
81-82, the className construction uses array filter/join and bracket property
access which is verbose; if the repo already includes clsx or classnames,
replace the array/filter/join with a single clsx(...) call (e.g.,
clsx(styles.editor, className)); otherwise simplify to dot access and
conditional concatenation using styles.editor and className (e.g., styles.editor
+ (className ? ` ${className}` : '')). Ensure you import clsx if used and remove
the unnecessary array/filter/join pattern.

style={{
border: `1px solid ${token.colorBorder}`,
borderRadius: token.borderRadius,
Expand Down
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

This file was deleted.

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,14 +79,14 @@ const Dataset = () => {
return (
<div className={styles["datasetContainer"]}>
{contextHolder}
<DatasetLabel/>
<InputLabel>Dataset</InputLabel>
<Select
Comment on lines +82 to 83
Copy link
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"] || ""}
Copy link
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.

Copy link
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}
options={(data || []).map((option) => ({label: option, value: option}))}
placeholder={"None"}
showSearch={true}
size={"large"}
size={"middle"}
Copy link
Member

Choose a reason for hiding this comment

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

Let's also set the size to "middle" in components/webui/client/src/pages/SearchPage/SearchControls/Dataset/index.tsx so they match

Copy link
Contributor Author

Choose a reason for hiding this comment

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

isnt that just this file, where it is already set?

Copy link
Member

Choose a reason for hiding this comment

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

Sorry i think i was using an older build. I confirmed that the visuals are fine in the latest commit

value={dataset}
disabled={
searchUiState === SEARCH_UI_STATE.QUERY_ID_PENDING ||
Expand Down

This file was deleted.

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
Copy link
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
Copy link
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>
<SqlInput
className={guidedGrid["noLeftBorderRadius"] || ""}
disabled={false}/>
</div>
);

export default From;
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
Copy link
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>
<Select
defaultValue={LIMIT_OPTIONS[0]?.value}
options={LIMIT_OPTIONS}
className={
`${guidedGrid["noLeftBorderRadiusSelect"]} ${
guidedGrid["widthSelect"]}`
}/>
</div>
);

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

.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
Copy link
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.

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

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

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