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.

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

Fix the Privacy Policy help notice #11999

Merged
merged 4 commits into from
Nov 19, 2018
Merged

Fix the Privacy Policy help notice #11999

merged 4 commits into from
Nov 19, 2018

Conversation

desrosj
Copy link
Contributor

@desrosj desrosj commented Nov 16, 2018 •
edited by gziolo
Loading

Related issue: #10448.
Depends on #11604.

The new editor does not support the edit_form_after_title action hook. Because WordPress Core uses this hook to output the notice, it is not printed to the screen.

After #11604 is merged, legacy style admin notices (<div class="notice">...notice content...</div>) will be consumed by the Notices API and displayed. This change ensures that when #11604 is merged the privacy policy notice will appear again when editing the privacy policy page.

This PR moves the notice to the admin_notices action hook to ensure it is printed to the page and is consumable.

To test, ensure the Need help putting together your new Privacy Policy page? notice is output in the page source only when editing the privacy policy page. To further test, pull this change into the update/legacy-notices branch locally, and verify the notice correctly appears when editing the privacy policy page.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards. (NA)
  • My code has proper inline documentation.

@desrosj
Display the privacy policy help notice with the admin_notices actio…
d1fd713 
…n hook.

The new editor does not support the `edit_form_after_title` action hook. Because WordPress Core uses this hook to output the notice, it is not printed to the screen.

After WordPress#11604 is merged, legacy style admin notices (`<div class="notice">...notice content...</div>`) will be consumed by the Notices API and displayed. This change ensures that when WordPress#11604 is merged the privacy policy notice will appear again when editing the privacy policy page.
This was referenced Nov 16, 2018
Merged
Closed
@desrosj desrosj self-assigned this Nov 16, 2018
@desrosj desrosj modified the milestones: WordPress 5.0, 4.5 Nov 16, 2018
@desrosj desrosj added the Backwards Compatibility Issues or PRs that impact backwards compatability label Nov 16, 2018
@aduth
Copy link
Member

aduth commented Nov 16, 2018

Is there an associated Trac ticket for which this can be considered to be handled more gracefully internal to core itself, as part of the Gutenberg merge?

@desrosj
Copy link
Contributor Author

desrosj commented Nov 16, 2018

@aduth I should have mentioned that.

https://core.trac.wordpress.org/ticket/45057 is the associated Trac ticket.

Ideally, this would get fixed in Gutenberg 4.5 as well as being merged into core. I think I am envisioning the core action hook being changed to admin_notices in 5.0, and the Classic Editor plugin doing the opposite of what is done in this PR when 5.0 is released.

@desrosj
Only display the notice if the function is actually hooked to `edit_f…
0db1eae 
…orm_after_title`.

This prevents the notice from displaying twice if the notice has already been moved to the `admin_notices` hook (as would happen in https://core.trac.wordpress.org/attachment/ticket/45057/45057.diff).
@desrosj
Copy link
Contributor Author

desrosj commented Nov 17, 2018

I updated the conditional to make sure the notice is hooked to edit_form_after_title before displaying the notice. This would prevent the notice from showing twice if the plugin is active after this is ported over to WordPress Core.

@gziolo gziolo added the [Priority] High Used to indicate top priority items that need quick attention label Nov 19, 2018
@gziolo
Copy link
Member

gziolo commented Nov 19, 2018 •
edited
Loading

So this PR depends on #11604. On its own, it doesn't display the notice when I edit Privacy Policy page.

@gziolo gziolo added the [Status] Blocked Used to indicate that a current effort isn't able to move forward label Nov 19, 2018
@gziolo
Copy link
Member

gziolo commented Nov 19, 2018

I tested it with #11604 and it works perfectly fine:

screen shot 2018-11-19 at 11 10 13.

Let's merge it and make sure that it still works properly when #11604 lands.

@gziolo gziolo removed the [Status] Blocked Used to indicate that a current effort isn't able to move forward label Nov 19, 2018
@gziolo gziolo merged commit c5e146e into WordPress:master Nov 19, 2018
@desrosj desrosj deleted the 10448-privacy-policy-notice branch November 19, 2018 14:44
This was referenced Nov 19, 2018
Merged
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wpscholar wpscholar wpscholar approved these changes

Assignees

@desrosj desrosj

Labels
Backwards Compatibility Issues or PRs that impact backwards compatability [Priority] High Used to indicate top priority items that need quick attention
Projects
None yet
Milestone
4.5
Development

Successfully merging this pull request may close these issues.
4 participants
@desrosj @aduth @gziolo @wpscholar