[LG-5532] feat(time-input): Segment misc #3387
Open
+328
−29
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
…for time input components
…ngodb/leafygreen-ui into LG-5532/segments-callback
…ngodb/leafygreen-ui into LG-5532/segments-callback
|
This was referenced Dec 15, 2025
shaneeza
commented
Dec 15, 2025
| }} | ||
| /> | ||
| )} | ||
| {is24HourFormat && ( |
Collaborator
Author
There was a problem hiding this comment.
Add a story for this
Contributor
There was a problem hiding this comment.
Pull request overview
This PR implements miscellaneous improvements to the TimeInputSegment component, focusing on segment reference management, event handling, and UI enhancements. The changes add support for segment refs to enable proper focus management, implement onChange event handling for individual segments, and add a "24 hours" label for 24-hour time formats.
Key Changes:
- Added segment reference management system using
useDynamicRefshook - Implemented
onSegmentChangehandler to fire synthetic change events when individual segments update - Added "24 hours" label UI for 24-hour time format with corresponding styling
- Enhanced
TimeInputSelectcomponent with size prop and improved variable naming
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/time-input/src/testing/testUtils.ts | Creates mock segment refs for testing |
| packages/time-input/src/shared.types.ts | Defines SegmentRefs type for managing segment references |
| packages/time-input/src/constants.ts | Adds "24 hours" display text constant |
| packages/time-input/src/TimeInputSelect/TimeInputSelect.tsx | Adds size prop and improves variable naming from u to unitOption |
| packages/time-input/src/TimeInputInputs/TimeInputInputs.tsx | Implements segment change handling, click handling, and 24-hour label rendering |
| packages/time-input/src/TimeInputInputs/TimeInputInputs.styles.ts | Adds styling for wrapper and 24-hour label |
| packages/time-input/src/TimeInputInputs/TimeInputInputs.spec.tsx | Adds tests for 24-hour label rendering in both 12/24 hour formats |
| packages/time-input/src/TimeInputBox/TimeInputBox.types.ts | Adds props for segment change handler and segment refs |
| packages/time-input/src/TimeInputBox/TimeInputBox.spec.tsx | Implements test for onSegmentChange callback |
| packages/time-input/src/TimeInput.stories.tsx | Adds onChange console logging to Storybook |
| packages/time-input/src/Context/TimeInputContext/useTimeInputComponentRefs.ts | Creates hook for managing segment refs |
| packages/time-input/src/Context/TimeInputContext/TimeInputContext.types.ts | Adds refs to context type |
| packages/time-input/src/Context/TimeInputContext/TimeInputContext.tsx | Integrates segment refs into context provider |
| z-index: 0; // Establish new stacking context | ||
| `, | ||
| { | ||
| [twelveHourFormatStyles]: !is12HourFormat, |
There was a problem hiding this comment.
The condition !is12HourFormat applies twelveHourFormatStyles when the format is 24-hour, but the styles are named for 12-hour format. This should be is12HourFormat instead of !is12HourFormat.
Suggested change
| [twelveHourFormatStyles]: !is12HourFormat, | |
| [twelveHourFormatStyles]: is12HourFormat, |
Comment on lines
+20
to
+22
| hour: getSegmentRef('hour') || undefined, | ||
| minute: getSegmentRef('minute') || undefined, | ||
| second: getSegmentRef('second') || undefined, |
There was a problem hiding this comment.
The || undefined is redundant since getSegmentRef already returns a ref object or undefined. Remove the || undefined clauses from all three lines.
Suggested change
| hour: getSegmentRef('hour') || undefined, | |
| minute: getSegmentRef('minute') || undefined, | |
| second: getSegmentRef('second') || undefined, | |
| hour: getSegmentRef('hour'), | |
| minute: getSegmentRef('minute'), | |
| second: getSegmentRef('second'), |
Contributor
|
Size Change: +920 B (+0.05%) Total Size: 1.82 MB
ℹ️ View Unchanged
|
shaneeza
requested review from
TheSonOfThomp and
stephl3
and removed request for
a team
…ngodb/leafygreen-ui into LG-5532/segments-callback
…and improve styling for 24-hour format
Contributor
|
Coverage after merging LG-5532/segments-callback into LG-5532/segments-display-values-state will be
Coverage Report for Changed Files
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
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
🎟 Jira ticket: Name of ticket
This PR implements various improvements, including support for segment refs to enable proper focus management, implementation of onChange event handling for individual segments, addition of a "24 hours" label for 24-hour time formats, and disabled states.
This PR is the fifth PR in a chain of PRs
✅ Checklist
🧪 How to test changes