feat(new-webui): Add a progress bar for search.#891
Conversation
WalkthroughA new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SearchControls
participant QueryBox
participant AntdInput
User->>SearchControls: Interact with search input
SearchControls->>QueryBox: Render with value, onChange, progress=null
QueryBox->>AntdInput: Render Input with passed props
QueryBox-->>SearchControls: (Optional) Display progress bar if progress is set
Suggested reviewers
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
visualization looks good to me. i discussed with @hoophalab offline that we might not want to maintain the progress percentage visualization logic on our own. @hoophalab will quickly experiment integration with https://ant.design/components/progress for future extensibility. if it's hard to integrate, maintaining only an input box with simple progress display seems also reasonable to me. @davemarco what do you think?
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
I integrated antd's |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
junhaoliao
left a comment
There was a problem hiding this comment.
thanks for getting it done quickly and it looks nice. i posted some stylish comments
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
2267351 to
6d5604e
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
@junhaoliao ready for your final review |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
components/log-viewer-webui/client/src/components/QueryBox/index.module.css (1)
12-22: 🧹 Nitpick (assertive)Consider moving CSS variable to root level.
The
--antd-border-radiusvariable is defined within the.queryBoxWrapperclass but would be more maintainable if defined at a higher level.+:root { + --antd-border-radius: 8px; +} .queryBoxWrapper { /* Prevents the progress bar from obstructing the input box. */ pointer-events: none; width: 100%; height: 100%; position: absolute; top: 0; overflow: hidden; border-radius: var(--antd-border-radius); - --antd-border-radius: 8px; }components/log-viewer-webui/client/src/components/QueryBox/index.tsx (2)
15-22: 🧹 Nitpick (assertive)Improve JSDoc return type documentation.
The JSDoc comment is missing a return type description.
/** * Renders an Input with a progress bar. * * @param props * @param props.progress * @param props.rest - * @return + * @return {JSX.Element} The rendered QueryBox component with input and progress bar */
23-42: 🧹 Nitpick (assertive)Consider using dot notation for CSS modules.
While bracket notation works, dot notation is more common and concise for accessing CSS module classes.
return ( - <div className={styles["queryBox"]}> - <div className={styles["queryBoxPosition"]}> + <div className={styles.queryBox}> + <div className={styles.queryBoxPosition}> <Input {...rest}/> - <div className={styles["queryBoxWrapper"]}> + <div className={styles.queryBoxWrapper}> <Progress - className={styles["queryBoxProgress"] || ""} + className={styles.queryBoxProgress} percent={progress ?? 0} showInfo={false} size={"small"} strokeLinecap={"butt"} style={{display: null === progress ? "none" : "block"}}/> </div> </div> </div> );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
components/log-viewer-webui/client/src/components/QueryBox/index.module.css(1 hunks)components/log-viewer-webui/client/src/components/QueryBox/index.tsx(1 hunks)components/log-viewer-webui/client/src/pages/IngestPage/Jobs/index.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}`: - Prefer `false == ` rather than `!`.
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}: - Preferfalse == <expression>rather than!<expression>.
components/log-viewer-webui/client/src/pages/IngestPage/Jobs/index.tsxcomponents/log-viewer-webui/client/src/components/QueryBox/index.tsx
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: lint-check (ubuntu-latest)
🔇 Additional comments (5)
components/log-viewer-webui/client/src/pages/IngestPage/Jobs/index.tsx (1)
79-80: JSDoc documentation improved with parameter details.The JSDoc comment has been enhanced with proper parameter documentation, which improves code readability and IDE autocompletion support.
components/log-viewer-webui/client/src/components/QueryBox/index.module.css (3)
1-3: Base styling for QueryBox looks good.The width styling provides appropriate flexibility for the component to adapt to its container.
5-10: Appropriate positioning context for absolute elements.The positioning context is correctly set up with a descriptive comment explaining its purpose.
24-27: Progress bar positioning looks good.The absolute positioning of the progress bar at the bottom of the container is appropriate for the UI design.
components/log-viewer-webui/client/src/components/QueryBox/index.tsx (1)
10-13: Well-documented prop interface with clear comments.The QueryBoxProps interface properly extends InputProps and includes a descriptive JSDoc comment for the progress property.
…ex.module.css Co-authored-by: Junhao Liao <junhao@junhao.ca>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
components/log-viewer-webui/client/src/components/QueryBox/index.module.css (1)
20-21: Consider centralizing the CSS variable.Defining
--antd-border-radiuslocally limits reuse. You can move it to:rootfor consistency across components:+:root { + --antd-border-radius: 8px; +} .queryBoxWrapper { - border-radius: var(--antd-border-radius); - --antd-border-radius: 8px; + border-radius: var(--antd-border-radius); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
components/log-viewer-webui/client/src/components/QueryBox/index.module.css(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: lint-check (ubuntu-latest)
🔇 Additional comments (2)
components/log-viewer-webui/client/src/components/QueryBox/index.module.css (2)
1-3: Approve.queryBoxcontainer styling.The block correctly sets full width for the input wrapper and follows the module’s naming conventions.
5-10: Approve positioning context in.queryBoxPosition.Using
position: relativewith full width/height and a clear doc comment establishes the necessary stacking context for absolutely-positioned children.
| .queryBoxProgress { | ||
| position: absolute; | ||
| bottom: -8px; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Ensure the progress bar spans full width.
By default, an absolutely positioned element without left and explicit width may not fill its container. Consider adding:
.queryBoxProgress {
position: absolute;
bottom: -8px;
+ left: 0;
+ width: 100%;
}📝 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.
| .queryBoxProgress { | |
| position: absolute; | |
| bottom: -8px; | |
| } | |
| .queryBoxProgress { | |
| position: absolute; | |
| bottom: -8px; | |
| left: 0; | |
| width: 100%; | |
| } |
| .queryBoxWrapper { | ||
| /* Prevents the progress bar from obstructing the input box. */ | ||
| pointer-events: none; | ||
| width: 100%; | ||
| height: 100%; | ||
| position: absolute; | ||
| top: 0; | ||
| overflow: hidden; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ensure the wrapper covers the full input area.
Without a left: 0, the absolutely positioned .queryBoxWrapper may not align correctly over the underlying input. Please add left: 0 to guarantee full coverage:
.queryBoxWrapper {
/* Prevents the progress bar from obstructing the input box. */
pointer-events: none;
width: 100%;
height: 100%;
position: absolute;
top: 0;
+ left: 0;
overflow: hidden;
border-radius: var(--antd-border-radius);
--antd-border-radius: 8px;
}📝 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.
| .queryBoxWrapper { | |
| /* Prevents the progress bar from obstructing the input box. */ | |
| pointer-events: none; | |
| width: 100%; | |
| height: 100%; | |
| position: absolute; | |
| top: 0; | |
| overflow: hidden; | |
| .queryBoxWrapper { | |
| /* Prevents the progress bar from obstructing the input box. */ | |
| pointer-events: none; | |
| width: 100%; | |
| height: 100%; | |
| position: absolute; | |
| top: 0; | |
| left: 0; | |
| overflow: hidden; | |
| border-radius: var(--antd-border-radius); | |
| --antd-border-radius: 8px; | |
| } |
Description
QueryBoxcomponent, which wraps an antdInputwith a progress bar at the bottom.InputinSearchControlswithQueryBox.Since server features are not merged yet,
SearchControlsdefaults to hide the progress bar.Checklist
breaking change.
Validation performed
QueryBoxlooks ok.As storybook isn't merged, check this branch to play with
QueryBoxin storybook.Summary by CodeRabbit
New Features
Style