-
Notifications
You must be signed in to change notification settings - Fork 286
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
Clicking the Site Kit logo within the header of DashboardEntityApp should redirect user to the main dashboard. #4793
Comments
Thanks @asvinb – I've thought about this before too as that is a very common UX pattern. I'm not quite sure about only doing it in one place though. If we do it I think it should be across the board everywhere the header is used. If there are situations we wouldn't want that then it might be better to leave it as-is, but I'll raise it with the rest of the team to discuss 👍 |
My actual instinct here is to have that logo always link to the main dashboard. That creates the expectation/informs the user that there is a link there, but also simplifies the logic of implementing it. I've seen that done with little confusion across many sites, basically that it's fine to leave that sort of "Home" link there all the time. We don't disable links in the menu: Let's just leave it on all the time—what do you think @jimmymadon? |
@tofumatt I'm happy to keep the logo as a link on the main dashboard as shown in your GIF. However, what should the behaviour of clicking on the logo be on a splash page? Would the link simply reload the page? |
@jimmymadon Yeah, I think in that case it'd be a link to the main dashboard and would reload the page. It's of semi-dubious value, but it lets the user know that's a link and would even allow a user to open it in a new tab with a "command + click". I've seen that behaviour in other sites/apps and it's not confusing, so let's do that 👍🏻 |
Hi @nfmohit, thanks for the IB. I would suggest a slightly different approach here, though. Rather than providing an A tweak to the styling of the anchor tag would also be needed to ensure the link is visually differentiated when focused. It looks like setting its Please take a look and see what you think. |
Thank you for the kind review, @techanvil! Should we pass the dashboard URL for all scenarios? I ask because the AC mentions the page should just be reloaded for splash screens. |
Hey @nfmohit, thanks for pointing that out. To be honest I don't think we need to be too "smart" about this. If we just use the dashboard URL, when the user doesn't have access to the dashboard it will redirect to the appropriate splash page anyway. I think this would probably be better than always reloading the current page. For example, when disconnecting from Site Kit, the following splash page is shown. To me, it would seem a bit strange if clicking on the Site Kit logo reloaded this page: On the other hand, if the dashboard URL is used for the link, clicking on it will result in a redirect to this splash page, which makes more sense to me: What do you think? |
Very valid point. I have updated the IB. Thank you @techanvil! |
Thanks @nfmohit. LGTM! IB ✅ |
QA Update ✅
Recording.58.mp4 |
Feature Description
When the user is on the entity dashboard (with the
unifiedDashboard
feature flag set), another intuitive way of getting back to the main dashboard is by clicking on the Site Kit logo in the header which is a common practice on sites when on sub pages.Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
Implementation Brief
assets/js/components/Header.js
:<Logo />
component. Wrap it using the<Link />
component. In thehref
prop, pass a URL to the Main Dashboard. The Main Dashboard URL can be found by using thegetAdminURL
selector of theCORE_SITE
store, passinggooglesitekit-dashboard
string passed to it.<Link />
component. Add thedisplay: inline-block;
styles to it so that the link can be visually differentiated when focused.Test Coverage
QA Brief
Changelog entry
The text was updated successfully, but these errors were encountered: