Make WordPress Core

Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#54392 closed defect (bug) (fixed)

Core themes need to display required text field information

Reported by: joedolson's profile joedolson Owned by: joedolson's profile 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 2 years 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 years ago.
With patch applied

Download all attachments as: .zip

Change History (14)

#1 @sabernhardt
2 years ago

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

@sabernhardt
2 years ago

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

#2 @sabernhardt
2 years 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 If the default links somehow do not belong in the form with these themes, however, the custom message argument could get rather messy here. Even if I want to use exactly the same required fields text as the core default argument now includes, that seems to require (copying and) re-writing the markup within the context of the theme.

Last edited 2 years ago by sabernhardt (previous) (diff)

#3 @poena
2 years ago

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

#4 @poena
2 years ago

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

@poena
2 years ago

With patch applied

#5 @sabernhardt
2 years 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 years ago

#7 @joedolson
2 years ago

  • Keywords commit added

#8 @joedolson
2 years ago

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

#9 @joedolson
2 years 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 years 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 years ago

In 52182:

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

Follow up to [52149].

See #54392.

#12 @sabernhardt
2 years ago

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