WordPress.org

Make WordPress Core

Opened 7 months ago

Last modified 5 weeks ago

#46829 accepted defect (bug)

Denote the special pages in Customizer Menu editor

Reported by: garrett-eclipse Owned by: garrett-eclipse
Milestone: 5.4 Priority: normal
Severity: normal Version:
Component: Menus Keywords: has-patch has-screenshots needs-testing early
Focuses: ui, administration, privacy Cc:
PR Number:

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 (12)

Screen Shot 2019-04-08 at 12.31.39 AM.png (32.3 KB) - added by garrett-eclipse 7 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 7 months ago.
Special page labels being introduced to menu items in #37782
46829.diff (2.9 KB) - added by audrasjb 6 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 6 months ago.
Privacy Policy page in Pages List
Capture d’écran 2019-05-19 à 11.08.09.png (43.5 KB) - added by audrasjb 6 months ago.
Privacy Policy page in Search Tool
46829.1.diff (2.9 KB) - added by audrasjb 4 months ago.
Patch refreshed
46829.2.diff (2.9 KB) - added by audrasjb 4 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 8 weeks ago.
46829.3.diff (6.8 KB) - added by garrett-eclipse 6 weeks 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 6 weeks 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 6 weeks 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 6 weeks ago.
Search also applies the label, and if you have Front page set 'Home' is suppressed

Download all attachments as: .zip

Change History (23)

@garrett-eclipse
7 months ago

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

@garrett-eclipse
7 months ago

Special page labels being introduced to menu items in #37782

#1 @audrasjb
7 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
6 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 6 months ago by garrett-eclipse (previous) (diff)

@audrasjb
6 months ago

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

@audrasjb
6 months ago

Privacy Policy page in Pages List

@audrasjb
6 months ago

Privacy Policy page in Search Tool

#3 @audrasjb
6 months ago

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

#4 @audrasjb
6 months ago

  • Keywords has-screenshots added

#5 @audrasjb
4 months ago

  • Keywords needs-refresh added

@audrasjb
4 months ago

Patch refreshed

@audrasjb
4 months ago

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

#6 @audrasjb
4 months ago

  • Keywords dev-feedback added; needs-refresh removed

#7 @audrasjb
8 weeks 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
7 weeks 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
6 weeks ago

Refreshed patch to align with sister ticket #37782

@garrett-eclipse
6 weeks ago

All the important pages labelled according to get_post_states

@garrett-eclipse
6 weeks ago

Pages added in customizer still take precedence

@garrett-eclipse
6 weeks ago

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

#9 @garrett-eclipse
6 weeks 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.


6 weeks ago

#11 @audrasjb
5 weeks 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.

Note: See TracTickets for help on using tickets.