fix issue 24539, show external icon on help button

[leutrimhusaj]

Description

I have added the external icon (icon={ external }) to the Help MenuItem for edit-post and edit-site plugin. Adding as an svg was important because when i referenced it only as a string (icon="external") it creates a Dashicon, which is 20x20 pixel big, it would break the padding of the text. The default icon size is 24x24 pixel and fits perfect into the more option menu.

How has this been tested?

Open gutenberg editor for a page.
Open the "more option" menu and check/click the help button.

Screenshots

Types of changes

Bug fix for #24539 with non-breaking change

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.

[jasmussen]

Thank you for the PR! This tests nicely, the code is clean and simple, and it solves the issue. The placement on the left feels a little off to me, though, and it might feel better if the icon immediately followed the link, inline in the text. We might even discuss making external a prop on menu items, so this gets simplified.

However because of the simplicity of this PR, and the issue it solves, I'm not opposed to approving this as an interim solution. I also still have a PR in progress (#23552#issuecomment-656049689) which would refactor some of this stuff, so it will be revisited.

I'd love a quick sanity check, @shaunandrews or @MichaelArestad in case you have them?

Thanks again!

[MichaelArestad]

@jasmussen It works well enough until it gets revisited. I believe I share your concern over the location of the icon. Right now it draws extra intention due to the way it aligns with other items on the list. If there's anything I can do to help your PR along, let me know.

[talldan]

Thanks so much for picking this up 👍

In addition to the icon, the other thing that's usually present on these external links is some visually hidden text for screenreader users.

I think you should be able to follow how the External Link component does that for implementing it on these two MenuItems:
https://github.com/WordPress/gutenberg/blob/master/packages/components/src/external-link/index.js#L38-L43

Alternatively, as @jasmussen proposes a prop for the MenuItem component could be an option so that it adds the icon and the visually hidden text implicitly.

[leutrimhusaj]

Thanks for the hint, i didn't knew about that. I have added the visually hidden text like in the example, works well.

[leutrimhusaj]

@talldan I thought about the alternative idea from @jasmussen and prepared another PR #24713 with the enhancement of the MenuItem. Maybe you could check if this is a better solution?

[jasmussen]

I'll refresh my PRs and rebase them off whatever lands here. In the mean time, this PR should not be held up.

This is what the preview dropdown looks like:

The same feedback applies to that, it looks a little weird on the left. But because this PR looks the same, I say let's move this forward. 👍 👍 from a design POV and thank you for the PR! If this gets a quick sanity check, we should land it.

[shaunandrews]

I find it a little weird, both visually (it feels unbalanced) and conceptually (the left-side space is usually reserved for the checkmark). I think it'd be better placed inline, to the right of the text, but I don't have a strong preference.

I think an ideal solution is to not open an external link directly from the menu. Instead, I'd expect a help modal to appear within the editor — and from that modal, I'd find general guidance along with links to external resources.

[jasmussen]

I have refreshed #23552, which now shows at least the preview icon on the right:

If that PR lands first, rebasing this one should address the most pressing feedback about the icon positioning. That does not preclude a further PR that changes that link to open a modal dialog instead — totally fair feedback — but until that, this one would be a nice little improvement.

[shaunandrews]

👍

[leutrimhusaj]

I have rebased the PR and fixed the conflicts. This PR looks like this now.

@jasmussen @talldan Is there anything else to do for this minimal PR?

The other feature should be done in other PR in my opinion.

[jasmussen]

This seems like a good small win. Thanks for the PR!

Before:

After:

Markup:

[github-actions]

Congratulations on your first merged pull request, @leutrimhusaj! We'd like to credit you for your contribution in the post announcing the next WordPress release, but we can't find a WordPress.org profile associated with your GitHub account. When you have a moment, visit the following URL and click "link your GitHub account" under "GitHub Username" to link your accounts:

https://profiles.wordpress.org/me/profile/edit/

And if you don't have a WordPress.org account, you can create one on this page:

https://login.wordpress.org/register

Kudos!