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

Fix render stability of IconPicker grid and update usage of deprecated Dropdown property #230

Merged
merged 7 commits into from
Jun 21, 2023

Conversation

fabiankaegy
Copy link
Member

Description of the Change

Fix how the IconGrid gets rendered inside the Icon Picker in order to prevent unwanted rerenders of the component. This should fix the issue described in #229. Whilst working in the codebase I also updated the usage of the deprecated placement prop of the Dropdown component (#226).

Closes #226, #229

How to test the Change

  1. Open the IconPicker
  2. Change the selected icon / type in the search bar

Expected Result:
The icons should not revert to a Spinner state when the state changes. Also, the browser console should not show any warnings.

Changelog Entry

Fixed - Prevent unintentional rerenders of IconGrid inside IconPicker component

Credits

Props @fabiankaegy, @benlk

Checklist:

  • I agree to follow this project's Code of Conduct.
  • I have updated the documentation accordingly.
  • I have added tests to cover my change.
  • All new and existing tests pass.

@fabiankaegy fabiankaegy self-assigned this Jun 6, 2023
@github-actions
Copy link

github-actions bot commented Jun 6, 2023

Size Change: +78 B (0%)

Total Size: 52.3 kB

Filename Size Change
dist/index.js 52.3 kB +78 B (0%)

compressed-size-action

@fabiankaegy
Copy link
Member Author

fabiankaegy commented Jun 6, 2023

@benlk In order to make it easier for you to test I published this as an alpha version to npm. You can install it like so:

npm i @10up/block-components@1.15.11-alpha.2

@fabiankaegy
Copy link
Member Author

@Antonio-Laguna just pinging you here to ensure you saw this. I will be out for the next two weeks starting tomorrow but I don't want this to stall out during that time :)

(The failing e2e tests are unrelated and as far as I know @Sidsector9 is looking at them)

Copy link
Member

@Antonio-Laguna Antonio-Laguna left a comment

Choose a reason for hiding this comment

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

LGTM

@Antonio-Laguna
Copy link
Member

@fabiankaegy thanks for pinging, I did see it but somehow it got missed!

@benlk
Copy link
Contributor

benlk commented Jun 8, 2023

Tested locally with npm i @10up/block-components@1.15.11-alpha.2 and the same components that I used when reporting #229 and #226:

@Antonio-Laguna
Copy link
Member

@benlk if you review we can get this merged and released as @next

@benlk
Copy link
Contributor

benlk commented Jun 12, 2023

Approved.

@benlk
Copy link
Contributor

benlk commented Jun 12, 2023

Correction: approved with suggestion for extra work.

@fabiankaegy fabiankaegy merged commit 5624fae into develop Jun 21, 2023
@fabiankaegy fabiankaegy deleted the fix/render-stability-of-icon-picker branch June 21, 2023 05:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment