WordPress.org

Make WordPress Core

Opened 15 months ago

Last modified 12 hours ago

#46829 reopened task (blessed)

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 needs-dev-note bumped
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 15 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 15 months ago.
Special page labels being introduced to menu items in #37782
46829.diff (2.9 KB) - added by audrasjb 14 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 14 months ago.
Privacy Policy page in Pages List
Capture d’écran 2019-05-19 à 11.08.09.png (43.5 KB) - added by audrasjb 14 months ago.
Privacy Policy page in Search Tool
46829.1.diff (2.9 KB) - added by audrasjb 11 months ago.
Patch refreshed
46829.2.diff (2.9 KB) - added by audrasjb 11 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 10 months ago.
46829.3.diff (6.8 KB) - added by garrett-eclipse 9 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 9 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 9 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 9 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 5 months ago.
Minor refresh
46829.5.diff (6.2 KB) - added by garrett-eclipse 5 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 2 months ago.
46829.6.diff (1.5 KB) - added by dlh 12 hours ago.

Download all attachments as: .zip

Change History (46)

@garrett-eclipse
15 months ago

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

@garrett-eclipse
15 months ago

Special page labels being introduced to menu items in #37782

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

@audrasjb
14 months ago

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

@audrasjb
14 months ago

Privacy Policy page in Pages List

@audrasjb
14 months ago

Privacy Policy page in Search Tool

#3 @audrasjb
14 months ago

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

#4 @audrasjb
14 months ago

  • Keywords has-screenshots added

#5 @audrasjb
11 months ago

  • Keywords needs-refresh added

@audrasjb
11 months ago

Patch refreshed

@audrasjb
11 months ago

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

#6 @audrasjb
11 months ago

  • Keywords dev-feedback added; needs-refresh removed

#7 @audrasjb
10 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
10 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
9 months ago

Refreshed patch to align with sister ticket #37782

@garrett-eclipse
9 months ago

All the important pages labelled according to get_post_states

@garrett-eclipse
9 months ago

Pages added in customizer still take precedence

@garrett-eclipse
9 months ago

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

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


9 months ago

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


6 months ago

#13 @audrasjb
6 months ago

  • Keywords needs-dev-note added

@garrett-eclipse
5 months ago

Minor refresh

#14 @garrett-eclipse
5 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
5 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
5 months ago

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

#16 @garrett-eclipse
5 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.


4 months ago

#18 @garrett-eclipse
4 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.


3 months ago

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


3 months ago

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


2 months ago

#22 @whyisjake
2 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
2 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
2 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.


8 weeks ago

#26 @whyisjake
5 weeks ago

  • Keywords has-patch commit removed

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


4 weeks ago

#28 @audrasjb
2 weeks ago

  • Keywords early removed

Removing early keyword.

#29 @whyisjake
3 days ago

  • Type changed from enhancement to task (blessed)

@dlh
12 hours ago

#30 @dlh
12 hours 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.

Note: See TracTickets for help on using tickets.