Make WordPress Core

Opened 23 months ago

Closed 23 months ago

Last modified 21 months 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 23 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 23 months ago.
With patch applied

Download all attachments as: .zip

Change History (14)

#1 @sabernhardt
23 months ago

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

@sabernhardt
23 months ago

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

#2 @sabernhardt
23 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 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 23 months ago by sabernhardt (previous) (diff)

#3 @poena
23 months ago

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

#4 @poena
23 months ago

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

@poena
23 months ago

With patch applied

#5 @sabernhardt
23 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.


23 months ago

#7 @joedolson
23 months ago

  • Keywords commit added

#8 @joedolson
23 months ago

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

#9 @joedolson
23 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
23 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
23 months ago

In 52182:

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

Follow up to [52149].

See #54392.

#12 @sabernhardt
21 months ago

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