WordPress.org

Make WordPress Core

Opened 17 months ago

Last modified 13 months ago

#37331 assigned defect (bug)

New site form has non-required fields that have to be filled in

Reported by: Shelob9 Owned by: rianrietveld
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Networks and Sites Keywords: required-fields has-patch
Focuses: ui, accessibility, administration, multisite Cc:

Description

In multi-site's New Site form, there are several fields that are not required to submit the form (required and aria-required attributes are not set) but the form will fail server-side validation if these fields, such as admin email are not filled in.

The form really should have client-side and server-side validation for UX and accessibility reasons.

Attachments (3)

37331.diff (3.5 KB) - added by dipesh.kakadiya 13 months ago.
37331-1.diff (3.7 KB) - added by jackreichert 13 months ago.
Building upon @dipesh.kakadiya's patch added note: "Required fields are marked"
37331-2.diff (3.4 KB) - added by jackreichert 13 months ago.

Download all attachments as: .zip

Change History (23)

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


16 months ago

#2 @afercia
16 months ago

  • Milestone changed from Awaiting Review to Future Release

Definitely there's a bit of inconsistency across forms in the admin. Many of them should be reviewed. Some years ago an attempt was made to use the HTML5 'required' attribute but seens browsers implementation was a bit poor at that time. See for example [29030] and #22183.

At the very least, I'd say there should be a visual indication of the required fields and aria-required. Further improvements should be carefully evaluated especially thinking at mobile devices, see the discussion on the ticket mentioned above.

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


15 months ago

#5 @rianrietveld
15 months ago

  • Milestone changed from Future Release to 4.7

Discussed this in the wpa11y bug scrub:
We'll have a go infixing this for 4.7. together with #16206

#6 @jeremyfelt
14 months ago

  • Keywords needs-patch added

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


14 months ago

This ticket was mentioned in Slack in #core-multisite by flixos90. View the logs.


14 months ago

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


14 months ago

#10 @rianrietveld
14 months ago

  • Owner set to rianrietveld
  • Status changed from new to assigned

#11 follow-up: @rianrietveld
14 months ago

Discussion about how to indicate the required field at:
https://wordpress.slack.com/archives/accessibility/p1476119464001821

#12 @afercia
14 months ago

  • Keywords required-fields added

Related: #23870

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


14 months ago

#14 in reply to: ↑ 11 @jackreichert
13 months ago

Replying to rianrietveld:

Discussion about how to indicate the required field at:
https://wordpress.slack.com/archives/accessibility/p1476119464001821

I'm not sure the discussion is conclusive, did I understand correctly that it would be best to add sprintf( ' ' . __('Required fields are marked %s'), '<span class="required">⁠⁠⁠</span>' ), or are they likely to circle back to another convention?

Last edited 13 months ago by jackreichert (previous) (diff)

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


13 months ago

#16 @dipesh.kakadiya
13 months ago

  • Keywords has-patch added; needs-patch removed

@jackreichert
13 months ago

Building upon @dipesh.kakadiya's patch added note: "Required fields are marked"

#17 follow-up: @ocean90
13 months ago

novalide="novalidate" was added on purpose. Can you verify that it doesn't reveal the previous issue? See ticket:22183:6 and [20196].

I don't see the need to make '*' translatable (it's currently not even translatable in the description). I'm curious if we can't add the * with CSS ::after.

#18 in reply to: ↑ 17 @jackreichert
13 months ago

Replying to ocean90:

novalide="novalidate" was added on purpose. Can you verify that it doesn't reveal the previous issue? See ticket:22183:6 and [20196].

Good point, I'll put it back.

I don't see the need to make '*' translatable (it's currently not even translatable in the description). I'm curious if we can't add the * with CSS ::after.

I was trying to work with the discussion linked to above, but I couldn't find other places in the admin that use *, in new user it has (required) which would make the translation more relevant. I'm happy to update the patch to either.
Would you recommend css or revert to (required)?

#19 @jackreichert
13 months ago

I found that this convention is being used in https://core.trac.wordpress.org/browser/trunk/src/wp-admin/includes/media.php#L1475 so I updated the patch to reflect that.

#20 @afercia
13 months ago

  • Milestone changed from 4.7 to Future Release

Not enough time to discuss and establish a convention for all the required form fields in core, punting for now.

Note: See TracTickets for help on using tickets.