WordPress.org

Make WordPress Core

Opened 5 months ago

Closed 5 months ago

#38790 closed defect (bug) (fixed)

Improve the new labels for the post templates select

Reported by: afercia Owned by: afercia
Milestone: 4.7 Priority: normal
Severity: normal Version: 4.7
Component: Posts, Post Types Keywords: has-screenshots has-patch commit
Focuses: template Cc:

Description

The new labels introduced in [38951] for the Post/Page/Attachment Attributes Meta box make perfectly sense when they're used for the Meta box heading. By the way, they shouldn't be used also for the hidden label associated with the "template" select:

https://cldup.com/XnPkKd2jNI.png

The ones used for the select would need new, more meaningful text. Worth noting the previous one was "Page Template" because only pages had templates. Now, even a generic "Template" could probably be appropriate.

Attachments (3)

38790.diff (2.7 KB) - added by afercia 5 months ago.
38790.2.diff (2.7 KB) - added by afercia 5 months ago.
38790.3.diff (2.8 KB) - added by afercia 5 months ago.

Download all attachments as: .zip

Change History (13)

#1 @afercia
5 months ago

The whole meta box could probably be improved a bit, I don't see a good reason why the visible "titles" (parent, template, order) should then be repeated for the hidden labels. Probably something for 4.8 :)

@afercia
5 months ago

#2 follow-up: @afercia
5 months ago

  • Keywords has-patch added; needs-patch removed

Well, after all this is a nice opportunity to improve things a bit. I'd propose to:

  • make the labels visible (and remove the existing visible "titles")
  • use a generic string "Template"
  • slightly adjust the CSS

Things to check:

  • the position of the "Template" label in respect of the action hook
  • escaping with esc_html_e()

Any feedback welcome.

#3 in reply to: ↑ 2 @ocean90
5 months ago

Replying to afercia:

  • escaping with esc_html_e()

There is no need to use esc_html_e() for the labels.

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


5 months ago

@afercia
5 months ago

#5 @afercia
5 months ago

Refreshed patch to use _e() instead of esc_html_e().

My only concern here is the position of the "Template" label in the source. From an accessibility perspective, there shouldn't be anything between the label and the select so it would be better to move it after the page_attributes_meta_box_template hook.

#6 @swissspidy
5 months ago

No objections to improving things here from my side. A before/after screenshot would be nice.

My only concern here is the position of the "Template" label in the source. From an accessibility perspective, there shouldn't be anything between the label and the select.

I wonder what use cases there are for the page_attributes_meta_box_template hook. Should it run before the label or rather after the input?

#7 @afercia
5 months ago

The hook was added in [34340] to add an "Edit link" for templates. Seems it was added for just a single use case... and in the middle of a not so ideal HTML. We have to deal with it now, will try to see what can be reasonably done here.

In the meanwhile, screenshot before and after:

https://cldup.com/dbOfFTZR2S.png

@afercia
5 months ago

#8 @afercia
5 months ago

  • Keywords commit added

Refreshed patch. Restoring the paragraphs to wrap the labels is not ideal but still valid HTML and would allow to use the hook without requiring any change. See screenshots below:

https://cldup.com/4vafwh4use.png

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


5 months ago

#10 @afercia
5 months ago

  • Owner set to afercia
  • Resolution set to fixed
  • Status changed from new to closed

In 39247:

Accessibility: Improve the Post Attributes meta box fields labels.

With [38951] the support for custom page templates has been extended to all post
types. By making the visible meta box titles real labels, accessibility improves
for all users and form fields have meaningful labels.

Fixes #38790.

Note: See TracTickets for help on using tickets.