Skip to content

feat(webui): Move run button under SQL query input; Modify SQL editor theming to look like AntD input. #1248

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Aug 27, 2025
Merged

feat(webui): Move run button under SQL query input; Modify SQL editor theming to look like AntD input. #1248

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
3188d90
latest
Aug 21, 2025
d9f2b74
latest
Aug 21, 2025
c5f23d5
latest
Aug 21, 2025
a810e09
latest
Aug 26, 2025
9dca18f
latest
Aug 27, 2025
8cf4126
latest
Aug 27, 2025
b09c0ca
latest
Aug 27, 2025
4e3e12b
latest
Aug 27, 2025
a1f3f44
latest
Aug 27, 2025
b7d6f9e
Merge branch 'main' into moveQuery
hoophalab Aug 27, 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
4 changes: 2 additions & 2 deletions .../SearchPage/SearchQueryStatus/Results.tsx → ...ge/SearchControls/QueryStatus/Results.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ import {
Typography,
} from "antd";

import useSearchStore from "../SearchState/index";
import {SEARCH_UI_STATE} from "../SearchState/typings";
import useSearchStore from "../../SearchState/index";
import {SEARCH_UI_STATE} from "../../SearchState/typings";


const {Text} = Typography;
Expand Down
3 changes: 2 additions & 1 deletion ...chPage/SearchQueryStatus/index.module.css → ...archControls/QueryStatus/index.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
.status {
margin-left: 2px;
margin-top: 4px;
padding-top: 4px;
margin-bottom: -4px;
Copy link
Contributor

@coderabbitai coderabbitai bot Aug 21, 2025

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Avoid negative margins; prefer layout alignment over spacing hacks

Using a negative bottom margin to visually align text with the button can cause overlap with adjacent clickable areas and unpredictable wrapping. Consider aligning via flex and baseline instead of padding/margin tweaks.

Apply this diff to improve robustness:

 .status {
-  margin-left: 2px;
-  padding-top: 4px;
-  margin-bottom: -4px;
+  margin-left: 2px;
+  padding-top: 4px;
+  margin-bottom: 0;
+  display: inline-flex;
+  align-items: baseline; /* aligns number with button text baseline */
+  gap: 4px;
 }
📝 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
padding-top: 4px;
margin-bottom: -4px;
.status {
margin-left: 2px;
padding-top: 4px;
margin-bottom: 0;
display: inline-flex;
align-items: baseline; /* aligns number with button text baseline */
gap: 4px;
}
🤖 Prompt for AI Agents 
In
components/webui/client/src/pages/SearchPage/SearchControls/QueryStatus/index.module.css
around lines 3-4, replace the negative bottom margin and extra padding with a
layout-based solution: remove margin-bottom: -4px; and adjust padding-top as
needed, then ensure the parent/container uses display:flex with
align-items:baseline (or center) and appropriate gap or padding to vertically
align the text with the button; this eliminates the negative margin hack and
prevents overlap while keeping visual alignment.

}

.badge {
Expand Down
8 changes: 4 additions & 4 deletions ...es/SearchPage/SearchQueryStatus/index.tsx → ...Page/SearchControls/QueryStatus/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import {Typography} from "antd";

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

Expand All @@ -13,7 +13,7 @@ const {Text} = Typography;
*
* @return
*/
const SearchQueryStatus = () => {
const QueryStatus = () => {
const {
searchJobId,
searchUiState,
Expand All @@ -39,4 +39,4 @@ const SearchQueryStatus = () => {
};


export default SearchQueryStatus;
export default QueryStatus;
13 changes: 13 additions & 0 deletions components/webui/client/src/pages/SearchPage/SearchControls/index.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,16 @@
display: flex;
gap: 10px;
}

.prestoSearchControlsContainer {
display: flex;
flex-direction: column;
gap: 10px;
}

.buttonAndStatusRow {
display: flex;
align-items: flex-end;
gap: 10px;
}

34 changes: 21 additions & 13 deletions components/webui/client/src/pages/SearchPage/SearchControls/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import styles from "./index.module.css";
import SqlQueryInput from "./Presto/SqlQueryInput";
import SqlSearchButton from "./Presto/SqlSearchButton";
import QueryInput from "./QueryInput";
import QueryStatus from "./QueryStatus";
import SearchButton from "./SearchButton";
import TimeRangeInput from "./TimeRangeInput";

Expand All @@ -23,30 +24,37 @@ const handleSubmit = (ev: React.FormEvent<HTMLFormElement>) => {
};

/**
* Renders controls for submitting queries.
* Renders controls for submitting queries and the query status.
*
* @return
*/
const SearchControls = () => {
return (
<form onSubmit={handleSubmit}>
<div className={styles["searchControlsContainer"]}>
{SETTINGS_QUERY_ENGINE !== CLP_QUERY_ENGINES.PRESTO ?
(
<>
{SETTINGS_QUERY_ENGINE !== CLP_QUERY_ENGINES.PRESTO ?
(
<div>
<div className={styles["searchControlsContainer"]}>
{CLP_STORAGE_ENGINES.CLP_S === SETTINGS_STORAGE_ENGINE && <Dataset/>}
<QueryInput/>
<TimeRangeInput/>
<SearchButton/>
</>
) :
(
<>
<SqlQueryInput/>
</div>
<div className={styles["statusRow"]}>
<QueryStatus/>
</div>
Copy link
Contributor

@hoophalab hoophalab Aug 26, 2025

Choose a reason for hiding this comment

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

Suggested change
<div className={styles["statusRow"]}>
<QueryStatus/>
</div>
<div className={styles["buttonAndStatusRow"]}>
<QueryStatus/>
</div>

</div>
) :
(

<div className={styles["prestoSearchControlsContainer"]}>
<SqlQueryInput/>
<div className={styles["buttonAndStatusRow"]}>
<SqlSearchButton/>
</>
)}
</div>
<QueryStatus/>
</div>
</div>
)}
</form>
);
};
Expand Down
6 changes: 1 addition & 5 deletions components/webui/client/src/pages/SearchPage/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import {
import styles from "./index.module.css";
import {ProgressBar} from "./Presto/ProgressBar";
import SearchControls from "./SearchControls";
import SearchQueryStatus from "./SearchQueryStatus";
import SearchResultsTable from "./SearchResults/SearchResultsTable";
import SearchResultsTimeline from "./SearchResults/SearchResultsTimeline";
import {useUpdateStateWithMetadata} from "./SearchState/useUpdateStateWithMetadata";
Expand All @@ -23,10 +22,7 @@ const SearchPage = () => {
<>
{SETTINGS_QUERY_ENGINE === CLP_QUERY_ENGINES.PRESTO && <ProgressBar/>}
<div className={styles["searchPageContainer"]}>
<div>
<SearchControls/>
<SearchQueryStatus/>
</div>
<SearchControls/>
{SETTINGS_QUERY_ENGINE !== CLP_QUERY_ENGINES.PRESTO && <SearchResultsTimeline/>}
<SearchResultsTable/>
Comment on lines 23 to 27
Copy link
Contributor

@coderabbitai coderabbitai bot Aug 26, 2025

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Minor readability: factor out the engine check to avoid repetition.

Extracting a local isPresto flag improves readability and keeps the engine condition consistent across the component.

   useUpdateStateWithMetadata();

-  return (
+  const isPresto = SETTINGS_QUERY_ENGINE === CLP_QUERY_ENGINES.PRESTO;
+  return (
     <>
-      {SETTINGS_QUERY_ENGINE === CLP_QUERY_ENGINES.PRESTO && <ProgressBar/>}
+      {isPresto && <ProgressBar/>}
       <div className={styles["searchPageContainer"]}>
         <SearchControls/>
-        {SETTINGS_QUERY_ENGINE !== CLP_QUERY_ENGINES.PRESTO && <SearchResultsTimeline/>}
+        {false == isPresto && <SearchResultsTimeline/>}
         <SearchResultsTable/>
       </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
{SETTINGS_QUERY_ENGINE === CLP_QUERY_ENGINES.PRESTO && <ProgressBar/>}
<div className={styles["searchPageContainer"]}>
<div>
<SearchControls/>
<SearchQueryStatus/>
</div>
<SearchControls/>
{SETTINGS_QUERY_ENGINE !== CLP_QUERY_ENGINES.PRESTO && <SearchResultsTimeline/>}
<SearchResultsTable/>
useUpdateStateWithMetadata();
const isPresto = SETTINGS_QUERY_ENGINE === CLP_QUERY_ENGINES.PRESTO;
return (
<>
{isPresto && <ProgressBar/>}
<div className={styles["searchPageContainer"]}>
<SearchControls/>
{false == isPresto && <SearchResultsTimeline/>}
<SearchResultsTable/>
</div>
</>
);
🤖 Prompt for AI Agents 
In components/webui/client/src/pages/SearchPage/index.tsx around lines 23 to 27,
the code repeats the engine check SETTINGS_QUERY_ENGINE ===
CLP_QUERY_ENGINES.PRESTO in multiple JSX spots; introduce a local boolean const
(e.g. isPresto) at the top of the component by evaluating that expression once,
then replace the repeated checks with the isPresto variable so the JSX reads
{isPresto && <ProgressBar/>} and {!isPresto && <SearchResultsTimeline/>},
improving readability and ensuring a single source of truth for the engine
condition.

</div>
Expand Down