Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better Login Error&Message Displaying #1155

Closed
abhansnuk opened this issue Oct 12, 2023 · 25 comments
Closed

Better Login Error&Message Displaying #1155

abhansnuk opened this issue Oct 12, 2023 · 25 comments
Assignees
Labels
6.4 Changes in 6.4 release [Status] Done Issue is completed
Milestone

Comments

@abhansnuk
Copy link
Collaborator

Trac ticket: https://core.trac.wordpress.org/ticket/30685

Owner: sabernhardt

Labeled as 'needs dev note'
Note Joe Dolson's comment on https://core.trac.wordpress.org/ticket/30685#comment:57

@abhansnuk abhansnuk converted this from a draft issue Oct 12, 2023
@github-actions
Copy link

Heads up @WordPress/docs-issues-coordinators, we have a new issue open. Time to use 'em labels.

@abhansnuk abhansnuk moved this from Todo to In Progress in WP 6.4 Documentation Oct 14, 2023
@abhansnuk
Copy link
Collaborator Author

Response from maintainer Rajin Sharwar that this will be ready on October 15, 2023.
Slack message.

@shabnam611 shabnam611 added the 6.4 Changes in 6.4 release label Oct 14, 2023
@shabnam611 shabnam611 added this to the 6.4 milestone Oct 14, 2023
@Rajinsharwar
Copy link

Rajinsharwar commented Oct 14, 2023

Hi @abhansnuk, here is a draft for the dev note. Requesting peer review and proofreading.
https://make.wordpress.org/core/?p=108803&preview=1&_ppp=6edd12b9d8

@shabnam611 shabnam611 moved this from In Progress to Needs 1st Review in WP 6.4 Documentation Oct 15, 2023
@nalininsbs
Copy link

nalininsbs commented Oct 16, 2023

@codente and @abhansnuk we will put this under 'Users' in the Field Guide as it is a sub component of this. It can then list Login and Registration.

@codente
Copy link

codente commented Oct 16, 2023

Hi @Rajinsharwar thank you and nice work on this dev note:

I do have a couple suggestions

  1. I think a word is missing here

WordPress 6.4 brings a number of key improvements to the HTML markup of the wp-login.php page to its structure

at the end there, I think we need the word make or a similar word in front of its

to make its structure

  1. In the second paragraph, I think you could remove the period before And here

And <div> tag was used for enclosing the error messages.

and just join it together without a separate sentence.

  1. In the 3rd paragraph, I think you could remove this And and just start it with "In the case of..."

"And, in the case of multiple error messages"

  1. Same paragraph, last sentence I'd rewrite it a little differently.

In the case of multiple errors, bullets have been removed in favor of a small margin between error messages.

  1. In the Some other changes section, do we need to do inline code when you mention the br tags and include the opening and closing tag so it looks like this <br>?

  2. I think you could remove the So from the last line.

@Rajinsharwar
Copy link

Rajinsharwar commented Oct 16, 2023

Hi @codente, just edited the doc with your suggestions. Let me know it it looks good now.

@codente
Copy link

codente commented Oct 16, 2023

@Rajinsharwar I think the word make got added in the wrong place. It should be before the word its and not after.

"WordPress 6.4 brings a number of key improvements to the HTML markup of the wp-login.php page to make its structure"

Otherwise, the changes look good from here :)

@abhansnuk
Copy link
Collaborator Author

@Rajinsharwar can you please add an excerpt to the dev note. I will add tags once the other the tech edit and changes are done.

Did any of the other maintainers do a tech peer review please? If so, do you have their WP.org IDs please? Thanks for working on this dev note.

@Rajinsharwar
Copy link

Ah lol, done @codente. :)

@Rajinsharwar
Copy link

Hi @abhansnuk, I added an excerpt. For now, none other than @codente did the review. I am not really sure of your WP.org ID @codente 😅

@abhansnuk
Copy link
Collaborator Author

Hello @Rajinsharwar Thanks for the update.

Could you reduce the size of the excerpt please as it is too long to be effective in search and embeds.

Could you make the images of before and after 6.4 the same height please assuming this is possible with the content wanting to be visible? If not, can they both start at the same top level?

I will share it for further feedback in core once you have finished the current editing you are doing, and do the second docs review too.

Thanks.

@Rajinsharwar
Copy link

Hi @abhansnuk, just re-updated the doc. Can you take a look now?

@abhansnuk
Copy link
Collaborator Author

abhansnuk commented Oct 17, 2023

Shared in core for another core review in the next few hours.

Now with the link

@zzap zzap added the [Status] To do Issue marked as Todo label Oct 17, 2023
@sabernhardt
Copy link

All three images need alternative text. These examples probably can be improved:

  • Login form comparison: multiple errors were separated by a line break, and now the errors are in a list with a small margin between them
  • Registration form comparison: the Register For This Site message paragraph is now inside a div element; multiple errors now have list markup and a margin between them
  • Reset password form comparison: both the informational message and the single error are paragraphs inside a div element instead of only one of those elements

@abhansnuk
Copy link
Collaborator Author

@Rajinsharwar are you able to deal with these please?

Thanks @sabernhardt for the review.

@abhansnuk abhansnuk moved this from Needs 1st Review to Edits after 1st Review in WP 6.4 Documentation Oct 17, 2023
@sabernhardt
Copy link

The sentence about multiple error messages mentions the styling, but it says nothing about the new list markup. Maybe just add that each message is a list item:

In the case of multiple errors, each message is a list item, and bullets have been removed in favor of a small margin between error messages.

Minor formatting recommendations:

  • The wp-login.php filename would be wrapped in inline code instead of italicizing it.
  • The quoted text for each link (“Log in“, “Register” and “Lost your password?”) should not need italicizing either because the quotation marks already set the text apart.

@Rajinsharwar
Copy link

Hi @sabernhardt @abhansnuk, just updated the doc. Can you take a look now?

@sabernhardt
Copy link

  1. The heading levels within the post content need to be raised by one. Right now, it skips from level one (the title) to level three. The two section headings should be H2.
  2. The second image caption text does not have a period.
  3. We probably should add another sentence to say plainly: "CSS selectors such as p.message will not apply styles to the informational messages." It could fit after mentioning "classes on the div (<div>) instead of the paragraph (<p>).").

@Rajinsharwar
Copy link

Just made the changes again @sabernhardt.

@abhansnuk abhansnuk moved this from Edits after 1st Review to Needs 2nd Review in WP 6.4 Documentation Oct 17, 2023
@codente
Copy link

codente commented Oct 17, 2023

@Rajinsharwar @sabernhardt

With this change:

CSS selectors such as p.message will not apply styles to the informational messages.

Stephen mentioned informational specifically but in the post it says notice messages. Does that distinction matter or is it fine as is?

Stephen's comment makes me think it only applies to certain (informational) notices while the way it is worded now seems like it may apply to all notices?

@sabernhardt
Copy link

I see it's already updated to "notice": "will not apply styles to the notice messages." Consistency should be better than saying "informational" there.

Technically, both errors and informational messages have the notice class. Then the notice-info and notice-error classes differentiate those two types.

@sabernhardt
Copy link

@nalininsbs nalininsbs moved this from Needs 2nd Review to Done in WP 6.4 Documentation Oct 17, 2023
@nalininsbs
Copy link

Is there anything on this for updating the Field Guide?

@nalininsbs
Copy link

Noting discussion in release docs area - add to other developer updates in the Field Guide.

@nalininsbs
Copy link

@zzap can you change the label to complete please. We are unable to change this. Thank you.

@zzap zzap closed this as completed Oct 25, 2023
@github-actions github-actions bot added [Status] Done Issue is completed and removed [Status] To do Issue marked as Todo labels Oct 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.4 Changes in 6.4 release [Status] Done Issue is completed
Projects
Status: Done
Development

No branches or pull requests

7 participants