[LG-5538] feat(time-input): Parse time #3378
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
…cale dependency and enhancing formatting logic
… format output with and without seconds
…meInputDisplayContext
…ity in time input context
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds time parsing utilities to the time-input component, enabling it to format and display time values based on locale and timezone settings. The changes introduce utilities for locale-based time formatting, AM/PM period detection, and time part management.
Key changes:
- Added utility functions for formatting time values according to locale/timezone
- Implemented locale-based AM/PM period detection to conditionally show day period selector
- Created type definitions for time parts and integrated parsing logic into the TimeInput component
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/install/src/ALL_PACKAGES.ts | Added canvas-header package to the list |
| packages/time-input/src/utils/index.ts | Exports new utility functions for time formatting |
| packages/time-input/src/utils/hasDayPeriod/hasDayPeriod.ts | Determines if a locale uses 12-hour format with AM/PM |
| packages/time-input/src/utils/hasDayPeriod/hasDayPeriod.spec.ts | Tests for day period detection across locales |
| packages/time-input/src/utils/getFormatter/getFormatter.ts | Creates Intl.DateTimeFormat instances for locale-specific formatting |
| packages/time-input/src/utils/getFormatter/getFormatter.spec.ts | Tests formatter creation for valid/invalid locales |
| packages/time-input/src/utils/getFormattedTimeParts/getFormattedTimeParts.ts | Converts format parts to structured time parts object |
| packages/time-input/src/utils/getFormattedTimeParts/getFormattedTimeParts.spec.ts | Tests time parts formatting and default merging |
| packages/time-input/src/utils/getFormatPartsValues/getFormatPartsValues.ts | Extracts time values from dates considering locale and timezone |
| packages/time-input/src/utils/getFormatPartsValues/getFormatPartsValues.spec.ts | Tests time part extraction across timezones and locales |
| packages/time-input/src/utils/getFormatParts/getFormatParts.ts | Generates format part templates for time input segments |
| packages/time-input/src/utils/getFormatParts/getFormatParts.spec.ts | Tests format part generation with/without seconds |
| packages/time-input/src/utils/getFilteredTimeParts/getFilteredTimeParts.ts | Filters out literal separators from time parts |
| packages/time-input/src/utils/getFilteredTimeParts/getFilteredTimeParts.spec.ts | Tests filtering of literal parts |
| packages/time-input/src/shared.types.ts | Defines TimePartKeys and TimeParts types |
| packages/time-input/src/constants.ts | Adds default time parts values |
| packages/time-input/src/TimeInputInputs/TimeInputInputs.tsx | Integrates time parsing and conditionally renders AM/PM selector |
| packages/time-input/src/TimeInput/TimeInput.types.ts | Adds showSeconds prop to component API |
| packages/time-input/src/TimeInput/TimeInput.tsx | Adds commented debug logging |
| packages/time-input/src/TimeInput.stories.tsx | Enhances stories with timezone/locale controls and state management |
| packages/time-input/src/Context/TimeInputDisplayContext/TimePickerDisplayContext.utils.ts | Adds showSeconds to display context defaults |
| packages/time-input/src/Context/TimeInputDisplayContext/TimeInputDisplayContext.types.ts | Adds shouldShowSelect and formatParts to context type |
| packages/time-input/src/Context/TimeInputDisplayContext/TimeInputDisplayContext.tsx | Calculates shouldShowSelect and formatParts in provider |
| packages/time-input/src/Context/TimeInputContext/TimeInputContext.tsx | Adds TODO comment for default date handling |
packages/time-input/src/utils/getFormatPartsValues/getFormatPartsValues.ts
Outdated
Show resolved
Hide resolved
packages/time-input/src/utils/getFormatPartsValues/getFormatPartsValues.ts
Outdated
Show resolved
Hide resolved
packages/time-input/src/utils/getFormatter/getFormatter.spec.ts
Outdated
Show resolved
Hide resolved
packages/time-input/src/Context/TimeInputContext/TimeInputContext.tsx
Outdated
Show resolved
Hide resolved
Contributor
|
Size Change: +1.15 kB (+0.06%) Total Size: 1.82 MB
ℹ️ View Unchanged
|
…db/leafygreen-ui into LG-5538/segments-parse-time
shaneeza
requested review from
TheSonOfThomp and
stephl3
and removed request for
a team
This was referenced Dec 15, 2025
stephl3
reviewed
Dec 15, 2025
| * // returns: [{ type: 'hour', value: '12' }, { type: 'minute', value: '30' }] | ||
| * ``` | ||
| */ | ||
| export const getFilteredTimeParts = ({ |
Collaborator
There was a problem hiding this comment.
nit: could more explicitly name this getNonLiteralTimeParts or excludeLiteralParts
Collaborator
There was a problem hiding this comment.
since it's only used in getFormatPartsValue, what do you think about bucketing it in that subdir with a barrel file exporting getFormatPartsValues?
| @@ -0,0 +1,17 @@ | |||
| import { getFilteredTimeParts } from './getFilteredTimeParts'; | |||
|
|
|||
| describe('packages/time-input/utils/getFilteredTimeParts', () => { | |||
Collaborator
There was a problem hiding this comment.
could include cases for undefined or empty array
| @@ -0,0 +1,13 @@ | |||
| export const TimePartKeys = { | |||
Collaborator
There was a problem hiding this comment.
Suggested change
| export const TimePartKeys = { | |
| export const DateTimePartKeys = { |
| * // returns: { hour: '12', minute: '00', second: '00', month: '01', day: '01', year: '2025', dayPeriod: 'PM' } | ||
| * ``` | ||
| */ | ||
| export const getFormatPartsValues = ({ |
Collaborator
There was a problem hiding this comment.
Similar to feedback about getFilteredTimeParts, since it's only used in getFormatPartsValue, what do you think about bucketing it in that subdir with a barrel file exporting getFormatPartsValues?
| jest.useRealTimers(); | ||
| }); | ||
|
|
||
| describe('and the time zone is', () => { |
| // TODO: min, max helpers | ||
|
|
||
| // Determines if the input should show a select for the day period (AM/PM) | ||
| const is12hFormat = !!hasDayPeriod(providerValue.locale); |
Collaborator
There was a problem hiding this comment.
Suggested change
| const is12hFormat = !!hasDayPeriod(providerValue.locale); | |
| const is12hFormat = hasDayPeriod(providerValue.locale); |
| * hasDayPeriod('iso-8601'); // false | ||
| * ``` | ||
| */ | ||
| export const hasDayPeriod = (locale: string) => { |
Collaborator
There was a problem hiding this comment.
is there a more restrictive type that fits here vs string?
| * ``` | ||
| */ | ||
| export const hasDayPeriod = (locale: string) => { | ||
| // If the locale is ISO_8601, return false |
Collaborator
There was a problem hiding this comment.
nit: comment seems unnecessary since it's restating what the code reads as
| if (!formatter) return false; | ||
|
|
||
| // Format a sample time and check for dayPeriod (AM/PM) | ||
| const parts = formatter?.formatToParts(new Date()); |
Collaborator
There was a problem hiding this comment.
Suggested change
| const parts = formatter?.formatToParts(new Date()); | |
| const parts = formatter.formatToParts(new Date()); |
|
|
||
| // Format a sample time and check for dayPeriod (AM/PM) | ||
| const parts = formatter?.formatToParts(new Date()); | ||
| const hasDayPeriod = parts?.some(part => part.type === 'dayPeriod'); |
Collaborator
There was a problem hiding this comment.
Suggested change
| const hasDayPeriod = parts?.some(part => part.type === 'dayPeriod'); | |
| const hasDayPeriod = parts?.some(part => part.type === TimePartKeys.dayPeriod; |
… and update related logic for clarity
shaneeza
requested review from
adamrasheed
and removed request for
a team and
adamrasheed
…nclude time parts and improve tests for locale handling
…meInputDisplayContext
Contributor
|
Coverage after merging LG-5538/segments-parse-time into shaneeza/segment-logic-integration 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: LG-5538
This PR is the first PR in a chain of PRs
This PR adds time parsing utilities, enabling it to format time values based on locale and timezone props, however, values are not displayed in this PR.
Included are utilities for locale-based time formatting, AM/PM period detection, and time part management.
✅ Checklist
🧪 How to test changes