Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Introduce TimeInput #551

Merged
merged 8 commits into from
Oct 16, 2020
Merged

Introduce TimeInput #551

merged 8 commits into from
Oct 16, 2020

Conversation

alexpaxton
Copy link
Contributor

@alexpaxton alexpaxton commented Oct 14, 2020

A specialized input designed for specifying units of time. This is defined as a number + a unit s, m, h, d, w, mo

Changes

  • Introduce TimeInput as a composed variation on Input
  • Create TimeUnit enum
  • Modify ButtonGroup to also affect inputs placed inside
  • Add monospace prop to Input

Screenshots

Screen Shot 2020-10-14 at 10 58 11 AM

Checklist

Check all that apply

  • Updated documentation to reflect changes
  • Added entry to top of Changelog with link to PR (not issue)
  • Tests pass
  • Peer reviewed and approved
  • Signed CLA (if not already signed)

@alexpaxton alexpaxton requested a review from mavarius October 14, 2020 18:04
Copy link
Collaborator

@mavarius mavarius left a comment

Choose a reason for hiding this comment

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

Looks good, though I wonder if we'll run into trouble hardcoding the units. We may end up needing a way to specify larger units like years or smaller units like milliseconds.

@alexpaxton
Copy link
Contributor Author

Looks good, though I wonder if we'll run into trouble hardcoding the units. We may end up needing a way to specify larger units like years or smaller units like milliseconds.

@mavarius I had a similar thought, like maybe instead of an enum it could just be an array that can be overridden if you want more or less units. Not 100% sure how this is gonna be used yet, all I know is there will be inputs in the alerts UI that want units of time specifically

@alexpaxton alexpaxton merged commit d5c62e9 into master Oct 16, 2020
@alexpaxton alexpaxton deleted the feat/time-input branch October 16, 2020 16:22
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants