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

Regression in dismissible notices no longer "sticky" #32593

Open
thomasplevy opened this issue Jun 10, 2021 · 2 comments
Open

Regression in dismissible notices no longer "sticky" #32593

thomasplevy opened this issue Jun 10, 2021 · 2 comments
Labels
[Package] Notices /packages/notices [Type] Bug An existing feature does not function as intended

Comments

@thomasplevy
Copy link
Contributor

thomasplevy commented Jun 10, 2021

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 #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)

notices-wp-572.mp4

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

notices-gbplugin.mp4

WordPress information

  • WordPress version: 5.7.2
  • Gutenberg version: 10.8.0
  • Are all plugins except Gutenberg deactivated? Yes
  • Are you using a default theme (e.g. Twenty Twenty-One)? Yes

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

  • Device: Desktop
  • Operating system: Ubuntu
  • Browser: Brave Version 1.8.96 Chromium: 81.0.4044.138 (Official Build) (64-bit)
@stokesman
Copy link
Contributor

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

@Mamaduka Mamaduka added [Package] Notices /packages/notices [Type] Bug An existing feature does not function as intended labels Jun 11, 2021
@thomasplevy
Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Notices /packages/notices [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants