Add Inline Error for Protected Post Password Field and Uncheck "Password Protected" for Empty Field #67034

sarthaknagoshe2002 · 2024-11-15T13:42:15Z

Fixes: #64157

What?

This PR addresses two issues related to the post password field:

Adds an inline error to warn users when a password exceeds the 255-character limit, preventing server errors.
Automatically unchecks the "Password Protected" checkbox when the password field is cleared, ensuring the UI accurately reflects the state of password protection.

Why?

The inline error prevents users from submitting passwords that are too long and helps avoid server errors. It also improves the user experience by providing clear feedback when the password is truncated.
Unchecking the Password Protected checkbox when the password field is cleared ensures that the UI is in sync with the actual post protection state, reducing confusion for users.

How?

For the inline error, the code checks the length of the entered password. If the length exceeds 255 characters, an inline error message is displayed.
To fix the Password Protected checkbox, the state is updated to automatically uncheck the box when the password field is cleared. This is done by updating the showPassword state when the password field is empty & the Dropdown is out of Focus, ensuring the checkbox reflects the correct state.

Testing Instructions

Test Inline Error for Password Length:
- Go to the post editor and enter a password longer than 255 characters in the Password field.
- An inline error should appear, notifying the user that the password exceeds the character limit and will be truncated.
Test Password Protected Checkbox Behavior:
- Create or edit a password-protected post.
- Check the Password Protected checkbox and enter a password.
- Save the post, then clear the password field.
- Ensure that the "Password Protected" checkbox is automatically unchecked when the password field is cleared.
- Save the post again and confirm that the password protection is disabled.

Screenshots or screencast

Password.protection.bug.mov

… option when password is empty

github-actions · 2024-11-15T13:42:30Z

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: sarthaknagoshe2002 <sarthaknagoshe2002@git.wordpress.org>
Co-authored-by: ramonjd <ramonopoly@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

github-actions · 2024-11-15T13:42:57Z

👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @sarthaknagoshe2002! In case you missed it, we'd love to have you join us in our Slack community.

If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information.

ramonjd · 2024-12-17T00:45:10Z

Thanks a lot @sarthaknagoshe2002! I tested:

setting and remove a password still works on the frontend.
the password control is toggled off if the field is empty and the panel closes
strings > 255 trigger the error

Here's a string of 256 chars to test:

abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789!@#$%^&()_+-=[]{}|;:,.<>?~abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789!@#$%^&*()_+-=[]{}|;:,.<>?~abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789!@#$%^&()12345678

I think it handles the edge cases well too, e.g.,

if you try paste the 256 char string in, it won't allow it in the field
if you paste a 255 char string and then try to add a few chars, it will preserve the first 256

I'll just add a couple of reviewers for sanity checks.

ntsekouras · 2024-12-17T08:13:02Z

packages/editor/src/components/post-status/index.js

 														id={ passwordInputId }
 														__next40pxDefaultSize
 														__nextHasNoMarginBottom
-														maxLength={ 255 }


Should we keep this check and instead of an error show a warning that the pasted code has been trimmed?

Additionally we could also add this limitation to a help text.. 🤔

If we keep that line, this condition won’t be triggered:

if ( newPassword.length > 255 ) { setError( __( 'Password cannot exceed 255 characters.' ) );

This means we won’t be able to check if the character limit is exceeded. To handle that, we would need to listen for keydown and paste events in order to show an error or warning when the limit is breached.

I mean listening events won't be that straight-forward as this approach.
If your experience says that we should change the approach then lets do it 🚀.
Let me know accordingly 😇

show a warning that the pasted code has been trimmed?

Just to make things clear, the error should be converted to warning & the message should say the following right?
Password cannot exceed 255 characters \n The pasted password may have been trimmed

Yeah, I see the dilemma. It would be nice to keep the HTML attribute for compatibility. HTML validation attributes also work well with assistive technologies, improving accessibility.

There's another, low-tech option that might be better UX: just tell folks that the maximum is 255 before they enter, and reinstate the maxLenth attr.

Given that it's an edge case anyway, I don't think we need to overthink it.

Then we could just remove the error message and associated handlers.

What do folks think?

I'm likely missing a nuance (apologies) but could it make sense to use browser native UI for this? Here's an example.

This seems like a good option, provided it doesn’t impact the validation process.

Another workaround could be displaying a warning at 255 characters, indicating that any additional characters will be trimmed off. This way we can keep the warning & the maxLenth attr too.

This seems like a good option, provided it doesn’t impact the validation process.

Another workaround could be displaying a warning at 255 characters, indicating that any additional characters will be trimmed off. This way we can keep the warning & the maxLenth attr too.

@ramonjd @ntsekouras @jameskoster awaiting your feedbacks on this suggestion.

Worth a try, I think.

Keeping the max length attribute may lead to the existing problem, copying in a password 300 characters long would be trimmed to 255 but the error message would inform the user "any furtther characters will be trimmed" or whatever the string turns out to be.

The MDN recommendation is to code up validation in an accessible manner:

Browsers often don't let the user type a longer value than expected into text fields. A better user experience than just using maxlength is to also provide character count feedback in an accessible manner and let the user edit their content down to size. An example of this is the character limit when posting on social media. JavaScript, including solutions using maxlength, can be used to provide this.
-- MDN: client side form validation

Then lets skip the max length attribute & just display an error message that the password can't exceed 255 chars

feat: show inline error for password limit & uncheck password protect…

fe82fdb

… option when password is empty

github-actions bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Nov 15, 2024

Merge branch 'WordPress:trunk' into fix/issue-64157

5217c6f

adamsilverstein added the [Type] Enhancement A suggestion for improvement. label Dec 16, 2024

ramonjd added [Package] Editor /packages/editor Needs Design Feedback Needs general design feedback. labels Dec 17, 2024

ramonjd requested review from ntsekouras, peterwilsoncc and ramonjd December 17, 2024 00:45

ntsekouras reviewed Dec 17, 2024

View reviewed changes

Add Inline Error for Protected Post Password Field and Uncheck "Password Protected" for Empty Field #67034

Are you sure you want to change the base?

Add Inline Error for Protected Post Password Field and Uncheck "Password Protected" for Empty Field #67034

Conversation

sarthaknagoshe2002 commented Nov 15, 2024

What?

Why?

How?

Testing Instructions

Screenshots or screencast

Uh oh!

github-actions bot commented Nov 15, 2024

Uh oh!

github-actions bot commented Nov 15, 2024

Uh oh!

ramonjd commented Dec 17, 2024

Uh oh!

ntsekouras Dec 17, 2024

Choose a reason for hiding this comment

Uh oh!

sarthaknagoshe2002 Dec 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sarthaknagoshe2002 Dec 17, 2024

Choose a reason for hiding this comment

Uh oh!

sarthaknagoshe2002 Dec 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ramonjd Dec 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sarthaknagoshe2002 Dec 18, 2024

Choose a reason for hiding this comment

Uh oh!

sarthaknagoshe2002 Dec 20, 2024

Choose a reason for hiding this comment

Uh oh!

jameskoster Dec 20, 2024

Choose a reason for hiding this comment

Uh oh!

peterwilsoncc Jan 7, 2025

Choose a reason for hiding this comment

Uh oh!

sarthaknagoshe2002 Jan 17, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

sarthaknagoshe2002 Dec 17, 2024 •

edited

Loading

sarthaknagoshe2002 Dec 17, 2024 •

edited

Loading

ramonjd Dec 18, 2024 •

edited

Loading