[LG-5504] feat(input-box): add InputBox
#3285
Open
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
…w features and documentation.
|
shaneeza
changed the title
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new internal package @leafygreen-ui/input-box that extracts and generalizes input segment functionality from the date-picker package. It provides reusable InputBox and InputSegment components for building date/time inputs with segmented fields (day, month, year, etc.).
Key Changes:
- Created generic
InputBoxcomponent for rendering segmented inputs with auto-formatting and keyboard navigation - Created
InputSegmentcomponent for individual input fields with validation and arrow key support - Added comprehensive test utilities and test coverage for both components
- Includes Storybook stories for visual documentation
Reviewed Changes
Copilot reviewed 23 out of 24 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
packages/input-box/tsconfig.json |
Added @leafygreen-ui/a11y package reference |
packages/input-box/package.json |
Added @leafygreen-ui/a11y dependency |
packages/input-box/src/utils/createExplicitSegmentValidator/createExplicitSegmentValidator.ts |
Updated function signature to accept allowZero parameter |
packages/input-box/src/testutils/testutils.mocks.ts |
Added mock data and utilities for testing |
packages/input-box/src/testutils/index.tsx |
Created test helper components and rendering utilities |
packages/input-box/src/index.ts |
Added exports for InputBox, InputSegment, and InputBoxContext |
packages/input-box/src/InputSegment/index.ts |
Created InputSegment exports |
packages/input-box/src/InputSegment/InputSegment.types.ts |
Defined TypeScript types for InputSegment |
packages/input-box/src/InputSegment/InputSegment.tsx |
Implemented InputSegment component logic |
packages/input-box/src/InputSegment/InputSegment.styles.ts |
Added styling for InputSegment |
packages/input-box/src/InputSegment/InputSegment.stories.tsx |
Created Storybook stories for InputSegment |
packages/input-box/src/InputSegment/InputSegment.spec.tsx |
Added comprehensive tests for InputSegment |
packages/input-box/src/InputBoxContext/index.ts |
Created InputBoxContext exports |
packages/input-box/src/InputBoxContext/InputBoxContext.types.ts |
Defined context types |
packages/input-box/src/InputBoxContext/InputBoxContext.tsx |
Implemented React context for sharing state |
packages/input-box/src/InputBoxContext/InputBoxContext.spec.tsx |
Added tests for context |
packages/input-box/src/InputBox/index.ts |
Created InputBox exports |
packages/input-box/src/InputBox/InputBox.types.ts |
Defined InputBox TypeScript types |
packages/input-box/src/InputBox/InputBox.tsx |
Implemented InputBox component logic |
packages/input-box/src/InputBox/InputBox.styles.ts |
Added styling for InputBox |
packages/input-box/src/InputBox/InputBox.spec.tsx |
Added comprehensive tests for InputBox |
packages/input-box/src/InputBox.stories.tsx |
Created Storybook stories for InputBox |
packages/input-box/README.md |
Created comprehensive documentation |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comment on lines
30
to
34
| * @param segment - The segment to validate | ||
| * @param value - The value to validate | ||
| * @param allowZero - Whether to allow zero values | ||
| * | ||
| * @example |
There was a problem hiding this comment.
The documentation structure is incorrect. The @param tags (lines 30-32) should not be nested under the @returns tag (line 28). These @param tags should document the returned function's parameters, but they're currently placed at the wrong indentation level. Move lines 30-32 to appear after line 34 (after the closing of the @returns description) so they document the returned function correctly.
Suggested change
| * @param segment - The segment to validate | |
| * @param value - The value to validate | |
| * @param allowZero - Whether to allow zero values | |
| * | |
| * @example | |
| * @example | |
| * @param segment - The segment to validate | |
| * @param value - The value to validate | |
| * @param allowZero - Whether to allow zero values |
Collaborator
Author
There was a problem hiding this comment.
It doesn't seem correct to add the params under the example.
Collaborator
There was a problem hiding this comment.
segment value and allowZero aren't function params, so they can probably be removed
Collaborator
There was a problem hiding this comment.
Oh I see, they're params of the returned function. Not sure how to do that in TSDoc
Contributor
|
Size Change: +8.67 kB (+0.48%) Total Size: 1.8 MB
ℹ️ View Unchanged
|
shaneeza
requested review from
a team,
TheSonOfThomp and
stephl3
and removed request for
a team
adamrasheed
reviewed
Nov 6, 2025
…r InputSegment styles for improved class handling
stephl3
reviewed
Nov 7, 2025
Collaborator
stephl3
left a comment
stephl3
left a comment
There was a problem hiding this comment.
Hi @shaneeza thanks for your hard work on this! 3000+ lines is quite a lot to review, and I worry about not being able to provide a quality review at this size. Can we break this down into smaller PRs to review/approve faster and more safely? I'm thinking a good way to split this up could be:
- InputBoxContext
- InputBox
- InputSegment
Also, what do you think about a more explicit naming pattern of DateTimeInputBox, DateTimeInputSegment, etc? Despite the verbosity, I think the clarity of a more explicit name could help differentiate from the existing InputOption
Collaborator
Author
I'm not opposed to this. Thinking about it, InputBox is ambiguous and can cause confusion. |
…associated types and tests
…en-ui into LG-5504/input-box-component
shaneeza
changed the base branch from
shaneeza/time-picker-integration
to
LG-5504/input-box-context
…egment component references
This was referenced Nov 7, 2025
…reen-ui into LG-5504/input-box-component
… and update InputSegment story with segmentEnum
stephl3
reviewed
Nov 13, 2025
Comment on lines
3
to
6
| export { | ||
| type InputSegmentChangeEventHandler, | ||
| isInputSegment, | ||
| } from './shared.types'; |
Collaborator
There was a problem hiding this comment.
I see in the README that Size is imported from the tokens package for usage in InputSegment. Do we want to pass through Size as a transitive export? Similar to the work from #3302
| const isNumber = Number(key) && key !== keyMap.Space; | ||
| // If the value is a number, we check if the input is full and reset it if it is. The number will be inserted into the input when onChange is called. | ||
| // This is to handle the case where the user tries to type a number when the input is already full. Usually this happens when the focus is moved to the next segment or a segment is clicked | ||
| const isNumber = /^[0-9]$/.test(key); |
Collaborator
There was a problem hiding this comment.
nit: isSingleDigitNumber might be a more explicit name here. Makes me think this could be a util
Collaborator
There was a problem hiding this comment.
Or just isSingleDigit
Collaborator
Author
There was a problem hiding this comment.
updated and created a util
| * Generic controlled input box component | ||
| * Renders an input box with appropriate segment order & separator characters. | ||
| * | ||
| * @internal |
Collaborator
There was a problem hiding this comment.
from what I understand, this is the "internal" component that is wrapped by forwardRef.
But to that point, this probably doesn't need to be exported, does it? (Also, we should probably rename it to make it clearer what's going on)
Collaborator
Author
There was a problem hiding this comment.
Do you have a name in mind?
Comment on lines
+60
to
+64
| export const LiveExample: StoryFn<typeof InputBox> = props => { | ||
| return ( | ||
| <InputBoxWithState {...(props as Partial<InputBoxProps<SegmentObjMock>>)} /> | ||
| ); | ||
| }; |
Collaborator
There was a problem hiding this comment.
similar to InputSegment, should we disable this snapshot and include a generated snapshot with some different combos?
Collaborator
There was a problem hiding this comment.
I won't block on it, but I think new stories pair well with their related component code similar to when adding specs with their related code
Collaborator
Author
There was a problem hiding this comment.
I missed this comment, but I added it!
|
|
||
| export const separatorLiteralDisabledStyles: Record<Theme, string> = { | ||
| [Theme.Dark]: css` | ||
| color: ${palette.gray.dark2}; |
Collaborator
There was a problem hiding this comment.
do we have a color token for this?
Collaborator
Author
There was a problem hiding this comment.
yeah, this has been updated already
|
|
||
| case keyMap.ArrowUp: | ||
| case keyMap.ArrowDown: { | ||
| // increment/decrement logic implemented by DateInputSegment |
Collaborator
There was a problem hiding this comment.
Suggested change
| // increment/decrement logic implemented by DateInputSegment | |
| // increment/decrement logic implemented by InputSegment |
Comment on lines
+91
to
+98
| /** | ||
| * The component that renders a segment. When mapping over the formatParts, we will render the segment component for each part using this component. | ||
| * This should be a React component that accepts the InputSegmentComponentProps<Segment> type. | ||
| * | ||
| * @example | ||
| * segmentComponent={DateInputSegment} | ||
| */ | ||
| segmentComponent: React.ComponentType<InputSegmentComponentProps<Segment>>; |
Collaborator
There was a problem hiding this comment.
Should this be optional, with a default of InputSegment?
Collaborator
Author
There was a problem hiding this comment.
Currently, I don't pass the min or max values to the segment wrapper component from InputBox. However, those are required in InputSegment, and I don't want to set defaults for those. My thinking was that the segment wrapper component should determine and pass the min/max values to InputSegment, because, for example, with DatePicker, the min and max days depend on the current date value.
| * @example | ||
| * { day: ref, month: ref, year: ref } | ||
| */ | ||
| segmentRefs: Record<Segment, React.RefObject<HTMLInputElement>>; |
Collaborator
Author
There was a problem hiding this comment.
Thinking about this a little more, I should be able to create the refs directly in InputBox. I'll test this out.
Collaborator
Author
There was a problem hiding this comment.
ok updated so refs are optional
Comment on lines
64
to
89
| /** | ||
| * The number of characters per segment | ||
| * | ||
| * @example | ||
| * { day: 2, month: 2, year: 4 } | ||
| */ | ||
| charsPerSegment: Record<Segment, number>; | ||
|
|
||
| /** | ||
| * An object that maps the segment names to their rules. | ||
| * | ||
| * maxChars: the maximum number of characters for the segment | ||
| * minExplicitValue: the minimum explicit value for the segment | ||
| * | ||
| * @example | ||
| * { | ||
| * day: { maxChars: 2, minExplicitValue: 1 }, | ||
| * month: { maxChars: 2, minExplicitValue: 4 }, | ||
| * year: { maxChars: 4, minExplicitValue: 1970 }, | ||
| * } | ||
| * | ||
| * Explicit: Day = 5, 02 | ||
| * Ambiguous: Day = 2 (could be 20-29) | ||
| * | ||
| */ | ||
| segmentRules: Record<Segment, ExplicitSegmentRule>; |
Collaborator
There was a problem hiding this comment.
I wonder if we can just merge the charsPerSegment into the ExplicitSegmentRule type
Collaborator
Author
There was a problem hiding this comment.
or maybe, we don't even need charsPerSegment. I can grab those from the segment rules.
Collaborator
Author
There was a problem hiding this comment.
I ended up getting rid of charsPerSegment and calculating the char count from the segment rules
| const isNumber = Number(key) && key !== keyMap.Space; | ||
| // If the value is a number, we check if the input is full and reset it if it is. The number will be inserted into the input when onChange is called. | ||
| // This is to handle the case where the user tries to type a number when the input is already full. Usually this happens when the focus is moved to the next segment or a segment is clicked | ||
| const isNumber = /^[0-9]$/.test(key); |
Collaborator
There was a problem hiding this comment.
Or just isSingleDigit
Comment on lines
-98
to
-99
| // if the value length is equal to the maxLength, reset the input. This will clear the input and the number will be inserted into the input when onChange is called. | ||
|
|
Collaborator
There was a problem hiding this comment.
Why rm? I think this is a good comment to keep
Collaborator
Author
There was a problem hiding this comment.
I moved the comment above
Base automatically changed from
LG-5504/input-box-segment
to
shaneeza/time-picker-integration
November 14, 2025 02:01
…eafygreen-ui into LG-5504/input-box-component
…y in digit input handling
…onsible for increment/decrement logic
…clarify functionality and usage
…nt components for enhanced customization
…nt usage across components
…mentRefs integration for improved focus handling
shaneeza
commented
Nov 14, 2025
| minSegmentValue={minValues[segment]} | ||
| maxSegmentValue={maxValues[segment]} | ||
| charsPerSegment={charsPerSegment[segment]} | ||
| size={Size.Default} |
Collaborator
Author
There was a problem hiding this comment.
This needs to be updated
stephl3
approved these changes
Nov 17, 2025
Comment on lines
+1
to
+7
| /** | ||
| * Checks if the key is a single digit. | ||
| * | ||
| * @param key - The key to check. | ||
| * @returns True if the key is a single digit, false otherwise. | ||
| */ | ||
| export const isSingleDigit = (key: string): boolean => /^[0-9]$/.test(key); |
Collaborator
There was a problem hiding this comment.
isSingleDigitKey is more explicit and matches the arg + test cases. isSingleDigit sound like it would apply to a number arg
Comment on lines
+60
to
+64
| export const LiveExample: StoryFn<typeof InputBox> = props => { | ||
| return ( | ||
| <InputBoxWithState {...(props as Partial<InputBoxProps<SegmentObjMock>>)} /> | ||
| ); | ||
| }; |
Collaborator
There was a problem hiding this comment.
I won't block on it, but I think new stories pair well with their related component code similar to when adding specs with their related code
…n testutils for enhanced testing capabilities
…nputBox stories with date and time segment examples
…stutils for cleaner code
shaneeza
commented
Nov 17, 2025
Collaborator
Author
There was a problem hiding this comment.
I renamed this file with git mv, so I'm unsure why it's showing up as a new file. I also added time mocks to this file.
…ging in InputBox component
Contributor
|
Coverage after merging LG-5504/input-box-component into shaneeza/time-picker-integration will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
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 is the second PR in a series of three PRs that add the new
InputBoxpackage and integrate it into the existingDatePickerpackage:InputSegment#3293InputBoxcomponent #3286This PR adds logic to the
InputBoxcomponent for the new@leafygreen-ui/input-boxpackage, which provides reusable input segment functionality for date and time components. The component extracts common logic and UI elements previously embedded in thedate-pickerpackage, making it available for use acrossDatePicker,TimeInput, and other similar components.🎟 Jira ticket: LG-5504
✅ Checklist
For new components
For bug fixes, new features & breaking changes
pnpm changesetand documented my changes🧪 How to test changes
InputBoxandInputSegmentcomponents render correctly in their respective stories