[LG-5504] feat(input-box): add input-box package with utility functions for date/time input handling #3265
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
Size Change: 0 B Total Size: 1.61 MB ℹ️ View Unchanged
|
shaneeza
requested review from
a team,
TheSonOfThomp,
stephl3 and
tsck
and removed request for
a team and
tsck
stephl3
reviewed
Oct 30, 2025
Comment on lines
+1
to
+11
| export { | ||
| createExplicitSegmentValidator, | ||
| type ExplicitSegmentRule, | ||
| isElementInputSegment, | ||
| isValidValueForSegment, | ||
| } from './utils'; | ||
| export { getValueFormatter } from './utils/getValueFormatter/getValueFormatter'; | ||
| export { | ||
| isValidSegmentName, | ||
| isValidSegmentValue, | ||
| } from './utils/isValidSegment/isValidSegment'; |
There was a problem hiding this comment.
should these go in the date-utils package?
There was a problem hiding this comment.
I don't believe so. date-utils are for Date manipulation and comparison utils not included in the date-fns library. These are more implementation-specific utils that used to live in date-picker/src/shared/utils.
Comment on lines
+1
to
+17
| export { | ||
| createExplicitSegmentValidator, | ||
| ExplicitSegmentRule, | ||
| } from './createExplicitSegmentValidator/createExplicitSegmentValidator'; | ||
| export { getNewSegmentValueFromArrowKeyPress } from './getNewSegmentValueFromArrowKeyPress/getNewSegmentValueFromArrowKeyPress'; | ||
| export { getNewSegmentValueFromInputValue } from './getNewSegmentValueFromInputValue/getNewSegmentValueFromInputValue'; | ||
| export { | ||
| getRelativeSegment, | ||
| getRelativeSegmentRef, | ||
| } from './getRelativeSegment/getRelativeSegment'; | ||
| export { getValueFormatter } from './getValueFormatter/getValueFormatter'; | ||
| export { isElementInputSegment } from './isElementInputSegment/isElementInputSegment'; | ||
| export { | ||
| isValidSegmentName, | ||
| isValidSegmentValue, | ||
| } from './isValidSegment/isValidSegment'; | ||
| export { isValidValueForSegment } from './isValidValueForSegment/isValidValueForSegment'; |
There was a problem hiding this comment.
do these all need to be exported in the package index file? I see several missing from there
There was a problem hiding this comment.
Yeah. Some of these will be exported and used in both DatePicker and TimeInput.
Comment on lines
3
to
7
| const segmentObj = { | ||
| Day: 'day', | ||
| Month: 'month', | ||
| Year: 'year', | ||
| } as const; |
There was a problem hiding this comment.
Appreciate the net-new tests across all these utils! General feedback: now that these utils have been made more generic to support time as well, it would be helpful for coverage + documentation if these also tested time-related segments as well
There was a problem hiding this comment.
Thoughts on the time-related segments I added here? The segments are gonna be very similar, so I added them to the same object.
Comment on lines
22
to
30
| * @example | ||
| * const segmentObj = { | ||
| * Day: 'day', | ||
| * Month: 'month', | ||
| * Year: 'year', | ||
| * }; | ||
| * const rules = { | ||
| * day: { maxChars: 2, minExplicitValue: 1 }, | ||
| * month: { maxChars: 2, minExplicitValue: 1 }, |
There was a problem hiding this comment.
I think the example is cut off here
Comment on lines
32
to
34
| export function createExplicitSegmentValidator< | ||
| T extends Record<string, string>, | ||
| >(segmentEnum: T, rules: Record<T[keyof T], ExplicitSegmentRule>) { |
There was a problem hiding this comment.
General feedback: most, if not all, these utils look like they could benefit from using a single object arg to make their call sites more readable:
Less readable:
createExplicitSegmentValidator(foo, bar)
More readable:
createExplicitSegmentValidator(segmentEnum: foo, rules: bar)
There was a problem hiding this comment.
@stephl3 Did you mean createExplicitSegmentValidator({segmentEnum: foo, rules: bar})?
| } from '../isValidSegment/isValidSegment'; | ||
|
|
||
| /** | ||
| * Configuration for determining if a segment value is explicit |
There was a problem hiding this comment.
what does it mean for a segment value to be "explicit"?
There was a problem hiding this comment.
Updated the docs
| * @param min - The minimum value for the segment | ||
| * @param max - The maximum value for the segment | ||
| * @param step - The step value for the arrow keys | ||
| * @param shouldNotRollover - The segments that should not rollover |
There was a problem hiding this comment.
nit: this param threw me off and the term "rollover" doesn't seem quite right although I see it's because the lib function is called rollover. I think something like shouldWrap or shouldCycle might be more appropriate; curious what your thoughts are?
Also, is there a particular reason for using a negative boolean vs a positive? i.e. should* vs shouldNot*
There was a problem hiding this comment.
Updated to shouldWrap!
| * month: { maxChars: 2, minExplicitValue: 1 }, | ||
| */ | ||
| export function createExplicitSegmentValidator< | ||
| T extends Record<string, string>, |
There was a problem hiding this comment.
General feedback: it'd be easier to read and understand these type generics if they were written out as specific words beyond T and V
Comment on lines
10
to
11
| * @param allowsZero - | ||
| * @param val - the value to format |
There was a problem hiding this comment.
allowZero* missing description?
should val be removed here?
There was a problem hiding this comment.
This is still needed. Why should it be removed?
| * @param val - the value to format | ||
| * @returns a value formatter function for the provided segment | ||
| * | ||
| * @example |
There was a problem hiding this comment.
if including examples, maybe one where allowZero is true to show how it differs?
adamrasheed
reviewed
Oct 30, 2025
| @@ -0,0 +1,97 @@ | |||
| import { createExplicitSegmentValidator } from './createExplicitSegmentValidator'; | |||
|
|
|||
| const segmentObj = { | |||
There was a problem hiding this comment.
should we rename this to something like dateSegmentObj? Might be a result of my un-familiarty with the input-box component, but just look at it i don't know right away what segment means
There was a problem hiding this comment.
I updated this example to include a few more items in the object, just to show a few more segments, so dateSegmentObj might not be the best name, but I updated the description for this prop in the TSDoc. Did that help?
…es for improved type safety and consistency
…r with detailed explanations and examples
…larity in segment value handling
…idator with corresponding validation rules and tests
…in segment value handling
…reateExplicitSegmentValidator
…ent and updating validation rules for day and year segments
…lues from 1970 to 2038 and add custom validation for specific range
adamrasheed
approved these changes
Nov 3, 2025
stephl3
approved these changes
Nov 4, 2025
| * otherwise, pad the string with 0s, or trim it to n chars | ||
| * | ||
| * @param charsPerSegment - the number of characters per segment | ||
| * @param allowsZero - whether to allow zero-like values |
There was a problem hiding this comment.
Suggested change
| * @param allowsZero - whether to allow zero-like values | |
| * @param allowZero - whether to allow zero-like values |
| * | ||
| * @param charsPerSegment - the number of characters per segment | ||
| * @param allowsZero - whether to allow zero-like values | ||
| * @param val - the value to format |
There was a problem hiding this comment.
Still seems odd that @param val makes it seem like it's a param of getValueFormatter when it should instead be a param of the value format function returned by getValueFormatter
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
✍️ Proposed changes
This PR introduces a new internal
@leafygreen-ui/input-boxpackage that provides shared utility functions for handling input segments in date and time components. These utilities were extracted from thedatepickerpackage and refactored to be generic and reusable across multiple components. The package includes utilities for segment validation, value formatting, keyboard navigation, and segment state management. This package is designed to be used internally by components likeDatePickerandTimeInputto provide consistent input handling behavior.Key utilities added:
createExplicitSegmentValidator- Creates validators for specific segment typesgetNewSegmentValueFromArrowKeyPress- Handles arrow key navigation logicgetNewSegmentValueFromInputValue- Processes user input for segmentsgetRelativeSegment- Manages segment focus and navigationgetValueFormatter- Formats segment values for displayisElementInputSegment- Validates if an element is an input segmentisValidSegment- Validates segment names and valuesisValidValueForSegment- Validates if a value is valid for a specific segment🎟️ Jira ticket: LG-5504
✅ Checklist
For new components
For bug fixes, new features & breaking changes
pnpm changesetand documented my changes