WordPress.org

Make WordPress Core

Opened 19 months ago

Closed 3 months ago

Last modified 3 months ago

#46829 closed task (blessed) (fixed)

Denote the special pages in Customizer Menu editor

Reported by: garrett-eclipse Owned by: garrett-eclipse
Milestone: 5.5 Priority: normal
Severity: normal Version:
Component: Menus Keywords: has-screenshots
Focuses: ui, administration, privacy Cc:

Description

Hello,

It was mentioned while working through #37782 that the special page labels (Front Page, Posts Page and Privacy Policy Page) would also be beneficial on the Customizer Menu editor.

Opening this to branch from there as it's focus for 5.2 will be the standard menu editor.

Cheers

Attachments (16)

Screen Shot 2019-04-08 at 12.31.39 AM.png (32.3 KB) - added by garrett-eclipse 19 months ago.
Customizer Menu w/ an important page. Would be nice to label it
Screen Shot 2018-09-25 at 12.49.52 AM.png (56.1 KB) - added by garrett-eclipse 19 months ago.
Special page labels being introduced to menu items in #37782
46829.diff (2.9 KB) - added by audrasjb 17 months ago.
Add privacy policy page specific page to customizer menu liste and search tool
Capture d’écran 2019-05-19 à 11.09.24.png (33.1 KB) - added by audrasjb 17 months ago.
Privacy Policy page in Pages List
Capture d’écran 2019-05-19 à 11.08.09.png (43.5 KB) - added by audrasjb 17 months ago.
Privacy Policy page in Search Tool
46829.1.diff (2.9 KB) - added by audrasjb 15 months ago.
Patch refreshed
46829.2.diff (2.9 KB) - added by audrasjb 15 months ago.
Coding standards + add condition if check if the privacy policy page exists
Capture d’écran 2019-09-21 à 17.25.04.png (108.0 KB) - added by audrasjb 13 months ago.
46829.3.diff (6.8 KB) - added by garrett-eclipse 13 months ago.
Refreshed patch to align with sister ticket #37782
Screen Shot 2019-10-02 at 1.17.41 AM.png (47.7 KB) - added by garrett-eclipse 13 months ago.
All the important pages labelled according to get_post_states
Screen Shot 2019-10-02 at 1.18.10 AM.png (53.0 KB) - added by garrett-eclipse 13 months ago.
Pages added in customizer still take precedence
Screen Shot 2019-10-02 at 10.11.38 AM.png (17.3 KB) - added by garrett-eclipse 13 months ago.
Search also applies the label, and if you have Front page set 'Home' is suppressed
46829.4.diff (6.1 KB) - added by garrett-eclipse 9 months ago.
Minor refresh
46829.5.diff (6.2 KB) - added by garrett-eclipse 9 months ago.
Some additional cleanup for CS. Also removed second call to get_post_states to use $post_states as it's made available before the conditional. No change in actual functionality
Screen Shot 2020-05-05 at 2.17.18 PM.png (237.1 KB) - added by whyisjake 6 months ago.
46829.6.diff (1.5 KB) - added by dlh 3 months ago.

Download all attachments as: .zip

Change History (53)

@garrett-eclipse
19 months ago

Customizer Menu w/ an important page. Would be nice to label it

@garrett-eclipse
19 months ago

Special page labels being introduced to menu items in #37782

#1 @audrasjb
18 months ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 5.2.1
  • Owner set to audrasjb
  • Status changed from new to assigned
  • Type changed from enhancement to defect (bug)

Hi @garrett-eclipse thanks for the ticket

Moving to the next minor since that would be nice to fix this inconsistency issue in 5.2.x branch.

#2 @garrett-eclipse
18 months ago

  • Milestone changed from 5.2.1 to 5.3

Thanks @audrasjb, sadly #37782 got pushed till 5.3 so I hope you don't mind I have bumped this to match and allow us to release the updates in tandem.

Last edited 18 months ago by garrett-eclipse (previous) (diff)

@audrasjb
17 months ago

Add privacy policy page specific page to customizer menu liste and search tool

@audrasjb
17 months ago

Privacy Policy page in Pages List

@audrasjb
17 months ago

Privacy Policy page in Search Tool

#3 @audrasjb
17 months ago

  • Focuses privacy added
  • Keywords has-patch added; needs-patch removed

#4 @audrasjb
17 months ago

  • Keywords has-screenshots added

#5 @audrasjb
15 months ago

  • Keywords needs-refresh added

@audrasjb
15 months ago

Patch refreshed

@audrasjb
15 months ago

Coding standards + add condition if check if the privacy policy page exists

#6 @audrasjb
15 months ago

  • Keywords dev-feedback added; needs-refresh removed

#7 @audrasjb
13 months ago

  • Keywords commit added; dev-feedback removed

The patch still applies cleanly on trunk.
Tested again and the patch looks ready for commit in my opinion.

#8 @desrosj
13 months ago

  • Keywords reporter-feedback added; commit removed

I have a few questions I'd like clarified before this can be deemed ready.

@garrett-eclipse Can you clarify the original intent of the ticket? As I understand it, you are saying that all three of those special page types should be denoted in the Customizer and your screenshot just happened to show the Privacy Policy. Is that correct?

Also, although these pages are "special", I'm not convinced that adding them to the menu as a custom type is the correct behavior. Home is kind of an outlier because there usually is not an actual page in WordPress for "home".

For example, say someone selects the page selected as the Privacy Policy page on their site, and then a week later, they change the URL slug for that page. The link would then be a 404. If the menu item was added as a regular Page item, the URL would update automatically.

@garrett-eclipse
13 months ago

Refreshed patch to align with sister ticket #37782

@garrett-eclipse
13 months ago

All the important pages labelled according to get_post_states

@garrett-eclipse
13 months ago

Pages added in customizer still take precedence

@garrett-eclipse
13 months ago

Search also applies the label, and if you have Front page set 'Home' is suppressed

#9 @garrett-eclipse
13 months ago

  • Keywords needs-testing added; reporter-feedback removed
  • Owner changed from audrasjb to garrett-eclipse
  • Status changed from assigned to accepted

Thanks for the ping @desrosj my goals here was to align with #37782 which introduces the get_post_states function and prioritizes the 3 important pages (Front Page, Posts Page, Privacy Policy Page).

@audrasjb thank you very much for the initial patch pointed me in the right direction there. But I agree we shouldn't be using a custom link type for these pages as @desrosj mentioned it can cause 404's on slug changes.

The Customizer approach and the core menu approach are very similar so I've migrated most of the code from #37782 into patch 46829.3.diff which addresses the following;

  1. Introduces the important pages at the top of the list.
  2. Updates the label on important pages on the list and in search to use get_post_states and fall page to the post_type singular name.
  3. I found a bug where the Home custom page stays even if you have a homepage, I've put the same checks as #37782 in both the load and search functions so Home will only be present if no Front Page is set.
  4. I kept the user-created menu nodes at the top above all so when created they don't get lost.
  5. In #37782 there was some code to set special flags on the objects, I avoided these as we don't use the objects aside from cloning them into the $items[] array so those flags wouldn't be present.
  6. In #37782 there was also a reset if the important pages were all the pages, this isn't necessary in the customizer version as the merge will end up with the same results with or without it.
  7. In #37782 we made the label bold, attempting this hear didn't have the desired effect and caused it to look odd as the title was already bold so I didn't carry over that CSS here, how the label weight is now, works much nicer IMO.

Aside form #5 & #6 this is a code re-implementation of the committed #37782 and would be nice to have it join it's sister in trunk.

Please test and review. I feel if we can get another test on this we should be good to move into commit.
CC @whyisjake - Flagging you as you committed #37782 so might want to have a review here.

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


13 months ago

#11 @audrasjb
13 months ago

  • Keywords early added
  • Milestone changed from 5.3 to 5.4

Hi,
Thanks for the work done on this ticket.
Unfortunately, we are now at Beta 3 in the release cycle. I'm moving this ticket to 5.4 early milestone.
Feel free to discuss it if you feel this ticket could land before 5.3 Release Candidate 1.

This ticket was mentioned in Slack in #core by david.baumwald. View the logs.


9 months ago

#13 @audrasjb
9 months ago

  • Keywords needs-dev-note added

@garrett-eclipse
9 months ago

Minor refresh

#14 @garrett-eclipse
9 months ago

  • Keywords early removed

I've made a minor refresh in 46829.4.diff to update some spacing and comments and remove the change to src/wp-admin/includes/class-walker-nav-menu-edit.php as it's unrelated to this change.

The patch applies cleanly and introduces the special pages handling to the Customizer menus.

@audrasjb or @desrosj could you review and test, this is good to go in my books.

Note: I removed the early tag as this can be merged anytime before RC and doesn't need much special handling as there doesn't seem to be any back-compat potential here.

@garrett-eclipse
9 months ago

Some additional cleanup for CS. Also removed second call to get_post_states to use $post_states as it's made available before the conditional. No change in actual functionality

#15 @audrasjb
9 months ago

It looks good to go on my side! Thanks @garrett-eclipse

#16 @garrett-eclipse
9 months ago

  • Keywords commit added; needs-testing removed

Thanks @audrasjb I appreciate you testing let's move this to committers review.

This ticket was mentioned in Slack in #core by david.baumwald. View the logs.


8 months ago

#18 @garrett-eclipse
8 months ago

  • Keywords bumped early added
  • Milestone changed from 5.4 to 5.5
  • Type changed from defect (bug) to enhancement

Moving to 5.5 early

This ticket was mentioned in Slack in #core by david.baumwald. View the logs.


6 months ago

This ticket was mentioned in Slack in #core by david.baumwald. View the logs.


6 months ago

This ticket was mentioned in Slack in #core by david.baumwald. View the logs.


6 months ago

#22 @whyisjake
6 months ago

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

In 47763:

Menus: Denote the special pages in the Customizer menu editor.

The special pages here are the pages that are used for the Privacy Policy, Home, and the Posts page.

Fixes #46829.
Props garrett-eclipse, audrasjb.

#23 @whyisjake
6 months ago

I didn't want to block the merge, but it might be worth ensuring those labels are brought up a level in the customizer screen too @garrett-eclipse.

#24 @SergeyBiryukov
6 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening to address comment:23.

This ticket was mentioned in Slack in #core by david.baumwald. View the logs.


5 months ago

#26 @whyisjake
5 months ago

  • Keywords has-patch commit removed

This ticket was mentioned in Slack in #core by david.baumwald. View the logs.


4 months ago

#28 @audrasjb
4 months ago

  • Keywords early removed

Removing early keyword.

#29 @whyisjake
4 months ago

  • Type changed from enhancement to task (blessed)

@dlh
3 months ago

#30 follow-up: @dlh
3 months ago

@audrasjb @garrett-eclipse I see what you mean about this last Customizer piece being tricky. I don't have any Customizer-specific ideas about how to handle it, unfortunately.

46829.6.diff is a tentative approach, but it still involves some logic duplication, and I haven't been able to completely think through any backwards compatibility implications.

#31 @audrasjb
3 months ago

  • Keywords needs-dev-note removed

Not sure it deserves a dev note since it doesn't introduce any new function or filter WordPress developers and/or Plugin authors can use. Not sure it introduces any breaking change for them as well :)
Feel free to re-add the keyword if you think I'm wrong :-)

#32 @audrasjb
3 months ago

  • Keywords needs-dev-note added

needs-dev-noteadded back as it will deserver a small place in the miscellaneous changes dev note :)

This ticket was mentioned in Slack in #core by whyisjake. View the logs.


3 months ago

#34 in reply to: ↑ 30 @SergeyBiryukov
3 months ago

Replying to dlh:

@audrasjb @garrett-eclipse I see what you mean about this last Customizer piece being tricky. I don't have any Customizer-specific ideas about how to handle it, unfortunately.

It looks like moving get_post_states() from wp-admin/includes/template.php to wp-includes/post.php, so that it's also available on frontend, is enough for the existing code in wp_setup_nav_menu_item() added in [47211] to handle this without any additional changes to the Customizer.

#35 @SergeyBiryukov
3 months ago

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

In 48619:

Posts, Post Types: Make get_post_states() available on frontend.

This allows special pages to be denoted as such when editing a menu in the Customizer.

This applies to the Front Page, Posts Page, and Privacy Policy Page.

Follow-up to [47211], [47213], [47763].

Props dlh, whyisjake, SergeyBiryukov.
Fixes #46829. See #49374.

#36 @SergeyBiryukov
3 months ago

In 48620:

Posts, Post Types: Move get_post_states() back to the admin for now, require the file in WP_Customize_Nav_Menus::customize_register() instead.

This provides a minor performance improvement by only running the function in contexts where it's needed.

Follow-up to [47211], [47213], [47763], [48619].

See #46829, #49374.

#37 @desrosj
3 months ago

  • Keywords bumped needs-dev-note removed

Looking this one over, I'm not sure I see anything here that needs to be detailed in a developer note. I'm going to remove the tag.

If I am missing something specific that should be noted, (how to properly use the new feature, how to adjust the behavior of the new feature, etc.), just let me know! But it seems that this change will mostly "just work" without any changes required by plugin/theme devs.

Note: See TracTickets for help on using tickets.