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

Minor UX/UI issue on Dashboard Sharing feature tour in specific scenario #5521

Closed
wpdarren opened this issue Jul 8, 2022 · 6 comments
Closed
Labels
P1 Medium priority Type: Bug Something isn't working

Comments

@wpdarren
Copy link
Collaborator

wpdarren commented Jul 8, 2022

Bug Description

A very minor UX/UI observation. Where the user dismisses the tour at step 1 and they click on the dashboard sharing settings icon, the tour is triggered again. When the tour tooltip appears on 'add user role' the highlighted area slightly overlaps the icon. It's more of an issue on Safari but noticed it on Chrome too.

image

Worth noting that moving to the next step then moving back will move the outline to the correct place:

CleanShot 2022-07-08 at 14 32 20


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

Acceptance criteria

  • When dismissing the first step of the feature tour for Dashboard Sharing, then opening Dashboard Sharing modal and seeing the next step of the tour, the tour should appear over the "Add Roles" section without looking like it's "off". eg it should look like this:

CleanShot 2022-07-08 at 14 31 57

Implementation Brief

In assets/js/components/dashboard-sharing/DashboardSharingSettingsButton.js:

  • Change the following useEffect to a useCallback, assigning the result to a constant with a suitably descriptive name, and remove the use of dialogOpen from it.
    useEffect( () => {
    if ( dialogOpen && ! triggeredTourRef.current ) {
    triggeredTourRef.current = true;
    triggerOnDemandTour( sharingSettingsTour );
    }
    }, [ dialogOpen, triggerOnDemandTour ] );
  • Pass the resulting callback to the Dialog using the onOpen prop.

Test Coverage

  • No new tests needed.

QA Brief

  • Enable the dashboard sharing feature flag
  • Open the feature tour and dismiss it after its first step
  • Open the dashboard sharing settings again
  • The feature tour should resume in the correct step, with the tooltip positioned correctly above the relevant component.

Changelog entry

  • N/A
@wpdarren wpdarren added the Type: Bug Something isn't working label Jul 8, 2022
@tofumatt tofumatt added the P0 High priority label Jul 8, 2022
@tofumatt tofumatt assigned tofumatt and unassigned tofumatt Jul 8, 2022
@tofumatt tofumatt added P1 Medium priority and removed P0 High priority labels Jul 11, 2022
@techanvil techanvil assigned techanvil and unassigned techanvil Jul 11, 2022
@aaemnnosttv
Copy link
Collaborator

@techanvil can we solve this by extending the styles here to apply to the settings tour as well?

body.googlesitekit-showing-feature-tour--dashboardSharing {

@techanvil
Copy link
Collaborator

@techanvil can we solve this by extending the styles here to apply to the settings tour as well?

body.googlesitekit-showing-feature-tour--dashboardSharing {

Hi @aaemnnosttv, thanks for pointing this out. I hadn't spotted this aspect. However, having tested I've found it doesn't quite work as desired - the dialog still transitions in, because the required googlesitekit-showing-feature-tour--dashboardSharingSettings class is only added to body once the TourTooltips component has mounted and in this scenario TourTooltip is rendered after the dialog has been toggled open.

Another thing I need to mention is I had made a mistake in my initial IB solution. I had referred to the wrong Dialog component and it was just by chance that the timings were effective. In fact it turns out the Dialog we are using presents an onOpen handler which makes for a more elegant solution, and I've updated the IB accordingly. Please see what you think.

@aaemnnosttv
Copy link
Collaborator

Great find @techanvil 👍

IB ✅

@wpdarren
Copy link
Collaborator Author

wpdarren commented Aug 2, 2022

QA Update: ⚠️

@makiost in the QAB it says:

The feature tour should not pop up again on this branch, compared to the current version where it pops up again.

Is that correct? When I click on dashboard settings icon the modal does pop up with the feature tour appears a it displays correctly now.

image

@maciejost
Copy link
Collaborator

QA Update: ⚠️

@makiost in the QAB it says:

The feature tour should not pop up again on this branch, compared to the current version where it pops up again.

Is that correct? When I click on dashboard settings icon the modal does pop up with the feature tour appears a it displays correctly now.

image

My bad! You're correct, @wpdarren , the tour resumes at the correct spot with the correct tooltip placement. I'll update the QA brief right away!

@wpdarren
Copy link
Collaborator Author

wpdarren commented Aug 3, 2022

QA Update: ✅

Verified:

  • When dismissing the feature tour and then clicking on the dashboard sharing settings icon, the feature tour appears and the tooltip is positioned correctly above the all roles field.
  • Tested this on a few different screen resolutions and the tooltip always highlighted the all roles field.

image

@wpdarren wpdarren removed their assignment Aug 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 Medium priority Type: Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

6 participants