feat(webui): Modify time range input to show presets in date time inputs. #1676
Conversation
WalkthroughThe changes add a new TimeDateInput component to the TimeRangeInput feature with custom styling and theme support. Updates include CSS constraints for the input container, integration with ConfigProvider for theme-aware disabled state styling, new utility exports for date range positioning, and a reusable display text mapping. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
components/webui/client/src/pages/SearchPage/SearchControls/TimeRangeInput/TimeDateInput.tsx(1 hunks)components/webui/client/src/pages/SearchPage/SearchControls/TimeRangeInput/index.module.css(2 hunks)components/webui/client/src/pages/SearchPage/SearchControls/TimeRangeInput/index.tsx(4 hunks)components/webui/client/src/pages/SearchPage/SearchControls/TimeRangeInput/utils.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}
⚙️ CodeRabbit configuration file
- Prefer
false == <expression>rather than!<expression>.
Files:
components/webui/client/src/pages/SearchPage/SearchControls/TimeRangeInput/utils.tsxcomponents/webui/client/src/pages/SearchPage/SearchControls/TimeRangeInput/index.tsxcomponents/webui/client/src/pages/SearchPage/SearchControls/TimeRangeInput/TimeDateInput.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: package-image
- GitHub Check: lint-check (ubuntu-24.04)
- GitHub Check: lint-check (macos-15)
🔇 Additional comments (4)
components/webui/client/src/pages/SearchPage/SearchControls/TimeRangeInput/index.module.css (1)
5-7: Scoped ant-select min-width looks reasonableTargeting
.timeRangeInputContainer :global(.ant-select)keeps the width constraint local to this control, which should stabilise the preset dropdown without unintended global side-effects. No issues from a styling or maintainability standpoint.components/webui/client/src/pages/SearchPage/SearchControls/TimeRangeInput/index.tsx (2)
5-9: I'll help you verify this review comment about the ConfigProvider theming in the TimeRangeInput component. Let me start by examining the actual implementation.
<function_calls>
Read the TimeRangeInput component file to verify the implementation details
#!/bin/bashFirst, let's find the exact file and read it
find . -path "*SearchPage/SearchControls/TimeRangeInput/index.tsx" -type f | head -1 | xargs wc -l
</function_calls>
<function_calls>
Read the full TimeRangeInput component to see the implementation
#!/bin/bashRead the file - first check its size, then read it
FILE=$(find . -path "*SearchPage/SearchControls/TimeRangeInput/index.tsx" -type f | head -1)
if [ -n "$FILE" ]; then
cat -n "$FILE"
fi
</function_calls>
<function_calls>
Find the TimeDateInput component referenced in the review to understand the custom input wiring
#!/bin/bashLook for TimeDateInput file
find . -name "TimeDateInput*" -o -name "timeDateInput*" | head -10
</function_calls>
<function_calls>
Check the Ant Design version in package.json to verify token compatibility
#!/bin/bashFind package.json for the webui client and check antd version
find . -path "*webui/client/package.json" -type f | xargs cat | grep -A 2 -B 2 '"antd"'
</function_calls>
108-113: Ant Design DatePicker.RangePickercomponents.inputcontract is valid and documented.The
components={{ input: TimeDateInput }}pattern is a supported, documented feature in Ant Design across multiple versions (tested 4.1.1 and 5.18.3). The contract expects:
components.inputasstring | React.ComponentType<any>- Custom input component receives standard input props:
value,onChange,onFocus,onBlur,onClick,onMouseDown,onSelect, and standard DOM/ARIA handlersThe code correctly wires the custom input and drives disabled state via
timeRangeOptionchecks. EnsureTimeDateInputproperly spreads or handles the standard input props forwarded by RangePicker.components/webui/client/src/pages/SearchPage/SearchControls/TimeRangeInput/TimeDateInput.tsx (1)
1-7: Based on my verification through web search, the review comment is substantively correct. Here's what I confirmed:
- React's behavior: React warns when non-standard props are spread onto native DOM elements (confirmed by React documentation).
- The issue: If
TimeDateInputspreads all props (includingDATE_RANGE_PROP_KEY) onto a native<input>, React will trigger "unknown prop" warnings.- The fix: The proposed destructuring solution is the standard, correct pattern recommended by React to separate internal props from DOM-intended props.
- AntD contract: DatePicker's custom input should receive
React.InputHTMLAttributes<HTMLInputElement>(standard HTML input props only), not custom application-specific markers likeDATE_RANGE_PROP_KEY.The review comment's suggestion to destructure
DATE_RANGE_PROP_KEYbefore spreading is aligned with React best practices and necessary to avoid runtime warnings.Rewritten Review Comment:
Strip
date-rangemarker before spreading props to native inputThe display-text logic is solid, but spreading all AntD input props onto a native
<input>also forwards the"date-range"marker (a custom non-DOM prop) to the DOM, triggering React's "unknown prop" warning and risking future incompatibilities.Destructure
DATE_RANGE_PROP_KEYbefore spreading to keep your rendering logic intact while avoiding invalid attributes on the DOM element:-const TimeDateInput = (props: TimeDateInputProps) => { - const {value, [DATE_RANGE_PROP_KEY]: dateRange} = props; +const TimeDateInput = (props: TimeDateInputProps) => { + const { + value, + [DATE_RANGE_PROP_KEY]: dateRange, + ...restProps + } = props; @@ - return ( - <input - {...props} - readOnly={true} - value={displayText}/> - ); + return ( + <input - {...restProps} - readOnly={true} - value={displayText}/> + );This preserves your custom text rendering and event handlers while conforming to Ant Design's contract for custom input components.
⛔ Skipped due to learnings
Learnt from: hoophalab Repo: y-scope/clp PR: 1108 File: components/webui/client/src/pages/SearchPage/SearchControls/Presto/SqlQueryInput/index.tsx:15-15 Timestamp: 2025-07-18T20:00:50.288Z Learning: In the y-scope/clp React webui client codebase, for Zustand store usage: use `useStore.getState().method` for callbacks since the output is not reactive and doesn't need state as a dependency in the hook, and use `useStore((state) => state.property)` with proper selectors for reactive components that need to re-render when state changes.
| border-bottom-left-radius: 0; | ||
| min-width: 200px; | ||
| max-width: 220px; | ||
| min-width: 350px; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Verify 350px min-width on smaller layouts
Bumping the range picker min-width to 350px will improve readability but may squeeze very narrow layouts. Please sanity-check responsive breakpoints to ensure this does not cause horizontal overflow or wrapping issues in tighter containers.
🤖 Prompt for AI Agents
In
components/webui/client/src/pages/SearchPage/SearchControls/TimeRangeInput/index.module.css
around line 18, the hard-coded min-width: 350px may cause horizontal overflow on
narrow screens; change this to a responsive rule (for example apply min-width:
350px only at and above a specific breakpoint via a media query, or replace with
min-width: min(350px, 100%) / set width: 100% with a max-width: 350px) and
verify the range picker fits inside its parent by checking container padding and
breakpoints to prevent wrapping or overflow on small viewports.
| /** | ||
| * AntD date range position. | ||
| */ | ||
| enum DATE_RANGE_POSITION { | ||
| START = "start", | ||
| END = "end", | ||
| } | ||
|
|
||
| /** | ||
| * AntD RangePicker prop key for date range position. | ||
| */ | ||
| const DATE_RANGE_PROP_KEY = "date-range"; | ||
|
|
||
| /** | ||
| * Map of time range options to their display text | ||
| */ | ||
| const TIME_RANGE_DISPLAY_TEXT_MAP: Record< | ||
| TIME_RANGE_OPTION, | ||
| Record<DATE_RANGE_POSITION, string> | ||
| > = { | ||
| [TIME_RANGE_OPTION.LAST_15_MINUTES]: { | ||
| [DATE_RANGE_POSITION.START]: "15 minutes ago", | ||
| [DATE_RANGE_POSITION.END]: "Now", | ||
| }, | ||
| [TIME_RANGE_OPTION.LAST_HOUR]: { | ||
| [DATE_RANGE_POSITION.START]: "1 hour ago", | ||
| [DATE_RANGE_POSITION.END]: "Now", | ||
| }, | ||
| [TIME_RANGE_OPTION.TODAY]: { | ||
| [DATE_RANGE_POSITION.START]: "Start of today", | ||
| [DATE_RANGE_POSITION.END]: "End of today", | ||
| }, | ||
| [TIME_RANGE_OPTION.YESTERDAY]: { | ||
| [DATE_RANGE_POSITION.START]: "Start of yesterday", | ||
| [DATE_RANGE_POSITION.END]: "End of yesterday", | ||
| }, | ||
| [TIME_RANGE_OPTION.LAST_7_DAYS]: { | ||
| [DATE_RANGE_POSITION.START]: "7 days ago", | ||
| [DATE_RANGE_POSITION.END]: "Now", | ||
| }, | ||
| [TIME_RANGE_OPTION.LAST_30_DAYS]: { | ||
| [DATE_RANGE_POSITION.START]: "30 days ago", | ||
| [DATE_RANGE_POSITION.END]: "Now", | ||
| }, | ||
| [TIME_RANGE_OPTION.LAST_12_MONTHS]: { | ||
| [DATE_RANGE_POSITION.START]: "12 months ago", | ||
| [DATE_RANGE_POSITION.END]: "Now", | ||
| }, | ||
| [TIME_RANGE_OPTION.MONTH_TO_DATE]: { | ||
| [DATE_RANGE_POSITION.START]: "Start of month", | ||
| [DATE_RANGE_POSITION.END]: "Now", | ||
| }, | ||
| [TIME_RANGE_OPTION.YEAR_TO_DATE]: { | ||
| [DATE_RANGE_POSITION.START]: "Start of year", | ||
| [DATE_RANGE_POSITION.END]: "Now", | ||
| }, | ||
| [TIME_RANGE_OPTION.ALL_TIME]: { | ||
| [DATE_RANGE_POSITION.START]: "First timestamp", | ||
| [DATE_RANGE_POSITION.END]: "Last timestamp", | ||
| }, | ||
|
|
||
| // Custom option is just a placeholder for typing purposes, its values should not | ||
| // be used. | ||
| [TIME_RANGE_OPTION.CUSTOM]: { | ||
| [DATE_RANGE_POSITION.START]: "Start date", | ||
| [DATE_RANGE_POSITION.END]: "End date", | ||
| }, | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Display text mapping is consistent and well-typed
The DATE_RANGE_POSITION enum, DATE_RANGE_PROP_KEY, and TIME_RANGE_DISPLAY_TEXT_MAP are cleanly modelled: every TIME_RANGE_OPTION is covered, and the nested Record keeps START/END text in sync with the logical ranges (e.g., “7 days ago”/“Now”). This should make the UI text easy to maintain. Consider adding a small unit test tying TIME_RANGE_DISPLAY_TEXT_MAP to TIME_RANGE_OPTION_DAYJS_MAP so future edits do not drift in meaning.
Also applies to: 195-205
🤖 Prompt for AI Agents
components/webui/client/src/pages/SearchPage/SearchControls/TimeRangeInput/utils.tsx
around lines 125-192 (and similarly 195-205): add a small unit test that asserts
the TIME_RANGE_DISPLAY_TEXT_MAP covers exactly the same set of TIME_RANGE_OPTION
keys as TIME_RANGE_OPTION_DAYJS_MAP and that for any non-custom option the
START/END semantics are consistent (e.g., END maps to "Now" or "End ..." when
the corresponding DAYJS map end is a "now" or end-of-period sentinel); implement
the test by importing both maps/enums, comparing Object.keys(...) equality, and
spot-checking a couple representative options for expected START/END text
patterns, failing the test if any drift is detected.
| .timeRangeInputContainer :global(.ant-select) { | ||
| min-width: 150px; | ||
| } | ||
|
|
There was a problem hiding this comment.
this style isn't actually used?
There was a problem hiding this comment.
its used. It forces the size of the select for timestamp presets
|
@hoophalab for small size, not sure we really have a good alternative at the moment. It will always eventually overflow. Maybe I can find a way to overflow on a new second line, but not sure. Even if i overflow to second line. it will still overflow.. |
2 participants
Description
Modifies time range input to display placeholder text for presets instead of date pickers disappearing. Change was due for a preference for fixed width components.
Antd allows for devs to replace their date picker input with a custom input. Custom input can display strings instead of date. Feature is not well documented, or typed, so had to use a custom type for its props.
Checklist
breaking change.
Validation performed
strings displayed
Summary by CodeRabbit
Release Notes
New Features
Style
✏️ Tip: You can customize this high-level summary in your review settings.