react-spectrum icon indicating copy to clipboard operation
react-spectrum copied to clipboard

fix: remove hours limit for `parseDuration`

Open limkhl opened this issue 1 year ago • 0 comments

Closes #5416

image

As mentioned in the issue, I reviewed the Temporal.Duration spec to align the behavior. It seems that the hours value can go up to 130 and beyond, with no explicit maximum limit. Therefore, I’ve adjusted the maximum value to be Infinity.

The previously invalid case P18Y7MT30H15S is valid in Temporal spec, as it returns:

{
    years: 18,
    months: 7,
    weeks: 0,
    days: 0,
    hours: 30,
    minutes: 0,
    seconds: 15
}

As a result, I’ve updated the test case accordingly.

Additional Notes: Currently, only the hours property has had its maximum value limit removed, but properties like years, months, minutes, and seconds should also not have maximum limits. To fully align with the Temporal.Duration specification, I think it’s necessary to remove the maximum value restrictions for all these properties.

✅ Pull Request Checklist:

  • [x] Included link to corresponding React Spectrum GitHub Issue.
  • [x] Added/updated unit tests and storybook for this change (for new code or code which already has tests).
  • [ ] Filled out test instructions.
  • [ ] Updated documentation (if it already exists for this component).
  • [ ] Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

🧢 Your Project:

limkhl avatar Sep 22 '24 07:09 limkhl