Skip to content

Add disableScaleDown to base.d.ts #6668

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
merged 2 commits into from
Mar 7, 2023
Merged

Add disableScaleDown to base.d.ts #6668

merged 2 commits into from
Mar 7, 2023

Conversation

taysea
Copy link
Collaborator

@taysea taysea commented Mar 6, 2023

What does this PR do?

Adds base.d.ts definition for theme.icon.disableScaleDown.

Where should the reviewer start?

What testing has been done on this PR?

How should this be manually tested?

Do Jest tests follow these best practices?

  • 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?

Related to grommet/grommet-icons#301

Screenshots (if appropriate)

Do the grommet docs need to be updated?

Should this PR be mentioned in the release notes?

Yes.

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

Backwards compatible.

@taysea taysea requested a review from jcfilben March 6, 2023 22:21
@ericsoderberghp
Copy link
Contributor

Just realized, should we add a test to ensure the typings are working?

@taysea
Copy link
Collaborator Author

taysea commented Mar 7, 2023

Just realized, should we add a test to ensure the typings are working?

Since grommet-icons hasn't been released, the functionality of disableScaleDown functionality doesn't come through on Grommet snapshots yet. However I confirmed locally that the typings and behavior (by upgrading local to point grommet-icons to stable) come through as expected:

Screen Shot 2023-03-07 at 10 49 53 AM

local when pointing to grommet-icons stable branch
Screen Shot 2023-03-07 at 10 49 17 AM

@ericsoderberghp Do you want me to push up the changes even though the master snapshots wouldn't include the vector-effect? or is this confirmation sufficient.

@ericsoderberghp ericsoderberghp merged commit 108eefa into grommet:master Mar 7, 2023
@jcfilben
Copy link
Collaborator

jcfilben commented Mar 7, 2023

I'm okay if we want to circle back and add a test later

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