WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#25430 closed defect (bug) (fixed)

Hooks Docs: wp-admin/options-reading.php

Reported by: siobhyb Owned by: DrewAPicture
Milestone: 3.7 Priority: normal
Severity: normal Version:
Component: Inline Docs Keywords: needs-patch
Focuses: Cc:
PR Number:

Description

First pass at inline docs for wp-admin/options-reading.php.

Attachments (5)

options-reading.2.patch (1.3 KB) - added by siobhyb 6 years ago.
Pass 2 at wp-admin/option-reading.php
options-reading.php.patch (1.3 KB) - added by siobhyb 6 years ago.
Pass 2 at wp-admin/option-reading.php
options-reading.patch (1.3 KB) - added by siobhyb 6 years ago.
wp-admin/option-reading.php
25430.diff (1.5 KB) - added by DrewAPicture 6 years ago.
Third pass
25430.2.diff (1.5 KB) - added by DrewAPicture 6 years ago.
s/This action/Hooking to this action

Download all attachments as: .zip

Change History (12)

#1 @DrewAPicture
6 years ago

  • Keywords needs-patch added; has-patch removed
  • Owner set to DrewAPicture
  • Status changed from new to reviewing
  • Type changed from enhancement to defect (bug)

Hi Siobhan, thanks for the patch :)

So, this hook is kind of interesting because it mostly serves to provide backward-compatibility for adding additional-level privacy setting to the fieldset that used to live in the now-defunct options-privacy.php See #16416 for when those settings screens were consolidated.

And if you look at the current code, you'll notice that the section of the table that the action runs in is only displayed if that hook has a callback hooked to it.

The logic behind it is explained pretty well in the commit message for [21838]:

When blog_public only has two values (as judged by whether the blog_privacy_selector action is used), convert from radio buttons to a checkbox, and rename from 'Site Visibility' to a more specific 'Search Engine Visibility'.

The reason I bring this up is that we should find a way to succinctly explain this behavior in the hook doc, probably using a long (but not too long!) description.

Also, in the future, could you please remember to generate patches from the WordPress root? :)

#2 @siobhyb
6 years ago

Hi Drew. I've attached the updated patch with the long description added. I borrowed heavily from the original commit message!

This action is required in order to add additional values to the privacy options form. When called, the form's default radio buttons are converted to check boxes, and the 'Site Visibility' heading is changed to the more specific 'Search Engine Visibility.'

I suspect it may still require some editing, let me know. Thanks for your help. :)

@siobhyb
6 years ago

Pass 2 at wp-admin/option-reading.php

@siobhyb
6 years ago

Pass 2 at wp-admin/option-reading.php

#3 @siobhyb
6 years ago

Sorry, had some formatting issues. "options-reading.patch​" is the latest.

Last edited 6 years ago by siobhyb (previous) (diff)

@siobhyb
6 years ago

wp-admin/option-reading.php

@DrewAPicture
6 years ago

Third pass

#4 @DrewAPicture
6 years ago

Looking at the long description in options-reading.patch, it's actually backwards. The default behavior is to show a checkbox, and if hooked, the action flips the form to radio buttons, opening the door for additional options to be added.

I reworked the language in 25430.diff. What do you think?

@DrewAPicture
6 years ago

s/This action/Hooking to this action

#5 @DrewAPicture
6 years ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 25717:

Inline documentation for wp-admin/options-reading.php.

Props siobhyb for the initial patch.
Fixes #25430.

#6 @DrewAPicture
6 years ago

  • Milestone changed from Awaiting Review to 3.7

#7 @siobhyb
6 years ago

Much better, thanks for improving.

Note: See TracTickets for help on using tickets.