Skip to content

[@wordpress/notices] Add a new action removeNotices which allows bulk removal of notices #39940

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.

Sign up for GitHub

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

Jump to bottom
Merged
nerrad merged 9 commits into from
May 30, 2023
Merged

[@wordpress/notices] Add a new action removeNotices which allows bulk removal of notices #39940

nerrad merged 9 commits into from
May 30, 2023

Conversation

@opr
Copy link
Contributor

@opr opr commented Mar 31, 2022 •
edited
Loading

What?

Adds a new action on the core/notices store that will remove several notices at once by supplying multiple IDs.

removeNotices( [ 'some-id', 'some-other-id' ] )

Why?

To remove multiple notices, you'd need to make several calls to removeNotice this makes a slight performance improvement by only dispatching one action.

An example use case is in WooCommerce Blocks where we need to remove several notices of a certain type.

Currently we loop through each notice and dispatch the removeNotice action for each of them. It would be more performant if we could dispatch a single action and achieve the same result.

How?

Adds a new action creator and the action is handled in a new case in the reducer.

Testing Instructions

  1. Run the automated tests.
  2. Run the following code in your console:
wp.data.dispatch( 'core/notices' ).createNotice( 'error', 'Test 1', { id: 'test1' } );
wp.data.dispatch( 'core/notices' ).createNotice( 'error', 'Test 2', { id: 'test2' } );
wp.data.dispatch( 'core/notices' ).createNotice( 'error', 'Test 3', { id: 'test3' } );
wp.data.dispatch( 'core/notices' ).removeNotices( [ 'test1', 'test2', 'test3' ] );
wp.data.select( 'core/notices' ).getNotices();
  1. Expect the select to return an empty array.
  2. Run the following code:
wp.data.dispatch( 'core/notices' ).createNotice( 'error', 'Test 1', { context: 'foo', id: 'foo1' } );
wp.data.dispatch( 'core/notices' ).createNotice( 'error', 'Test 2', { context: 'bar', id: 'bar1' } );
wp.data.dispatch( 'core/notices' ).removeNotices( [ 'foo1', 'bar1' ], 'foo' );
wp.data.select( 'core/notices' ).getNotices( 'bar' );
  1. Expect the select to return an array containing the Test 2 error.

@opr opr mentioned this pull request Apr 8, 2022
Merged
4 tasks
@nerrad nerrad added [Type] Performance Related to performance efforts [Package] Notices /packages/notices labels Sep 11, 2022
@opr opr force-pushed the add/remove-notices branch from 08082b4 to 927fb58 Compare September 11, 2022 17:06
@opr opr requested a review from fabiankaegy as a code owner September 11, 2022 17:06
@ryanwelcher
Copy link
Contributor

ryanwelcher commented Sep 11, 2022

Thank you for putting this together!

Would you be able to add an example to the docblock? It does a long way in helping developers wanting to use this action. Here is an example for reference

nerrad
nerrad approved these changes Sep 11, 2022
View reviewed changes
Copy link
Contributor

@nerrad nerrad 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, test well. E2E tests seem to be unrelated.

@opr
Copy link
Contributor Author

opr commented Sep 11, 2022

@ryanwelcher great idea, thanks. I've updated the PR to include an example.

@opr opr mentioned this pull request Sep 11, 2022
Merged
@opr opr force-pushed the add/remove-notices branch from 20f5580 to 62d38b2 Compare May 25, 2023 12:53
@opr opr requested a review from ndiego as a code owner May 25, 2023 12:53
@opr
Copy link
Contributor Author

opr commented May 25, 2023

@nerrad this has been rebased and tested. On my end it works OK.

@nerrad
Copy link
Contributor

nerrad commented May 30, 2023 •
edited
Loading

LGTM, merging. Actually, will you add a CHANGELOG entry please?

@nerrad nerrad enabled auto-merge (squash) May 30, 2023 13:59
@nerrad nerrad merged commit 98c5147 into WordPress:trunk May 30, 2023
@github-actions github-actions bot added this to the Gutenberg 16.0 milestone May 30, 2023
@opr opr deleted the add/remove-notices branch May 30, 2023 15:11
sethrubenstein pushed a commit to pewresearch/gutenberg that referenced this pull request Jul 13, 2023
@opr @sethrubenstein
[@wordpress/notices] Add a new action removeNotices which allows …
fad4646 
…bulk removal of notices (WordPress#39940)

* Add removeNotices action creator

* Handle REMOVE_NOTICES in reducer

* Add tests for reducer and actions

* Move test to better position and add one for multiple contexts

* Use .filter instead of reject

* Add example component to doc block

* Add changelog entry

* Correct spacing on list item

* Add correct changelog heading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nerrad nerrad nerrad approved these changes

@ajitbohra ajitbohra Awaiting requested review from ajitbohra ajitbohra is a code owner

@mkaz mkaz Awaiting requested review from mkaz

@ryanwelcher ryanwelcher Awaiting requested review from ryanwelcher

@juanmaguitar juanmaguitar Awaiting requested review from juanmaguitar juanmaguitar is a code owner

@fabiankaegy fabiankaegy Awaiting requested review from fabiankaegy fabiankaegy is a code owner

@ndiego ndiego Awaiting requested review from ndiego

Assignees

No one assigned

Labels

[Package] Notices /packages/notices [Type] Performance Related to performance efforts

Projects

None yet

Milestone

Gutenberg 16.0

Development

Successfully merging this pull request may close these issues.

3 participants

@opr @ryanwelcher @nerrad