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

Moved "Welcome guide" link to options modal dialog #19499

Closed
wants to merge 1 commit into from

Conversation

mapk
Copy link
Contributor

@mapk mapk commented Jan 7, 2020

Description

Fixes #19443.

Move the Welcome Guide link to the Option modal dialog. It is now under the "General" category next to the "Inserter Help Panel" which seems fitting. This is in an effort to help alleviate the redundancy of two "Tools" sections in the interface as recorded in #19409.

How has this been tested?

Tested locally.

Screenshots

Option modal dialog

Screen Shot 2020-01-07 at 2 25 37 PM

Gif of interaction

welcome-guide 2020-01-07 14_21_28

Types of changes

Moving a link from one location to another. Non-breaking, maybe?

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR. .

@mapk mapk requested a review from talldan as a code owner January 7, 2020 22:27
@mapk mapk added Needs Design Feedback Needs general design feedback. Needs Technical Feedback Needs testing from a developer perspective. Needs Accessibility Feedback Need input from accessibility labels Jan 7, 2020
@mapk
Copy link
Contributor Author

mapk commented Jan 7, 2020

One concern I have is that by adding this to the Options modal dialog, when I click to check this feature "on", the dialog disappears immediately and the Welcome Guide appears.

Is this okay a11y-wise? Design-wise?

@mapk mapk requested a review from noisysocks January 7, 2020 22:31
@mapk mapk self-assigned this Jan 7, 2020
@@ -13,7 +13,6 @@ import CopyContentMenuItem from './copy-content-menu-item';
import ManageBlocksMenuItem from './manage-blocks-menu-item';
import KeyboardShortcutsHelpMenuItem from './keyboard-shortcuts-help-menu-item';
import ToolsMoreMenuGroup from '../components/header/tools-more-menu-group';
import WelcomeGuideMenuItem from './welcome-guide-menu-item';
Copy link
Member

@noisysocks noisysocks Jan 16, 2020

Choose a reason for hiding this comment

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

Since this change means that it is no longer used anywhere, let's delete the packages/edit-post/src/plugins/welcome-guide-menu-item directory as part of this PR.

@enriquesanchez
Copy link
Contributor

One concern I have is that by adding this to the Options modal dialog, when I click to check this feature "on", the dialog disappears immediately and the Welcome Guide appears.

I think this could cause issues due to the unexpected focus loss. Because the checkmark is not really a button or link, users wouldn't be expecting for it to move them elsewhere in the UI.

I think we could work around that by using a link instead.

@mapk
Copy link
Contributor Author

mapk commented May 5, 2020

@enriquesanchez I'm willing to try that route if it helps minimize the links under the Tools section in the Options popover. Let's make it a link.

@noisysocks
Copy link
Member

noisysocks commented May 6, 2020

Before #18041 we had it so that persisting the option wasn't done until the modal was closed. In other words, the user would toggle the checkbox, press X, and then see the welcome guide appear.

Of course, a link would also work! 🙂

@enriquesanchez
Copy link
Contributor

@noisysocks That is a good point.

If I check the option and see the Welcome Guide modal, it means that once I'm finished going through the steps this option will be again turned off, correct?

I ask because I'm trying to figure out if a checkbox is the right control for this. If this is not a "persistent" setting, should it be a setting at all?

To me, the current behavior feels more appropriate.

If we wanted to take it a step further, we could even change the link to "See the Welcome Guide".

@mapk
Copy link
Contributor Author

mapk commented May 6, 2020

If we can keep it a checkbox, I prefer that option. It will align with the other settings better.

@noisysocks
Copy link
Member

If I check the option and see the Welcome Guide modal, it means that once I'm finished going through the steps this option will be again turned off, correct?

Correct, if we implement something similar to what was done before #18041.

@mapk
Copy link
Contributor Author

mapk commented Sep 2, 2020

I, too, believe it would be appropriate to automatically check "off" the Welcome Guide after someone has gone through it or dismissed it. Yes, this makes the checkbox interaction a little awkward, so I'm open for alternatives like a link. But my big desire here is to move it from the first "Options" popover into the second "Options" modal.

@mapk
Copy link
Contributor Author

mapk commented Nov 17, 2020

There's been some recent changes to the Options modal, so we should evaluate if this is still a good solution or not. I'd like to continue to make sure we're grouping settings effectively and logically. I'll have another look at this soon.

Base automatically changed from master to trunk March 1, 2021 15:43
@annezazu
Copy link
Contributor

A ton has changed in this general experience since this PR was opened. Closing this out as a result due to inactivity.

@annezazu annezazu closed this Jul 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Accessibility Feedback Need input from accessibility Needs Design Feedback Needs general design feedback. Needs Technical Feedback Needs testing from a developer perspective.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move "Welcome Guide" link to Options modal dialog
4 participants