Skip to content

feat(time picker): added min/max time constraints#6681

Merged
tlabaj merged 3 commits intopatternfly:mainfrom
evwilkin:fix/5853-timepicker-minmax
Jan 4, 2022
Merged

feat(time picker): added min/max time constraints#6681
tlabaj merged 3 commits intopatternfly:mainfrom
evwilkin:fix/5853-timepicker-minmax

Conversation

@evwilkin
Copy link
Copy Markdown
Member

@evwilkin evwilkin commented Dec 8, 2021

What: Closes #5853 and closes #6665

This PR:

  • adds minTime, maxTime, and invalidMinMaxErrorMessage props which enable restricting the options shown to a user and throwing an error if the user inputs a time outside the constraints.
  • adds an example to the time picker showing these new features.
  • fixes bug from TimePicker shows invalid on 12hour lowercase delimiter #6665 where custom delimiter is always changed to uppercase upon time selection/entry when is24Hour prop is not present.

@patternfly-build
Copy link
Copy Markdown
Collaborator

patternfly-build commented Dec 8, 2021

maxTime = convertTo24Hour(maxTime);

// simple string comparison for 24hr times
return minTime <= time && time <= maxTime;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In your example, it looks like the max time is not included in the list. Should it be?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nicolethoen it shouldn't be included as it's currently designed, but we could change that. I'm not touching the time options that are created from the stepMinutes other than to remove any that are earlier than the min or greater than the max.

In the example, the stepMinutes defaults to 30 minute intervals and because the maxTime is 17:15 the last time option shown is 17:00 (as the next option, 17:30, is above the maxTime).

@mcarrano would you expect that we should add an additional time option with their max time (or min time) if it doesn't align with one of the existing time options? In my example, where maxTime=17:15, rather than the last option being 17:00 there would be one more that would be 17:15?

@nicolethoen nicolethoen requested a review from kmcfaul December 20, 2021 20:32
@mcarrano
Copy link
Copy Markdown
Member

@mcarrano would you expect that we should add an additional time option with their max time (or min time) if it doesn't align with one of the existing time options? In my example, where maxTime=17:15, rather than the last option being 17:00 there would be one more that would be 17:15?

No I wouldn't expect this. The user could always enter that from the keyboard. Seems like pretty much an edge case.

@tlabaj tlabaj merged commit a6bb8b9 into patternfly:main Jan 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TimePicker shows invalid on 12hour lowercase delimiter Add maximum and minimum properties for TimePicker

5 participants