Opened 8 years ago
Last modified 7 weeks ago
#30685 accepted enhancement
Better Login Error&Message Displaying
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 6.3 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Login and Registration | Keywords: | has-patch needs-refresh |
Focuses: | ui, accessibility | Cc: |
Description
Current way of displaying errors and messages of wp-login.php is not best, I think.
Because:
- Should not join messages with
<br />\n
. Each messages has independent meanings. So, this is far from optimal HTML markup. <p>
is used to enclose messages. But errors are using<div>
. This should be unified.
This patch fixes these 2 problems and includes its related changes.
Attachments (8)
Change History (47)
#4
@
8 years ago
From an accessibility perspective, while it's a slight improvement to go from using <br> to <p>, it would be a more meaningful step to use an unordered list with list items. The advantage is that screen reader users would be notified how many errors are in the list, and would know that there's more to read. With the <p> method, it's hypothetically possible that the user would read just the first error, then jump directly into the form to correct it, without realizing there was another error needing to be fixed.
Redoing this to produce list items would be a more accessible solution.
#5
follow-ups:
↓ 6
↓ 8
@
8 years ago
I'm sorry maybe I'm missing something, but which steps should be followed to reproduce this and have more than one error displayed at a time? Whatever I do, I get just on error at a time.
See screenshot, dumping $wp_error->get_error_codes()
#6
in reply to:
↑ 5
@
8 years ago
Replying to afercia:
I'm sorry maybe I'm missing something, but which steps should be followed to reproduce this and have more than one error displayed at a time?
I could not reproduce it with the login form, but here are the steps for the registration form:
- Enable registration and go to the registration form.
- Submit it with both fields empty.
- You'll get "Please enter a username" and "Please type your e-mail address".
#7
@
8 years ago
Thanks Sergey.
Seems the proposed patch has some issues...
foreach ( $errors ) { ... foreach( $messages ) { ...
etc.
#8
in reply to:
↑ 5
@
8 years ago
Replying to afercia:
I'm sorry maybe I'm missing something, but which steps should be followed to reproduce this and have more than one error displayed at a time? Whatever I do, I get just on error at a time.
See screenshot, dumping$wp_error->get_error_codes()
"more than one error" is not displayed at the login form by default. But it's possible when we use some plugin.
#9
@
8 years ago
@extendwings thanks very much for the clarification.
With the new proposed patch, error messages will use a list. Together with the proposed use of aria-describedby to make errors be announced automatically by screen readers, this could be a very nice improvement for accessibility.
In the attached patch:
- errors are listed with a
<ul>
, messages will still use a<p>
- both use now the "notice" CSS classes
- messages are all blue, see ticket:27418#comment:9
- added focus style for links
- changed link color
#999
in#6e6e6e
in order to have a sufficient color contrast for WCAG level AA - simplified CSS (e.g. removed transitions on links, avoided things like
.login *
) - removed a few title attributes see #24766
- removed two
<br class="clear" />
because screen readers will read out "blank" - removed
<div class="clear"></div>
before</body>
didn't see any reason for that - added CSS class
modal-open
for the body in wp-auth-check.js for consistency with modals
I may have missed something, please let me know. Tried to test all the possible actions, a few example screenshots:
https://cloudup.com/cNxU3BhyLL1
Aside:
many CSS rules here are duplicated, for example the "notice" style are already in common.css
.
Was thinking to include common.css
in the login screen and clean out login.css
but maybe better to wait for the CSS roadmap to sanity
This ticket was mentioned in Slack in #accessibility by afercia. View the logs.
8 years ago
This ticket was mentioned in Slack in #accessibility by rianrietveld. View the logs.
7 years ago
This ticket was mentioned in Slack in #core by chriscct7. View the logs.
7 years ago
This ticket was mentioned in Slack in #core by voldemortensen. View the logs.
7 years ago
#17
@
7 years ago
- Milestone changed from 4.6 to Future Release
Punting from 4.6. Please move back if an updated patch is produced soon.
This ticket was mentioned in Slack in #accessibility by rianrietveld. View the logs.
7 years ago
#20
follow-ups:
↓ 22
↓ 23
@
7 years ago
- Keywords needs-refresh removed
Simplified the changes and limited them to the core issue only: put the error messages in a list and adjusted the CSS for that issue only.
I could not understand why
$( 'body' ).addClass( 'modal-open' );
needed to be added to wp-includes/js/wp-auth-check.js because it's already there in the code and the class is added to the body when the model is open. So I left the changes to wp-auth-check.js out.
#22
in reply to:
↑ 20
@
7 years ago
Replying to rianrietveld:
I could not understand why
$( 'body' ).addClass( 'modal-open' );needed to be added to wp-includes/js/wp-auth-check.js because it's already there
Good catch. It was added in a separate commit, see [31945].
#23
in reply to:
↑ 20
;
follow-up:
↓ 24
@
7 years ago
Replying to rianrietveld:
adjusted the CSS for that issue only.
The new CSS is necessary to properly display the error list, without that it will look like this:
#24
in reply to:
↑ 23
@
7 years ago
Replying to afercia:
Replying to rianrietveld:
adjusted the CSS for that issue only.
The new CSS is necessary to properly display the error list, without that it will look like this:
Yes, and I thought that looked rather pretty ;)
#25
@
7 years ago
- Milestone changed from Future Release to 4.6
Discussed this ticket on the contributor day in WordCamp Europe last week with @afercia @jipmoors and @webmandesign. And we came up with the following (patch 30685.5)
CSS class notice:
The designers in the discussion preferred the login_error block separated from the notice-info.
Andrea prefers to use the "notice" CSS classes they should be used this way: "notice notice-type".
aria-describedby:
In the login screen:aria-describedby="login_error
will only be added to the input fields when there are actually error messages.
To ensure the aria-describedby attribute is used only when there are errors, the issue is everything here is technically a WP_Error but some of them are just informative messages,
for example when:
?loggedout=true
?action=register
The only way we found is to check for the error_data
associated to the errors.
The aria-describedby should also be added for the registration screen, but we will open a separate ticket for this.
#26
@
7 years ago
- Milestone changed from 4.6 to Future Release
Trunk is closed to all new features and enhancements.
This ticket was mentioned in Slack in #accessibility by afercia. View the logs.
7 years ago
#30
@
4 years ago
- Keywords needs-refresh added
- Milestone changed from 5.1 to Future Release
Patch needs refreshing.
This ticket was mentioned in Slack in #accessibility by afercia. View the logs.
4 years ago
#32
@
4 years ago
Discussed during today's accessibility bug-scrub. This issue would probably need some research on modern best practices. Reference:
https://www.w3.org/TR/WCAG21/#error-identification
Rationale and suggested techniques:
Understanding Success Criterion 3.3.1: Error Identification
https://www.w3.org/WAI/WCAG21/Understanding/error-identification.html
Before: Two messages are displayed as if they are one message!