[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.
Comment on lines
117
to
118
| // We also check for `space` because Number(' ') returns true | ||
| const isNumber = Number(key) && key !== keyMap.Space; |
There was a problem hiding this comment.
The condition Number(key) is incorrect for checking if a key is a numeric digit. Number('') returns 0 which is falsy, but an empty string is not a number. Additionally, Number('0') returns 0 which is also falsy, meaning pressing '0' would fail this check. Use !isNaN(Number(key)) && key !== keyMap.Space && key !== '' or preferably check if the key matches /^[0-9]$/ to properly validate numeric key presses.
Suggested change
| // We also check for `space` because Number(' ') returns true | |
| const isNumber = Number(key) && key !== keyMap.Space; | |
| // Use regex to check for single digit key presses | |
| const isNumber = /^[0-9]$/.test(key); |
Contributor
|
Size Change: +6.91 kB (+0.43%) Total Size: 1.61 MB
ℹ️ View Unchanged
|
shaneeza
requested review from
a team,
TheSonOfThomp and
stephl3
and removed request for
a team
adamrasheed
reviewed
Nov 6, 2025
| font-size: var(--base-font-size, ${typeScales.body1.fontSize}px); | ||
| `, | ||
| [Size.Large]: css` | ||
| font-size: ${18}px; // Intentionally off-token |
Collaborator
There was a problem hiding this comment.
Should this be font-size: 18px;?
Comment on lines
96
to
102
| return css` | ||
| ${baseStyles} | ||
| ${fontSizeStyles[baseFontSize]} | ||
| ${segmentThemeStyles[theme]} | ||
| ${segmentSizeStyles[size]} | ||
| ${className} | ||
| `; |
Collaborator
There was a problem hiding this comment.
should we wrap all of these in a cx instead?
Collaborator
Author
There was a problem hiding this comment.
good callout. I copied these all over from DatePicker and forgot to update this file.
| }; | ||
| } | ||
|
|
||
| // TODO: consider renaming min/max names to minSegment/maxSegment |
Collaborator
There was a problem hiding this comment.
should we implement this?
Collaborator
Author
There was a problem hiding this comment.
which is clearer to you, min or segmentMin?
Collaborator
There was a problem hiding this comment.
minSegment is clearer to me :)
|
|
||
| /** The percentage of 1ch these specific characters take up */ | ||
| export const characterWidth = { | ||
| // // Standard font |
Collaborator
There was a problem hiding this comment.
Suggested change
| // // Standard font | |
| // Standard font |
| 'segmentEnum', | ||
| ], | ||
| }, | ||
| }, |
Collaborator
There was a problem hiding this comment.
nit: should we add a default size via args?
…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. |
…en-ui into LG-5504/input-box-component
shaneeza
changed the base branch from
shaneeza/time-picker-integration
to
LG-5504/input-box-context
This was referenced Nov 7, 2025
…en-ui into LG-5504/input-box-component
shaneeza
changed the base branch from
LG-5504/input-box-context
to
LG-5504/input-box-segment
shaneeza
changed the title
input-box component
shaneeza
changed the title
input-box componentInputBox component
shaneeza
changed the title
InputBox componentInputBox
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 third PR in a series of PRs that adds the new
InputBoxpackage and integrates it into the existingDatePickerpackage:InputBoxContext#3290InputSegment#3293This 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