Make WordPress Core

Opened 3 months ago

Closed 2 months ago

Last modified 3 weeks ago

#54392 closed defect (bug) (fixed)

Core themes need to display required text field information

Reported by: joedolson Owned by: joedolson
Milestone: 5.9 Priority: normal
Severity: normal Version:
Component: Bundled Theme Keywords: required-fields has-patch has-dev-note
Focuses: ui, accessibility Cc:

Description

Following #16206, two of the core themes replace/remove the text that explains required fields when user's are logged-in. These themes should keep this text so that the meaning of the asterisk is explained in text.

https://core.trac.wordpress.org/ticket/16206#comment:70

Attachments (2)

54392.diff (980 bytes) - added by sabernhardt 3 months ago.
keep (core) default logged_in_as text for Twenty Nineteen and Twenty Twenty-One
2019-2021-comment-area-logged-in.png (25.3 KB) - added by poena 2 months ago.
With patch applied

Download all attachments as: .zip

Change History (14)

#1 @sabernhardt
3 months ago

  • Keywords required-fields added
  • Milestone changed from Awaiting Review to 5.9

@sabernhardt
3 months ago

keep (core) default logged_in_as text for Twenty Nineteen and Twenty Twenty-One

#2 @sabernhardt
3 months ago

  • Keywords has-patch added; needs-patch removed

The logged_in_as text has been null since the initial commits for both Twenty Nineteen and Twenty Twenty-One, so I do not know the reasoning behind that choice.

The simplest way to fix the missing required fields message is to undo that removal.

If we change it to the required fields text only, however, the custom message argument could get rather messy here. Even if I want to use exactly the same text as the core default argument now includes, that seems to require (copying and) re-writing the markup within the context of the theme.

Version 1, edited 3 months ago by sabernhardt (previous) (next) (diff)

#3 @poena
2 months ago

I did not understand the last paragraph about "change it to the required fields text only,".

#4 @poena
2 months ago

Are you referring to that the logged in text and required fields text are on the same line?

@poena
2 months ago

With patch applied

#5 @sabernhardt
2 months ago

By setting the logged_in_as message to null, these themes removed the two default links plus their paragraph container. If there was a good reason for removing those, then the themes' custom logged_in_as argument would need the "Required fields are marked *" text on its own inside a paragraph. I edited the comment above so it's hopefully clearer.

This ticket was mentioned in Slack in #accessibility by ryokuhi. View the logs.


2 months ago

#7 @joedolson
2 months ago

  • Keywords commit added

#8 @joedolson
2 months ago

  • Owner set to joedolson
  • Status changed from new to accepted

#9 @joedolson
2 months ago

A little history: setting the comments logged_in_as argument to null dates back at least as far as version 1.0 of Seedlet, the theme used as a base for Twenty Twenty One. There's no evidence it was ever discussed during theme development, so we're considering it a circumstantial element inherited from prior art, rather than a theme decision.

#10 @joedolson
2 months ago

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

In 52149:

Bundled Theme: Display required text field in core themes.

Fix Twenty Nineteen and Twenty Twenty-One setting the logged_in_as parameter to null, which also eliminated the required field text. See #16206.

Props sabernhardt, poena.
Fixes #54392.

#11 @desrosj
2 months ago

In 52182:

Twenty Nineteen: Apply coding standards fix from running composer format.

Follow up to [52149].

See #54392.

#12 @sabernhardt
3 weeks ago

  • Keywords has-dev-note added; commit removed
Note: See TracTickets for help on using tickets.