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

Simplify description and option names in the Lock modal dialog #67437

Merged
merged 3 commits into from
Dec 7, 2024

Conversation

sarthaknagoshe2002
Copy link
Contributor

Fixes: #62843

What?

This PR simplifies the description and renames the options in the Lock modal dialog to make the interface clearer and more intuitive for users.

Why?

The original description was too long and confusing, with mismatched visuals and terminology that added cognitive load. Additionally, the varying verbs for options were inconsistent and unnecessary. Simplifying the language and standardizing the option names enhances user understanding.

How?

  1. The description was updated to focus on user intent: "Choose 'Lock all' to disable all actions, or select specific features to lock."
  2. Option names were standardized to:
    • Lock all
    • Lock editing
    • Lock movement
    • Lock removal

Testing Instructions

  1. Add a Navigation block to a post.
  2. Select the Navigation block and click Options > Lock.
  3. Observe the updated description and option names. Ensure the visual order aligns with the description.
  4. Verify the interface is now clearer and easier to understand.

Screenshots or screencast

Screenshot 2024-11-30 at 12 37 54 AM

Copy link

github-actions bot commented Nov 29, 2024

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 <[email protected]>
Co-authored-by: t-hamano <[email protected]>
Co-authored-by: afercia <[email protected]>
Co-authored-by: mtias <[email protected]>

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

@t-hamano t-hamano added [Type] Copy Issues or PRs that need copy editing assistance [Feature] Block Locking The API allowing for the ability to lock/unlock blocks labels Nov 30, 2024
Copy link
Contributor

@t-hamano t-hamano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR!

In order to resolve the E2E test failure, could you also update the strings Disable movement and Prevent removal contained in the following files?

  • test/e2e/specs/editor/blocks/columns.spec.js
  • test/e2e/specs/editor/various/block-locking.spec.js
  • test/e2e/specs/editor/various/block-switcher.spec.js

@t-hamano t-hamano requested review from Mamaduka and afercia November 30, 2024 11:01
@afercia
Copy link
Contributor

afercia commented Dec 2, 2024

Thanks for the PR! One of the points from the issue #62843 is that this text is now the legend of a fieldset element. It's too long to be a legend, it would be nice to make it way shorter.

What about something along the lines of Choose the features to lock?

Screenshot 2024-12-02 at 12 08 44

@sarthaknagoshe2002
Copy link
Contributor Author

sarthaknagoshe2002 commented Dec 2, 2024

Choose the features to lock. sounds good too.
Alternatively it can also be Select the features you want to lock.

@sarthaknagoshe2002
Copy link
Contributor Author

@t-hamano I wanted to know whether I should also change the test title string?

@t-hamano
Copy link
Contributor

t-hamano commented Dec 3, 2024

I wanted to know whether I should also change the test title string?

Thanks for the PR! I think the title of the test is fine as is.

@sarthaknagoshe2002
Copy link
Contributor Author

sarthaknagoshe2002 commented Dec 3, 2024

Okay the I guess we are good to merge.
Just need to re-run the playwright test

@t-hamano t-hamano self-requested a review December 4, 2024 02:12
Copy link
Contributor

@t-hamano t-hamano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally, I think all the text is correct and we can merge this PR.

image

@afercia Any additional feedback would be appreciated.

afercia
afercia previously requested changes Dec 4, 2024
Copy link
Contributor

@afercia afercia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only point left is adjusting the legend text

@afercia
Copy link
Contributor

afercia commented Dec 4, 2024

To me, the only thing left is to improve the legend text. The current one:

Choose "Lock all" to disable all actions, or select specific features to lock

  • is a too long: please consider that some screen readers may repeat a fieldset legend when focusing each form control inside a fieldset.
  • The UI is already self-explanatory: there is already a 'Lock all' checkbox with sub-options. As such, the current text of the legend seems unnecessary.
  • More importantly, a fieldset legend is meant to give a name to the grouping of the fieldset. It's not a place for instructions. It should clarify what the form controls within the fieldset are about.

'Choose the features to lock' or 'Choose the features to lock' would be more appropriate.

@sarthaknagoshe2002
Copy link
Contributor Author

@afercia & @t-hamano referring to this comment

Between the two, "Select the features you want to lock" sounds better.

"Select the features you want to lock" sounds better because it’s clearer and more user-friendly.

It feels more personal by saying "you want to lock," which puts the focus on the user’s choice. It also uses the word "select," which is a common term in apps or websites, so it feels natural if someone is clicking or choosing something on a screen.

On the other hand, "Choose the features to lock" is shorter but a bit vague. It doesn’t feel as specific or connected to the user.

@afercia
Copy link
Contributor

afercia commented Dec 6, 2024

"Select the features you want to lock" sounds good to me.

@t-hamano
Copy link
Contributor

t-hamano commented Dec 7, 2024

@sarthaknagoshe2002 Thanks for the update! Based on the discussion so far, this PR looks ready to ship 👍

@t-hamano t-hamano dismissed afercia’s stale review December 7, 2024 03:52

This PR has been updated based on the feedback

@t-hamano t-hamano merged commit 959bb6b into WordPress:trunk Dec 7, 2024
61 of 63 checks passed
@github-actions github-actions bot added this to the Gutenberg 19.9 milestone Dec 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Block Locking The API allowing for the ability to lock/unlock blocks [Type] Copy Issues or PRs that need copy editing assistance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve Lock modal description and options naming
3 participants