Make WordPress Core

Opened 5 years ago

Last modified 5 years ago

#44981 assigned enhancement

Make notice on Privacy Policy page to 'Check out our guide' dismissable

Reported by: garrett-eclipse's profile garrett-eclipse Owned by: garrett-eclipse's profile garrett-eclipse
Milestone: Future Release Priority: normal
Severity: normal Version: 4.9.6
Component: Privacy Keywords: has-patch has-screenshots needs-refresh
Focuses: Cc:

Description

This ticket branches from #44669 with this comment from @norcross;
https://core.trac.wordpress.org/ticket/44669?replyto=4#comment:4

The notice reference can be found here;
https://github.com/WordPress/WordPress/blob/f7ba175491b725a5f3d636072c8b432774d38ae3/wp-admin/includes/misc.php#L1613-L1629

With this notice the is-dismissable class could be added;
https://codex.wordpress.org/Plugin_API/Action_Reference/admin_notices

If a new Privacy Policy page is created this notice should re-appear until dismissed.

Attachments (4)

0Yv8SNzA.png (97.3 KB) - added by garrett-eclipse 5 years ago.
Notice to make dismissable
44981.diff (499 bytes) - added by garrett-eclipse 5 years ago.
Make notice dismissable
44981.2.diff (1.1 KB) - added by garrett-eclipse 5 years ago.
Screen Shot 2019-01-09 at 10.53.57 PM.png (44.4 KB) - added by garrett-eclipse 5 years ago.
Notice w/ Close Button

Download all attachments as: .zip

Change History (19)

@garrett-eclipse
5 years ago

Notice to make dismissable

@garrett-eclipse
5 years ago

Make notice dismissable

#1 @garrett-eclipse
5 years ago

  • Keywords has-patch added

Added the .is-dismissable class to the privacy policy notice.

#2 @garrett-eclipse
5 years ago

  • Owner set to garrett-eclipse
  • Status changed from new to assigned

#3 @garrett-eclipse
5 years ago

  • Milestone changed from Awaiting Review to 5.0.1
  • Version set to 4.9.6

#4 @pento
5 years ago

  • Milestone changed from 5.0.1 to 5.0.2

#5 @pento
5 years ago

  • Milestone changed from 5.0.2 to 5.0.3

#6 @audrasjb
5 years ago

  • Keywords commit added

This ticket is triaged in milestone 5.0.3. The patch looks good, small and self-contained enough to land in the next minor. Adding commit keyword to see if it can land in 5.0.3.

#7 @desrosj
5 years ago

  • Milestone changed from 5.0.3 to 5.1

This falls outside the 5.0.3 scope (block editor related updates, major bugs, regressions), so let's tackle it in 5.1.

#8 @pento
5 years ago

  • Keywords commit removed

This patch doesn't add the close button for me. @garrett-eclipse, could you investigate, please?

#9 @pento
5 years ago

  • Keywords needs-refresh added
  • Milestone changed from 5.1 to Future Release

@garrett-eclipse
5 years ago

Notice w/ Close Button

#10 @garrett-eclipse
5 years ago

  • Keywords has-screenshots dev-feedback needs-testing added; needs-refresh removed

Thanks @pento,

Sorry about that, had it working in /build and hadn't applied to /src before making that original diff.

Latest patch is correct with proper spelling of is-dismissible class and calls makeNoticesDismissible on wp-pp-notice to invoke the function to inject the button.

Minor notes;
Upon reload the notice will re-appear which is expected.
As well just this was started pre-5.0 so this notice currently only displays with Classic Editor enabled.

Please apply the latest and let me know if there's any issues.

#11 @pento
5 years ago

Thank you for refreshing the patch, @garrett-eclipse.

I think it needs to remember that the notice has been dismissed. As you noted, refreshing the page makes it reappear, which includes when you click "Save Draft". A dismissed notice that keeps coming back is a more frustrating experience than a non-dismissable notice. 😉

#12 @garrett-eclipse
5 years ago

Thanks for the feedback @pento I tend to agree would be nice to save the dismissed state of the notice there.

I can definitely work on that functionality but am hoping for a little direction if you don't mind.
To keep the notice dismissed I would be adding a flag either as a Cookie, Session data or User Meta and am wondering if there's an existing standard (maybe an example) for this in core?

As well whatever storage method is used I feel should get cleared when the policy text has changed so as to re-introduce the guide link.
If I do that it would make sense to be done here along side the policy_text_changed_notice notice which re-appears on the privacy.php screen.
And by here I mean - https://github.com/WordPress/WordPress/blob/51155f3989c29bbae5322b6cba58952a7c58b0ba/wp-admin/includes/misc.php#L1428

I'm leaning towards using User Meta over Cookie/Session data as it pertains across browsers/devices and is a more permanent dismissal but would love your thoughts.

Thanks

#13 @garrett-eclipse
5 years ago

  • Keywords needs-refresh added; dev-feedback needs-testing removed

#46098 introduces the notice to the block editor. The persistent dismiss will need to be hooked into both notices.

#14 @garrett-eclipse
5 years ago

Some notes from #46098 to help with persistence on Gutenberg version of the notice.

As the dismiss isn't persistent in that a reload will revive the notice we should remove the dismiss from this notice and address it along w/ the dismissal for the classic editor version in #44981.

Yeah, I agree. I think it should also be a feature built-in to the notices module, and could probably leverage existing persistence functionality offered to data stores (of which notices extends) via the data persistence plugin.

https://github.com/WordPress/gutenberg/tree/master/packages/data/src/plugins/persistence

This ticket was mentioned in Slack in #core-privacy by garrett-eclipse. View the logs.


5 years ago

Note: See TracTickets for help on using tickets.