Skip to content

TimeField: properly handle isRequired prop#772

Open
mkernohanbc wants to merge 1 commit into
mainfrom
bug/timefield-disabled
Open

TimeField: properly handle isRequired prop#772
mkernohanbc wants to merge 1 commit into
mainfrom
bug/timefield-disabled

Conversation

@mkernohanbc

@mkernohanbc mkernohanbc commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

This PR fixes a mistake in how the isRequired prop is used in TimeField that surfaced during some testing work.

Previously, isRequired was destructured in order to render the visible "(required)" label — but this silently broke the actual required-input functionality, resulting in data-required="true" not being set on the TimeField.

This change removes this explicit destructuring, and instead properly passes it as a render prop (matching the pattern in other components) to handle the display of the label. With this change, data-required="true" is now also properly set on the TimeField.

@mkernohanbc mkernohanbc added this to the Components v0.9.0 milestone Jul 2, 2026
@mkernohanbc mkernohanbc self-assigned this Jul 2, 2026
@mkernohanbc mkernohanbc requested a review from a team as a code owner July 2, 2026 20:14
@mkernohanbc mkernohanbc added the bug label Jul 2, 2026
@mkernohanbc mkernohanbc changed the title pass isRequired as render prop instead of destructuring TimeField: properly handle isRequired prop Jul 2, 2026
@mkernohanbc mkernohanbc requested a review from ty2k July 2, 2026 20:14
@ty2k ty2k requested a review from Copilot July 2, 2026 20:22

Copilot AI left a comment

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.

Pull request overview

Fixes TimeField so the isRequired prop correctly drives both required behavior (e.g., data-required="true") and the visible “(required)” indicator, by consuming isRequired from React Aria render props instead of destructuring it out of the component props.

Changes:

  • Stop destructuring isRequired from TimeField props so it remains included in ...props passed to ReactAriaTimeField.
  • Read isRequired from the ReactAriaTimeField render-props state to render the “(required)” label consistently with other components.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@mkernohanbc mkernohanbc added the react-components Changes or issues affecting the design-system-react-components package label Jul 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug react-components Changes or issues affecting the design-system-react-components package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants