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

Implement new Unified Dashboard header design/UI behind feature flag #4048

Closed
tofumatt opened this issue Sep 15, 2021 · 22 comments
Closed

Implement new Unified Dashboard header design/UI behind feature flag #4048

tofumatt opened this issue Sep 15, 2021 · 22 comments
Labels
P0 High priority Rollover Issues which role over to the next sprint Type: Enhancement Improvement of an existing feature
Milestone

Comments

@tofumatt
Copy link
Collaborator

tofumatt commented Sep 15, 2021

Feature Description

The Unified Dashboard has an updated Header design that should be implemented, preferably in the existing Header component, behind a feature flag.

It should start with the header being "sticky", the user avatar being only an icon on all screen sizes, and a "dummy" button that a user can click to open the URL/page title search, eg:

Screenshot 2021-09-15 at 20 27 57


Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

  • The existing Header component (assets/js/components/Header.js) should continue to be used. All modifications in this issue should only be present when the feature flag unifiedDashboard is enabled.
    • When the feature flag is not enabled, no changes to the header should be made.
  • The entire Header should be "sticky", eg. it should remain on-screen/at the top of the page, even as the page is scrolled.
  • The user menu should never show the user's login/email; it should always show only the user's avatar. (This is already the behaviour on mobile viewports—essentially, all viewports should now match the mobile viewport's behaviour.)
  • A new button should be added for the "URL search" feature.

In its default state the URL Search button should look like this:

Screenshot 2021-09-29 at 15 36 17

When clicked it should expand to show an <input> component:

Screenshot 2021-09-29 at 15 36 44

Note the exact design and functionality will be implemented in #4049, so a simple <input> element when the "URL Search"/magnifying glass icon button is clicked for now is fine.

The desktop Figma file is here: https://www.figma.com/file/VILRieNSLg2DOVyEgFBkXe/%5BMVP%5D-Generic-Dashboard?node-id=0%3A1 (under the "Single Search: Full frames" section). The mobile versions are available here: https://www.figma.com/file/VILRieNSLg2DOVyEgFBkXe/%5BMVP%5D-Generic-Dashboard?node-id=382%3A1699

When the input is blurred it should disappear and the "URL Search" button should appear again. No functionality other than the show/hide functionality should be implemented as part of this issue.

Implementation Brief

Create a new component assets/js/components/EntitySearchInput, it will need a single piece of state isActive, when it is inactive (false === isActive) it renders as a <Button component. The icon needs to have a magnifying glass (per the AC), for which we need a new image and the text "URL Search". In the onClick handler for the button, the isActive state is set to true, and the component renders as an <input /> field, which will be further implemented in #4049. In the onBlur handler for the input field, the isActive state is set back to false.

  • We will be updating two components, assets/js/components/DashboardEntityApp.js and assets/js/components/DashboardMainApp.js. Since they are both already behind the feature flag for unified dashboard, we don't need to further check for the feature flag. The changes will need to occur in both files.

Add three child components to the <Header /> component: <EntitySearchInput /> (created above), <DateRangeSelector />, <HelpMenu />. If you're familiar with DashboardDetailsApp, the order of HelpMenu and DateRangeSelector is intentionally switched to align with the figma files.

<Header>
    <EntitySearchInput />
    <DateRangeSelector />
    <HelpMenu />
</Header>
  • In assets/js/components/UserMenu.js add a check for the feature flag const unifiedDashboardEnabled = useFeature( 'unifiedDashboard' );, and when the feature is enabled, ignore the return when ! userEmail, and don't show the text in the button { userEmail }.

QA Brief

  • Enable unifiedDashboard feature flag.
  • Observe the header bar resembling the picture in the AC, make sure
    • New (unstyled) EntitySearchWidget with the behavior described in the AC.
    • User Menu no longer showing email address in the header.
    • Check in both main main and entity dashboard.
  • UPDATE: Also ensure that,
    • Input field is focused when search button is clicked.
    • Input field closes when clicked outside it.
    • search icon is on the right with enough spacing.
    • User Avatar is centered when hovered.
    • User Menu has a tooltip "Account".
    • New design changes made to User Menu does not affect Old Dashboard.

Changelog entry

  • Implement new Unified Dashboard header design/UI.
@johnPhillips
Copy link
Contributor

@tofumatt Just a note that coverage of some of the events mentioned in #4057 might need including in the ACs for this issue so that the old and new headers are aligned.

@ivankruchkoff
Copy link
Contributor

Following from the above comment, same for events in #4058

@tofumatt tofumatt assigned tofumatt and unassigned tofumatt Sep 28, 2021
@tofumatt
Copy link
Collaborator Author

The GA events in #4057 only apply once the URL is actually "activated" eg. navigated to. That won't happen as a part of this issue, but rather in #4049.

Everything else in #4058 will already be present here 👍🏻

@tofumatt tofumatt removed their assignment Sep 28, 2021
@eugene-manuilov eugene-manuilov self-assigned this Oct 1, 2021
@eugene-manuilov
Copy link
Collaborator

eugene-manuilov commented Oct 1, 2021

Check for feature flag being enabled with a new const which will check if the unifiedDashboard feature flag is enabled const unifiedDashboardEnabled = isFeatureEnabled( 'unifiedDashboard' ). We will need to do this in several components

We need to use the useFeature hook instead of isFeatureEnabled.

In assets/js/components/dashboard/DashboardApp.js and assets/js/components/dashboard-details/DashboardDetailsApp.js

We need to make changes in the new DashboardEntityApp and DashboardMainApp components. The current DashboardApp and DashboardDetailsApp should remain the same.

Also, when the feature is enabled, prior to both the '' and add a new <Button component. The icon needs to have a magnifying glass (per the AC), for which we need a new image and the text "URL Search". In the onClick handler it should change to an field.

Let's create a new component for it instead of adding button and input elements directly to the DashboardEntityApp/DashboardMainApp component.

@eugene-manuilov
Copy link
Collaborator

IB ✔️

@eugene-manuilov eugene-manuilov removed their assignment Oct 4, 2021
@kuasha420 kuasha420 self-assigned this Oct 5, 2021
@eugene-manuilov
Copy link
Collaborator

@kuasha420 i see you are already working on this ticket. Please, add the correct event tracking to the new EntitySearchInput component as described for the existing URLSearchWidget component in the #4057.

@kuasha420 kuasha420 removed their assignment Oct 6, 2021
@kuasha420
Copy link
Contributor

@eugene-manuilov Wouldn't it be better to add the tracking event on #4049 as that's where the EntitySearchComponent will be properly implemented? These cross ticket IBs made me a little confused!

@aaemnnosttv
Copy link
Collaborator

@wpdarren the ACs don't require that the search field matches the design, this is more of a scaffolding of the foundation for the feature. The ACs mention that this will be handled in a subsequent issue:

Note the exact design and functionality will be implemented in #4049, so a simple <input> element when the "URL Search"/magnifying glass icon button is clicked for now is fine.

2. This might be being overly picky but the user avatar isn't centred to the background colour when you hover.

I raised this above; I'm not sure it matters too much which issue it's fixed in, so long as we agree on one before this one moves to approval (cc: @kuasha420).

@tofumatt anything else needed here?

@wpdarren
Copy link
Collaborator

@aaemnnosttv it does mention in QAB referencing the AC which says

In its default state the URL Search button should look like this:

image

This is why I have included the observations in my comment.

The reference to the other ticket is related to the input field, I agree can be excluded from my observations.

@tofumatt
Copy link
Collaborator Author

I meant for this issue to cover that URL Search style, but I think it's fine to fix in #4049 instead, where any other issues here can be fixed. Again: this issue is largely about scaffolding, so I think it's fine as-is—even the arrow spacing Evan mentioned here: #4048 (comment) could be fixed there, I think.

@felixarntz felixarntz added Type: Enhancement Improvement of an existing feature and removed Type: Feature New feature labels Oct 12, 2021
@kuasha420
Copy link
Contributor

@tofumatt I think I will create a follow up pr on this issue and fix avatar and the button here so CR/QA doesn't get too much out of control on 4049. Thanks for the discussion and clarification!

cc: @wpdarren

@kuasha420 kuasha420 removed their assignment Oct 14, 2021
@kuasha420
Copy link
Contributor

Update:

Made a follow up PR addressing the above issues. Also updated QAB.

Cheers.

@tofumatt tofumatt assigned tofumatt and kuasha420 and unassigned tofumatt Oct 14, 2021
@kuasha420 kuasha420 assigned tofumatt and unassigned kuasha420 Oct 18, 2021
@tofumatt tofumatt removed their assignment Oct 19, 2021
@wpdarren wpdarren self-assigned this Oct 19, 2021
@wpdarren
Copy link
Collaborator

wpdarren commented Oct 19, 2021

QA Update: ❌

As per chat in Slack, when I connect Site Kit / Search console, I am redirected to the Dashboard, and receive a ‘Additional Permissions required’ Sending back to Execution.

db

@aaemnnosttv
Copy link
Collaborator

Back to you @wpdarren for another pass 👍

@aaemnnosttv aaemnnosttv assigned wpdarren and unassigned aaemnnosttv Oct 20, 2021
@wpdarren
Copy link
Collaborator

wpdarren commented Oct 20, 2021

QA Update: ⚠️

@tofumatt I have two observations:

User Avatar is centered when hovered.

When you hover over the User Avatar the background does not seem the same as other items in the header, i.e. Search URL button on hover. Not as much padding? I do see that the avatar is centered though. Thoughts? - Screenshot

The search icon is on the right with enough spacing.

Looks fine on desktop but there are spacing/styling issues on smaller screen sizes. - Screenshot

It's possible that these will be picked up in #4049 but wanted to check.

@tofumatt
Copy link
Collaborator Author

@wpdarren Yes, those issues are fine—the avatar seems okay to me (and how those avatars look on other Google products).

The header should be fixed elsewhere, I think the main thing is to hide the "Site Kit" text on mobile, but it's not part of this issue I think 👍🏻

So should be good 🙂

@wpdarren
Copy link
Collaborator

QA Update: ✅

Verified:

  • When the feature flag is not enabled, no changes to the header are made.

image

  • The entire Header is "sticky" and remain on-screen, even as the page is scrolled. Checked this on desktop and smaller screen sizes.

  • The user menu never shows the user's login/email; it shows only the user's avatar. Tested this on desktop and smaller screen sizes.

image

  • A new button is added for the "URL search" feature and it looks like the AC screenshot.

  • When the "URL Search" button is clicked it expands to show an component.

image

  • When the input is blurred it disappears and the "URL Search" button appears again. I tested this by clicking outside of the input field and also tabbing away.

  • Input field is focused when search button is clicked.

  • The search icon is on the right with enough spacing. See observation re. small screen sizes.

  • User Menu has a tooltip "Account".

@wpdarren wpdarren removed their assignment Oct 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P0 High priority Rollover Issues which role over to the next sprint Type: Enhancement Improvement of an existing feature
Projects
None yet
Development

No branches or pull requests

9 participants