-
Notifications
You must be signed in to change notification settings - Fork 72
[LG-5532] feat(time-input): Segment state #3386
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: LG-5532/segments-state-utils
Are you sure you want to change the base?
[LG-5532] feat(time-input): Segment state #3386
Conversation
…mponents with UTC conversion and logging for better time handling
…comparison logic in useSelectUnit hook
…logic for improved clarity
…alidation and date handling logic, including new utility functions for explicit segment checks
…n TimeInputInputs, removing unnecessary console logs and enhancing segment checks
…ime change checks for improved clarity and accuracy
… segment validation and date handling, enhancing utility functions for better clarity and performance
…d 24 hour formats, improving clarity on explicit value checks and segment validation
…across components, enhancing clarity and organization
…ate and segment updates, enhancing clarity in handling locale and timezone changes
…oks by consolidating utility functions and enhancing logging for better clarity and performance
…meInputInputs for improved segment and unit management, enhancing logging and state handling
…o TimeInputInputs, enhancing segment management and reducing console log clutter for improved clarity
…improved segment update logic and logging, ensuring better handling of date changes and segment validation
…uts logic, enhancing segment update handling and reducing console log clutter for improved clarity
… behavior and segment rendering
…leafygreen-ui into LG-5532/segments-display-values-state
…d enhance logging for time changes
…rendering behavior
…ime segment handling in hooks
… and explicit value handling
…date DatePicker initial state
…cessary variable assignment
…CFromTimeZone for UTC date conversion
…leafygreen-ui into LG-5532/segments-display-values-state
…electUpdate for clarity
…fygreen-ui into LG-5532/segments-display-values-state
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR implements state management for time input segments and the AM/PM select unit, replacing the temporary implementation with a proper hook-based solution. The changes introduce useTimeSegmentsAndSelectUnit hook to centrally manage time segment state, select unit state, and coordinate updates between them while handling timezone and locale changes.
Key Changes
- New
useTimeSegmentsAndSelectUnithook consolidates time segment and select unit state management - Removed deprecated
useSelectUnithook in favor of the unified solution - Updated segment input logic to properly set values when segments change
- Fixed
minExplicitValuecalculation for hour segments in different time formats
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
packages/time-input/src/hooks/useTimeSegmentsAndSelectUnit/useTimeSegmentsAndSelectUnit.types.ts |
Defines TypeScript types and action enums for the new hook |
packages/time-input/src/hooks/useTimeSegmentsAndSelectUnit/useTimeSegmentsAndSelectUnit.ts |
Implements the core hook with reducer logic for managing segments and select unit |
packages/time-input/src/hooks/useTimeSegmentsAndSelectUnit/useTimeSegmentAndSelectUnit.spec.ts |
Adds comprehensive test coverage for the new hook across timezones and locales |
packages/time-input/src/hooks/useTimeSegmentsAndSelectUnit/index.ts |
Exports the new hook and types |
packages/time-input/src/hooks/useSelectUnit/useSelectUnit.ts |
Removes deprecated hook implementation |
packages/time-input/src/hooks/useSelectUnit/index.ts |
Removes export of deprecated hook |
packages/time-input/src/hooks/index.ts |
Updates exports to use new hook instead of deprecated one |
packages/time-input/src/constants.ts |
Adjusts minExplicitValue for hour segment validation |
packages/time-input/src/TimeInputInputs/TimeInputInputs.tsx |
Integrates the new hook and implements segment/select update handlers |
packages/time-input/src/TimeInputInputs/TimeInputInputs.spec.tsx |
Adds extensive integration tests for the TimeInputInputs component |
packages/time-input/src/TimeInput.stories.tsx |
Updates story with console logging and display of UTC values |
| import { useTimeInputDisplayContext } from '../Context/TimeInputDisplayContext/TimeInputDisplayContext'; | ||
| import { useSelectUnit } from '../hooks'; | ||
| import { TimeSegmentsState } from '../shared.types'; | ||
| import { OnUpdateCallback,useTimeSegmentsAndSelectUnit } from '../hooks'; |
Copilot
AI
Dec 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing space after comma in import statement. Should be OnUpdateCallback, useTimeSegmentsAndSelectUnit.
| import { OnUpdateCallback,useTimeSegmentsAndSelectUnit } from '../hooks'; | |
| import { OnUpdateCallback, useTimeSegmentsAndSelectUnit } from '../hooks'; |
| if (hasSelectUnitChanged) { | ||
| onUpdate?.({ | ||
| newSelectUnit: newSelectUnit, | ||
| prevSelectUnit: { ...selectUnit }, |
Copilot
AI
Dec 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using spread operator on selectUnit which is a UnitOption object is unnecessary since UnitOption appears to be a simple object. For consistency with line 226 where selectUnit is passed directly as prevSelectUnit, this should also pass selectUnit directly without spreading.
| prevSelectUnit: { ...selectUnit }, | |
| prevSelectUnit: selectUnit, |
|
Size Change: +2.53 kB (+0.14%) Total Size: 1.82 MB
ℹ️ View Unchanged
|
…fygreen-ui into LG-5532/segments-display-values-state
|
Coverage after merging LG-5532/segments-display-values-state into LG-5532/segments-state-utils will be
Coverage Report for Changed Files
|
|||||||||||||||||||||||||||||||
✍️ Proposed changes
🎟 Jira ticket: Name of ticket
This PR implements state management for time input segments and the AM/PM select unit. The changes introduce
useTimeSegmentsAndSelectUnit(open to improving this name) hook to manage time segment state, select unit state, and coordinate updates between them while handling timezone and locale changes.This PR is the fourth PR in a chain of PRs
🧪 How to test changes