Site: editor: Add view site link to site editor nav #50420
Conversation
glendaviesnz
added
[Type] Enhancement
|
Size Change: +4.86 kB (0%) Total Size: 1.41 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 82a7024. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5074114556
|
|
Hey @glendaviesnz I've just found this PR and it seems there's some disagreements/confusion about what needs to be done, that we need to clear out. Separately I've started this PR #50369 as a starting point to address #50378 which also includes a "view post/view page" links. I don't know if we're supposed to have both of these? Anyway, just wanted to link to these and loop in designers to get more clarity @jameskoster @jasmussen @SaxonF |
|
Riad there's a bit more context in #50405 (comment) for this, cc: @richtabor |
The issue is to add a view site link outside of the Edit View, so you don't have to navigate first into a template/page to then view the site. |
|
The main question for me is whether the link should always open the homepage, or be contextual and open whatever is visible in the frame (assuming it's compatible). If it's a generic "open the homepage" link, then the placement seems good. Though I'd echo Joen's comment about making use of the site title rather than adding another icon button. If the link is to be contextually related to the frame then I would question the placement a bit, and suggest exploring something more connected to the frame. |
I'm thinking homepage. |
|
Okay. If we made use of the site title, we could still use the icon attached in a smaller variant: 
We use a similar pattern elsewhere: 
Or maybe a simple |
|
I would think the underline is sufficient, but either way can we surface the icon on hover only? |
|
Underline works for me, and avoids the inconsistent icon sizing which looks a bit awkward. |
|
Can even combine with tooltip if that's not enough. Lots of options! |
|
I have updated it to an underlined link with tooltip: site-view2.mp4 |
glendaviesnz
force-pushed
the
add/view-site-to-site-edit-nav
branch
from
1242563
to
ec88366
Compare
|
Tooltip, no underline, is my vote 😅 |
|
I wouldn't mind the underline on hover, but agree the resting state should not have an underline. Thanks for keeping at this! |
Done 😃 |
|
It was mentioned yesterday that using the site title can create confusion whereby you mistakenly interpret the icon + title to be a single element, which can result in mis-clicks. An alternative option which avoids adding noise would be to only expose the externalink when you hover the site hub:
I suppose it would need to be permanently visible on mobile, but that seems to be a reasonable trade-off. What do y'all think? |
I don't mind this. |
@jasmussen are you also happy with this approach? |
|
Sure let's go with that. I still feel like we kind of urgently need a better external link icon, it's huge. Here's a sketch that updates the View menu as well: 
That's not blocking for now, just a design task. |
|
I have switched it to the hover only icon: hover-only.mp4@jasmussen 18px x 18px seems to balance reasonably well against the command center search icon, but let me know what sort of sizing you think is best. |
|
I have switched to button and also set the icon to display if the hub content area is focused, but I had trouble getting that to focus using the keyboard. |
|
A couple of details (apologies, you must be tired of this one):
Aside from that and any outstanding a11y issues we should be good to merge. |
Hey @jasmussen , that sounds good to me. We won't necessarily have the capacity to work on these issues firsthand, but we're always happy to help with reviews and advice (and with some authoring when necessary like we did on the editor frame resizing PR) |
|
@jameskoster I have fixed the hover issue and reduced the gap. |
|
Thanks Glen! Pending a11y sign-off this is looking good to go. |
|
Accessibility-wise:
|
Yes, that seems to work, have added that. |
|
I am going to go ahead and merge as it should be easy to follow up on if any other accessibility issues are raised. |
|
Just noticed that this PR broke the layout of the Site hub when tabbing with the keyboard. Created #51257 and suggested a possible fix there. |
|
A couple additional accessibility follow-ups:
|
|
@joedolson I have added a PR to fix the focus and label issues. @jameskoster would be the person to answer the question about why it is only visible on mouseover/focus. |
|
Iirc, as it is not 100% clear how the 'site hub' will evolve as we explore the broader admin design there was motivation to keep it as lightweight as possible for now. Nothing is set in stone, and it may be something we revisit later. |
|
We may have different definitions of lightweight; I see the hidden link as a more complex design with a higher cognitive and discovery load. However, I do appreciate that until there's some kind of admin and management component in place, there isn't really a well-defined context for this. But not a fan of hidden controls. |
I'd totally second this, and I'd like to add that in the last 6 years one of the most common accessibility concerns that was very often reported is specifically about UI controls that continuously appear and disappear. That's an accessibility anti-pattern and I'd greatly appreciate all designers to be more aware of that. Thank you. |
What?
Adds a
View sitelink to the site title at top of site editor left navWhy?
Fixes: #50405
How?
Adds an
ExternalLinkcomponent withgetUnstableBase().homeas the url.Testing Instructions
View sitetooltip appears if icon is moused overScreenshots or screencast
hover-only.mp4