Skip to content
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

Use React's AriaAttributes type directly rather than recreating #4941

Merged
merged 8 commits into from Jan 13, 2022

Conversation

prichey
Copy link
Contributor

@prichey prichey commented Dec 6, 2021

Fixes #4940

@changeset-bot
Copy link

changeset-bot bot commented Dec 6, 2021

🦋 Changeset detected

Latest commit: e760921

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
react-select Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@codesandbox-ci
Copy link

codesandbox-ci bot commented Dec 6, 2021

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit e760921:

Sandbox Source
react-codesandboxer-example Configuration

@@ -76,7 +77,7 @@ export interface Props<
/** HTML ID of an element containing an error message related to the input**/
'aria-errormessage'?: string;
/** Indicate if the value entered in the field is invalid **/
'aria-invalid'?: boolean;
'aria-invalid'?: AriaInvalid;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of redefining AriaInvalid, can we do AriaAttributes['aria-invalid'] here (and import AriaAtributes from react)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, definitely. I'll make that change now

Copy link
Contributor Author

@prichey prichey Dec 8, 2021

Choose a reason for hiding this comment

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

Do you think it's worth updating the other aria attributes to pull from that type for the sake of consistency? @Methuselah96

Copy link
Collaborator

Choose a reason for hiding this comment

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

If you're up for that, that would be great. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done ✅

@prichey prichey changed the title Update types for aria-invalid Use React's AriaAttributes directly rather than recreating Dec 8, 2021
@prichey prichey changed the title Use React's AriaAttributes directly rather than recreating Use React's AriaAttributes type directly rather than recreating Dec 8, 2021
Copy link
Collaborator

@Methuselah96 Methuselah96 left a comment

Choose a reason for hiding this comment

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

Thanks!

@Methuselah96 Methuselah96 added this to the 5.3 milestone Dec 8, 2021
@prichey
Copy link
Contributor Author

prichey commented Dec 8, 2021

@Methuselah96 took the liberty to update the only other place we were recreating types from AriaAttributes: 9ace36c. Going to stop poking at this now! Looking forward to 5.3 😄

@Methuselah96
Copy link
Collaborator

Methuselah96 commented Dec 8, 2021

Thanks! Forgot to ask earlier: can you add a changeset?

@prichey
Copy link
Contributor Author

prichey commented Dec 8, 2021

@Methuselah96 Just added one - let me know if I did it correctly, this was my first time using that package

@Methuselah96
Copy link
Collaborator

Yeah, looks good.

@prichey
Copy link
Contributor Author

prichey commented Jan 13, 2022

@Methuselah96 How do you determine when to cut a new release? It'd be nice to upgrade without having to do my own manual patching. Thanks!

@Methuselah96
Copy link
Collaborator

Methuselah96 commented Jan 13, 2022

@prichey Thanks for the reminder, as you can see we haven't been doing a whole lot of merging/releasing recently. I'll try to get a release out soon. I noticed you classified this as a minor change instead of a patch. Any particular reason?

@prichey
Copy link
Contributor Author

prichey commented Jan 13, 2022

@Methuselah96 No problem! Tbh it could totally be a patch, I think I did that after you had tagged it with 5.3 so I assumed you wanted it to be a minor release. Patch would be awesome if that helps it get released!

@Methuselah96 Methuselah96 enabled auto-merge (squash) January 13, 2022 17:16
@Methuselah96 Methuselah96 enabled auto-merge (squash) January 13, 2022 17:17
@Methuselah96 Methuselah96 merged commit 54f9538 into JedWatson:master Jan 13, 2022
@github-actions github-actions bot mentioned this pull request Jan 13, 2022
@Methuselah96
Copy link
Collaborator

@prichey Published in v5.2.2.

@prichey
Copy link
Contributor Author

prichey commented Jan 13, 2022 via email

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.

TypeScript: Incompatible 'aria-invalid' types for Select
2 participants