search

WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
10.43k stars 4.17k forks source link

Regression in dismissible notices no longer "sticky" #32593

Open thomasplevy opened 3 years ago

thomasplevy commented 3 years ago

Description

My plugin LifterLMS uses dismissible notices to report validation issues with custom blocks. Perhaps not the intended use case for notices but it's the best solution I've come up with for reporting validation issues client-side in the block editor.

I noticed today while testing WP core 5.8 nightlies that dismissible notices are no longer "sticky" per https://github.com/WordPress/gutenberg/pull/32238

The casualty is that dismissible notices are no longer sticky (i.e. they no longer scroll with you down the page). But honestly I think that's an upgrade.

So it seems that this isn't exactly a bug but I wanted to raise the issue because I don't see any notes anywhere opposing this "casualty" and I wasn't sure the best way to raise my concern otherwise.

My concern here is that if a notice like this is added when a user is "below the fold" (or scrolled down so the user can't see the notice that's being displayed) they will simply not see the notice until they scroll back to the top of the screen.

In my case we're using wp.data.dispatch( 'core/editor' ).lockPostSaving( ... ) to lock a post with invalid data (and then unlock once the validation issue has been resolved). The notice is created to alert the user to validation issues. This means that a post may be locked for no discernible reason if the user is scrolled down when a validation issue is encountered.

So, from my perspective there's a regression here and I'm almost certain that this will be reported to LifterLMS as a bug in LifterLMS once 5.8 is released.

Step-by-step reproduction instructions

Head to a post and execute the following code:

wp.data.dispatch( 'core/notices' ).createInfoNotice( 'A pinned notice', { isDismissible: false } );
wp.data.dispatch( 'core/notices' ).createInfoNotice( 'A dismissible notice', { isDismissible: true } );

Then scroll

Expected behaviour

The second notice, "A dismissible notice" should "stick" as it did in WP 5.7

Actual behaviour

The second notice is "pinned" like the first notice.

Screenshots or screen recording (optional)

Expected behavior (ran on WP 5.7.2)

https://user-images.githubusercontent.com/1290739/121585824-e023c100-c9e7-11eb-81ff-179a3f8b8570.mp4

Actual behavior (ran on GB latest or on wp 5.8 latest nightly)

https://user-images.githubusercontent.com/1290739/121585827-e0bc5780-c9e7-11eb-9535-1fc7a920703e.mp4

WordPress information

Note: my videos have other plugins and a non-default theme running but I retested after recording with just gutenberg and twenty twenty-one and the same behavior occurs

Device information

stokesman commented 3 years ago

There’s #32488 that would fix this. We'll have to see how that's received.

thomasplevy commented 3 years ago

@stokesman thanks for the draft, I just tested it out and it looks good from my perspective. Hope that can get merged in!