Fix : Snackbar Notice Inconsistency

[Vrishabhsk]

What?

Fixes: #66364

• Enable View Options other than Distraction Fee to show a snackbar notice when they are toggled
• View Options affected : Top toolbar Spotlight mode Fullscreen mode

Why?

• View options: Distraction free is the only option to trigger a snackbar notice #66364

Either all these options should trigger a snackbar notice or none.
I'd like to propose to remove the snackbar notice of the Distraction free mode for consistency and simplicity. I'm not sure this snackbar notice adds much value.

Note: recommended to check what happens when toggling these options via the Command palette.

How?

• Create toggle actions similar to toggleDistractionFree for other View Options
• Replace hardcoded functionality with these toggles which allows Snackbar Notice to be visible when these options are toggled via the More Menu

Testing Instructions

1. Edit a post.
2. Go to the top bar > Options.
3. Toggle on and off each view option.
4. Observe all modes triggers a snackbar notice.

[github-actions]

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: Vrishabhsk <vrishabhsk@git.wordpress.org>
Co-authored-by: ntsekouras <ntsekouras@git.wordpress.org>
Co-authored-by: mtias <matveb@git.wordpress.org>
Co-authored-by: afercia <afercia@git.wordpress.org>

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

[ntsekouras]

Thanks for the PR, it looks good! My comments are mostly nits about inline comments :).

The thing that is not handled here though is the duplicate speak message. I think for these use cases it makes most sense to just use MenuItem component directly and avoid the weird handleToggling and messageActivated/messageDeactivated messages.

[Vrishabhsk]

Hi @ntsekouras 👋. Thanks for the detailed review

• I have added a couple of changes to fix the duplicate speak message issue
• Added disableSpeech prop to PreferenceToggleMenuItem component to prevent speakMessage() from executing
• Added speakMessage option to the createNotice function called by createInfoNotice for custom message capability

Let me know your thoughts on the changes. Thanks

[Vrishabhsk]

To confirm and validate the A11y Speak message, you can use the following script : a11y-speak-monitor.js

[ntsekouras]

Thank you for iterating!

Related though to the latest feedback on the issue, maybe it's better not to show a notice on the more menu clicks, but also preserve the spoken messages.

So some refactoring will be needed. I think by adding a createNotice param (with default true) in toggleDistractionFreeand revert to usePreferenceToggleMenuItem` in the more menu items would be enough.

[Vrishabhsk]

So just that we are on the same page :

• Add createNotice param in toggleDistractionFree function
• Revert MenuItem back to PreferenceToggleMenuItem and remove the need to use onToggle as it was setup initially
• Leave the newly created actions and notice related changes as they are for now

[Vrishabhsk]

I also noticed quite a few changes made to the labels here : #66371

[ntsekouras]

So just that we are on the same page :

Add createNotice param in toggleDistractionFree function
Revert MenuItem back to PreferenceToggleMenuItem and remove the need to use onToggle as it was setup initially
Leave the newly created actions and notice related changes as they are for now

That were my first thoughts but didn't spent time in code. Probably it will be enough.

To elaborate a bit..

1. The new actions are needed to use in context of commands etc.. They just toggle a single preference and speak the message.
2. PreferenceToggleMenuItem handles a preference toggling as is and handles speaking a message. So in the more menu items we don't need to dispatch the action. We need though the messages to match the ones in the actions.
3. toggleDistractionFree is already an action and is also triggered through other user actions. Since we have extra logic in that one, we should probably have a param(createNotice) in the action not to create a notice in the more menu item.

Makes sense?

[ntsekouras]

This is really close, thank you!

Besides the comments I left, we need to do:

1. Remove scope and name here as we don't use them. Also pass the new param at onToggle like this: toggleDistractionFree( { createNotice: false } ).
2. We should have consistent messages in both more menus and actions. That means either have ${option} off/on or ${option activated/deactivated. I'd go for on/off personally.
3. Commented about it, but very important to remove the speakMessage prop you added here.

With these addressed, I think we can land.

[Vrishabhsk]

Thanks @ntsekouras for the review 👍

• I went ahead with ${option} activated/deactivated. to keep the messages consistent.
• Disabled the notice for Distraction Free Menu Item
• We cannot remove scope and name since it determines its isActive attribute which renders the check icon
• I have removed the speakMessage prop

[ntsekouras]

Great work here, thanks!