[LG-5504] refactor(date-picker): migrate to use InputBox component
#3286
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
… improved type handling and event management
…tSegment with additional props
…with improved type handling and segment management
…ved segment management and type handling
…es for better segment management
…pe handling and segment management
… new props for step handling and rollover management
…icker components for improved segment management and type handling
…nce segment validation logic for improved date handling
…ean up InputBox component
…alidation and formatting functions
…tSegment and InputBox tests for better coverage
…ent components by introducing utility functions
…gment components for improved clarity and documentation
…ent types with clearer examples
…Rules in InputBox types for better clarity
…ting getValueFormatter to accept segment-specific character counts
… for year segment and enhance validation logic
…ype safety and clarity
…ng and props validation
…r consistency and clarity across components
… tests for InputBox and InputSegment components
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the date-picker package to use the new @leafygreen-ui/input-box component, significantly simplifying the codebase by delegating input segment handling to a shared component instead of maintaining custom implementations.
Key changes:
- Replaced custom
DateInputSegmentimplementation withInputSegmentfrom@leafygreen-ui/input-box - Removed custom validation, formatting, and keyboard navigation utilities now handled by input-box
- Added
DateInputBoxContextto provide date value context to segments
Reviewed Changes
Copilot reviewed 37 out of 38 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
tools/install/src/ALL_PACKAGES.ts |
Added new packages (compound-component, feature-walls, input-box) to package list |
packages/date-picker/package.json |
Added @leafygreen-ui/input-box dependency |
packages/date-picker/tsconfig.json |
Added input-box to TypeScript references |
packages/date-picker/src/shared/constants.ts |
Added dateSegmentRules configuration for input-box integration |
packages/date-picker/src/shared/utils/**/*.ts |
Removed custom validation, formatting, and navigation utilities (replaced by input-box) |
packages/date-picker/src/shared/utils/isEverySegmentValid/isEverySegmentValid.ts |
Updated to use isValidValueForSegment from input-box with custom year validation |
packages/date-picker/src/shared/utils/isEverySegmentValueExplicit/isEverySegmentValueExplicit.ts |
Updated to use createExplicitSegmentValidator from input-box |
packages/date-picker/src/shared/utils/getSegmentsFromDate/*.ts |
Updated to use getValueFormatter from input-box |
packages/date-picker/src/shared/components/DateInput/DateInputSegment/*.tsx |
Simplified to wrap InputSegment from input-box |
packages/date-picker/src/shared/components/DateInput/DateInputBoxContext/** |
Added new context to provide date value to segments |
packages/date-picker/src/shared/components/DateInput/DateInputBox/DateInputBox.tsx |
Refactored to use InputBox component from input-box |
packages/date-picker/src/DatePicker/DatePickerInput/DatePickerInput.tsx |
Removed keyboard navigation logic (now in input-box), updated imports |
.changeset/input-box.md |
Added changeset for input-box initial release |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
| const defaultSegmentProps = { | ||
| value: '', | ||
| onChange: () => {}, | ||
| onChange: () => {}, //TODO: remove this |
There was a problem hiding this comment.
The TODO comment indicates incomplete work. Either remove this deprecated onChange prop or create a task to address this in a follow-up change.
Suggested change
| onChange: () => {}, //TODO: remove this | |
| onChange: () => {}, |
| /> | ||
| </DateInputBoxProvider> | ||
| </InputBoxProvider> | ||
| , |
There was a problem hiding this comment.
Extraneous comma outside the JSX closing tag. This should be removed as it serves no purpose and may cause syntax issues.
Suggested change
| , |
Contributor
|
Size Change: -1.41 kB (-0.08%) 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 7, 2025
Comment on lines
39
to
43
| ctx?.args.segment === 'day' | ||
| ? 'DD' | ||
| : ctx?.args.segment === 'month' | ||
| ? 'MM' | ||
| : 'YYYY' |
Collaborator
There was a problem hiding this comment.
can we use if/else instead?
https://github.com/mongodb/leafygreen-ui/blob/main/STYLEGUIDE.md
Collaborator
Author
There was a problem hiding this comment.
turns out i don't even need this, removing
…ygreen-ui into LG-5504/input-box-in-date-picker-3
…tories for better segment handling
shaneeza
commented
Nov 7, 2025
| '@leafygreen-ui/code', | ||
| '@leafygreen-ui/code-editor', | ||
| '@leafygreen-ui/combobox', | ||
| '@leafygreen-ui/compound-component', |
Collaborator
Author
There was a problem hiding this comment.
TODO: need to pull the latest main in these PRs
This was referenced Nov 7, 2025
shaneeza
changed the title
InputBox component
…ygreen-ui into LG-5504/input-box-in-date-picker-3
…ygreen-ui into LG-5504/input-box-in-date-picker-3
… props and enhancing context usage
…ygreen-ui into LG-5504/input-box-in-date-picker-3
…reen-ui into LG-5504/input-box-in-date-picker-3
…reen-ui into LG-5504/input-box-in-date-picker-3
Contributor
|
Coverage after merging LG-5504/input-box-in-date-picker-3 into LG-5504/input-box-component will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
stephl3
reviewed
Nov 13, 2025
| const segment = part.type as DateSegment; | ||
| const formatter = getValueFormatter(segment); | ||
| const formatter = getValueFormatter({ | ||
| charsPerSegment: charsPerSegment[segment], |
Collaborator
There was a problem hiding this comment.
It feels like there could be some confusion that the charsPerSegment param is a number while the charsPerSegment const is an object mapping a segment to a number. Could be helpful to rename charsPerSegment constant more explicitly. i.e. charsPerSegmentObj or charsPerSegmentMap
Comment on lines
+19
to
+23
| customValidation: | ||
| segment === DateSegment.Year | ||
| ? (value: DateSegmentValue) => inRange(Number(value), 1000, 9999 + 1) | ||
| : undefined, | ||
| }), |
Collaborator
There was a problem hiding this comment.
could we add a comment about what this is for? does this override min / max validation?
Comment on lines
+10
to
+15
| export interface DateInputBoxProviderProps { | ||
| /** | ||
| * Date value in UTC time | ||
| */ | ||
| value?: DateType; | ||
| } |
Collaborator
There was a problem hiding this comment.
I think something like this could work as well and be a little DRYer
Suggested change
| export interface DateInputBoxProviderProps { | |
| /** | |
| * Date value in UTC time | |
| */ | |
| value?: DateType; | |
| } | |
| export interface DateInputBoxProviderProps extends PropsWithChildren<DateInputBoxContextType> {}; |
Comment on lines
+102
to
+113
| onKeyDown={onKeyDown} | ||
| segmentRefs={segmentRefs} | ||
| segmentEnum={DateSegment} | ||
| charsPerSegment={charsPerSegment} | ||
| formatParts={formatParts} | ||
| segments={segments} | ||
| setSegment={setSegment} | ||
| disabled={disabled} | ||
| segmentRules={dateSegmentRules} | ||
| onSegmentChange={onSegmentChange} | ||
| labelledBy={labelledBy} | ||
| segmentComponent={DateInputSegment} |
Collaborator
There was a problem hiding this comment.
if there are props that are not used for component logic, can we instead pass them through to InputBox through the spread ...rest?
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 three PRs that add the new
InputBoxpackage and integrate it into the existingDatePickerpackage:InputSegment#3293InputBox#3285This PR refactors the date-picker package to leverage the new
@leafygreen-ui/input-boxcomponent instead of maintaining custom input segment implementations. This migration significantly simplifies the codebase by removing redundant logic and utilities, while maintaining the same functionality through the shared input-box component.Key changes include:
DateInputSegmentimplementation withInputSegmentfrom@leafygreen-ui/input-box🎟️ Jira ticket: LG-5504
✅ Checklist
pnpm changesetand documented my changes🧪 How to test changes