Skip to content

Updates type of onChange of RangeSelector #6970

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

Merged

Conversation

vinodkv2511
Copy link
Contributor

@vinodkv2511 vinodkv2511 commented Oct 13, 2023

Improves event handler type for RageSelector component.

What does this PR do?

Improves event handler type for RageSelector component.

Where should the reviewer start?

index.d.ts of RangeSelector

What testing has been done on this PR?

Checked the types locally

How should this be manually tested?

Verify the typing

Do Jest tests follow these best practices?

Not applicable

  • screen is used for querying.
  • The correct query is used. (Refer to this list of queries)
  • userEvent is used in place of fireEvent.
  • asFragment() is used for snapshot testing.

Any background context you want to provide?

What are the relevant issues?

#3165

Screenshots (if appropriate)

Do the grommet docs need to be updated?

Should this PR be mentioned in the release notes?

Is this change backwards compatible or is it a breaking change?

May be breaking for people using typescript. Because we are now making the type stricter.

@vinodkv2511
Copy link
Contributor Author

@jcfilben Can you please review this?

Copy link
Collaborator

@jcfilben jcfilben 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!

@jcfilben jcfilben requested a review from britt6612 October 16, 2023 22:21
@jcfilben jcfilben merged commit 59535ba into grommet:master Oct 17, 2023
@vinodkv2511 vinodkv2511 deleted the event-handler-type-for-range-selector branch October 18, 2023 04:03
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.

3 participants