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

Nav Unification: Show a modal to customers the first time they log in #47049

Closed
cpapazoglou opened this issue Nov 3, 2020 · 30 comments · Fixed by #49593
Closed

Nav Unification: Show a modal to customers the first time they log in #47049

cpapazoglou opened this issue Nov 3, 2020 · 30 comments · Fixed by #49593
Assignees
Labels
[Feature] Calypso & wp-admin Navigation All navigation in Calypso and wp-admin, and the unified transitions between the two. [Pri] BLOCKER Requires immediate attention. [Type] Task

Comments

@cpapazoglou
Copy link
Contributor

cpapazoglou commented Nov 3, 2020

What

If a user is logging in for the first time since the nav unification project is enabled, show them a modal explaining the recent changes.

Why

We need to help customers orient themselves to the change by explaining what has changed, the reasons behind it and how it will benefit them.

Acceptance criteria

  • Should a user login for the first time since the nav unification change. They will see a modal which explains the recent changes to them.
  • Should a user close the modal, they will not be presented with this again upon future logins.

Resources

For any design improvements, adding some animated GIFs would be preferred see #47049 (comment)

Design i1 JPGs:

Design i2 GIFs, JPGs + mp4 walkthrough videos:

Design files:

Related to #47047

@cpapazoglou cpapazoglou added [Type] Task [Feature] Calypso & wp-admin Navigation All navigation in Calypso and wp-admin, and the unified transitions between the two. labels Nov 3, 2020
@cpapazoglou cpapazoglou added this to the Nav Unification v1 Rollout milestone Nov 3, 2020
@cpapazoglou
Copy link
Contributor Author

@davemart-in could you please elaborate more on this? Do we have any designs, user journey?

@davemart-in
Copy link
Contributor

I'll cc @sfougnier on this one.

@ghost ghost self-assigned this Nov 4, 2020
@obenland obenland added the [Status] Needs Design Add this to PRs that need input from Design label Nov 16, 2020
@ghost
Copy link

ghost commented Nov 23, 2020

I have been exploring using a stepped approach that allows us to showcase the updated features. The intention of this is to introduce the unified nav and tell a brief story of how it works in an attempt to cut down on any confusion.

A few things:

  • The copy is not final but I wanted to get the design out in the world.
  • Is there potential to use animated gifs in lieu of static imagery? It could help tell the story and show off specific interactions.

Desktop

Desktop - Modal 1

Desktop - Modal 2

Desktop - Modal 3

Mobile

Screen Shot 2020-11-23 at 6 02 59 PM

Figma link

@cpapazoglou
Copy link
Contributor Author

Thanks @sfougnier for working on this! This stepped approach seems very good and makes sense!

The copy is not final but I wanted to get the design out in the world.

I understand that the copy is not final, but is the wp-admin wording something that may confuse most of the simple users? Many of them may not even know its existence. A usp for all the users would be "Faster navigation" cause we have surfaced the most used links (Posts, Pages) and we have also used the flyouts.

Is there potential to use animated gifs in lieu of static imagery? It could help tell the story and show off specific interactions.

I don't see a blocker here, go for it! We just need to make sure that we create images / gifs for the variation that we will launch (currently we have developed only i1).

@ghost
Copy link

ghost commented Nov 24, 2020

is the wp-admin wording something that may confuse most of the simple users?

Agreed, I definitely don't think we would want to include that jargon here. I imagine that can be cut, but I am not the best wordsmith and wanted to express intent here.

I don't see a blocker here, go for it! We just need to make sure that we create images / gifs for the variation that we will launch (currently we have developed only i1).

Awesome! I can work on creating those.

@ghost
Copy link

ghost commented Dec 3, 2020

For pairing this modal with the sidebar design that's currently being tested, let's use static designs:
Desktop - Modal 4
Desktop - Modal 5
Desktop - Modal 6

Screen Shot 2020-12-14 at 10 43 35 AM

For any design improvements, adding some animated GIFs would be preferred:
Desktop
Mobile

Design i1 JPGs
Desktop.zip
Mobile.zip

Design i2 GIFs, JPGs + mp4 walkthrough videos
Desktop.zip
Mobile.zip

Design files
Figma designs

@obenland obenland removed the [Status] Needs Design Add this to PRs that need input from Design label Dec 8, 2020
@obenland obenland unassigned ghost Dec 8, 2020
@obenland
Copy link
Member

obenland commented Dec 8, 2020

@davemart-in Priority-wise, should this still go into next iteration or should we focus on alpha feedback first?

@cpapazoglou
Copy link
Contributor Author

@sfougnier I am afraid you have created visuals with the new designs (i2) that are not developed. Should we create the visuals with i1? Or is it something I am missing?

@cpapazoglou
Copy link
Contributor Author

@davemart-in , sorry for the double ping ❤️ ! In which environments / cases should this modal be shown:

  • Calypso Screens
  • Calypso block editor
  • wp-admin

Also, are we showing it only once?

@davemart-in
Copy link
Contributor

davemart-in commented Dec 8, 2020

should this still go into next iteration or should we focus on alpha feedback first?

@obenland Up to you. We'll need both before launch.

In which environments / cases should this modal be shown:

@cpapazoglou I'd say we could keep it to Calypso Screens to reduce scope.

Also, are we showing it only once?

Yes, users should only ever see this modal once.

@ghost
Copy link

ghost commented Dec 14, 2020

Should we create the visuals with i1? Or is it something I am missing?

@cpapazoglou I added i1 visuals to my comment above. Let's keep anything i1 static for now and reserve the fancy GIFs for any design improvements.

@getdave getdave added the [Pri] High Address as soon as possible after BLOCKER issues label Jan 25, 2021
@getdave
Copy link
Contributor

getdave commented Jan 25, 2021

Scope and designs look to have been confirmed.

This now requires investigation and quick research into how much effort is required so we have a handle on scope.

@frontdevde
Copy link
Contributor

Note: there's a WP.com What’s New Product Announcements feature (pbBQWj-AH-p2) that will do something similar in Calypso. They are using the core Guide component and it was suggested we consider this as well.

@tjcafferkey tjcafferkey self-assigned this Feb 1, 2021
@tjcafferkey
Copy link
Contributor

Couple of questions:

  • Should we only be showing this to existing users, or new users as well?
  • To avoid any dependency on Calypso should we create this in /packages/[announcements / nav-unification] in case scope increases in future and this does end up getting rolled out to other environments? (E.g. WPCOM Block Editor, WP Admin)

@cpapazoglou
Copy link
Contributor Author

Should we only be showing this to existing users, or new users as well?

That's a good question. Current plans, include enabling nav-unification for specific segments ( eg all users with User IDs ending in 3 ) and then ramping up. With that in mind, new users should also be shown that modal when their User ID is included to the launched segment. On the other hand, since we are not a/b testing at all it could make sense enabling nav-unification for all new users and thus not needing to show that modal. But again, showing that modal doesn't do any harm in any case. So, for the sake of this issue, I lean to yes.

@tjcafferkey
Copy link
Contributor

Thanks @cpapazoglou, I guess it will be based on the following criteria then:

  • User is an existing user, or a new user.
  • User falls into the segment which nav unification is enabled for.
  • User has not seen this modal before.

On this last point. How do we want to track this state? The obvious answer would be localStorage however we know that if the user:

  • Clears browser data
  • Uses a different browser
  • Uses a different device

Then they will be shown the modal repeatedly which could become annoying/frustrating over time. For the Block Editor NUX modal this value is sent to a specific endpoint when dismissed. For comparison the Whats New modal isn't tracked at all, since it's accessible via the menu in the editor.

Any ideas/preference around this aspect of the functionality?

@cpapazoglou
Copy link
Contributor Author

cpapazoglou commented Feb 1, 2021

I guess it will be based on the following criteria then: ...

Hmm, now that I read my comment, my answer "yes" doesn't make sense. What I meant is, show the modal regardless of whether the user is new or existing. Just use config.isEnabled( 'nav-unification' ) and whether the modal hasn't been shown before.

How do we want to track this state

Not sure how this pbBQWj-AH-p2 will work, I suppose it should already have a tracking mechanism. FWIW though, Banner already has a tracking mechanism

tracksDismissName="calypso_banner_upgrade_dismiss"

@tjcafferkey
Copy link
Contributor

Sorry, I think I miscommunicated what I meant by tracking the state. The tracking you mention above is tracking events, whereas I meant how we want to keep track of the state of the modal so that we know it has been shown (e.g. persistent storage).

I'll move towards a localStorage solution for now and we can update that if required.

@cpapazoglou
Copy link
Contributor Author

cpapazoglou commented Feb 1, 2021

I think I miscommunicated what I meant by tracking the state ...

Nope, you did pretty well! I have probably guided you to the wrong line. Here you are

dismissPreferenceName: PropTypes.string,
dismissTemporary: PropTypes.bool,

with dismissTemporary = false

You can better understand how it works here

dismissCard: () => {
const preference = `${ PREFERENCE_PREFIX }${ ownProps.preferenceName }`;
if ( ownProps.temporary ) {
return setPreference( preference, true );
}
return savePreference( preference, true );
},

@tjcafferkey
Copy link
Contributor

tjcafferkey commented Feb 1, 2021

By the look of things the <Guide /> component does not achieve 100% design parity by default unless some we start overriding the component markup & styles. The guide component renders the image and text on top of each other rather than side-by-side as per the designs.

(Ignore the placeholder image)

Screenshot 2021-02-01 at 16 22 11

The way around this would be to use a <GuidePage /> component but as you can see here this isn't recommended as it's deprecated. (Note: This is what was done for the Whats New modal).

It seems like there are two obvious options here:

  1. Use the <Guide /> component how it's recommended, which will render the image and text on top of each other rather than side-by-side.
  2. Use the (deprecated) <GuidePage /> component with <Guide /> and using additional markup and styles (overriding the core components styles) achieve design parity.

cc: @sfougnier @getdave

@getdave
Copy link
Contributor

getdave commented Feb 2, 2021

Thanks for checking in @tjcafferkey and for the nice research into this.

Using the deprecated component would seem like it could cause maintenance issues down the line. As you say it looks like the pages prop of <Guide /> is the right direction.

The Issue appears to be that by default there is no built in mechanism for a "side-by-side" layout.

How about we spin up Storybook locally and test to see if it's possible to provide markup + style overides to achieve a side-by-side layout using the <Guide>?

Failing that, we should check whether @sfougnier is ok to adjust the design for the sake of expediency. If (as I suspect) it turns out that the side-by-side layout is a critical piece of the design, then we should get some context from the team that implemented the welcome modal as to why they decided it was ok to use the GuidePage and what their upgrade/maintenance plans are for the future.

I'd like to avoid Ajax getting sidetracked with developing an entirely new component if possible.

@ghost
Copy link

ghost commented Feb 2, 2021

What's the reason we can't follow the same side-by-side layout that's in the What's new modal?
This design was meant to reuse that component. It seems like the stacked approach deviates from that.

@tjcafferkey
Copy link
Contributor

This design was meant to reuse that component

@sfougnier sorry for not updating this issue yesterday. The good news is we are able to make it look like the What's New modal by overriding the <Guide /> core CSS (which is what was done in the case of What's New).

However, we may need to deviate from the mobile designs (which look different to the What's New modal) because the markup doesn't allow for certain aspects of it, mainly around the pagination and its position.

This PR should be ready for review today so it would be great if you could also cast your eye over it if you have time 😄

@obenland obenland added [Pri] BLOCKER Requires immediate attention. and removed [Pri] High Address as soon as possible after BLOCKER issues labels Feb 5, 2021
@mmtr
Copy link
Member

mmtr commented Feb 5, 2021

I think there are a couple of things to follow up:

  • Images contain English texts. Should we display them to users with a different language set?
  • We need to make sure that new users don't get this modal, only existing users.

@tjcafferkey
Copy link
Contributor

@mmtr yes you're right. I'll be creating separate issues for these to follow up.

@cpapazoglou
Copy link
Contributor Author

cpapazoglou commented Feb 5, 2021

I think there are a couple of things to follow up:

  • Images contain English texts. Should we display them to users with a different language set?
  • We need to make sure that new users don't get this modal, only existing users.

We would need to iterate on the "new users" condition the day we actually launch to customers.

@davemart-in
Copy link
Contributor

Images contain English texts. Should we display them to users with a different language set?

I don't think this is necessary. The text in the screenshot is UI based, not informational. User's don't need to be able to read the text within the image to understand what is going on.

@mmtr
Copy link
Member

mmtr commented Feb 5, 2021

We would need to iterate on the "new users" condition the day we actually launch to customers.

Right, the rollout will be gradual, so we have to handle that. The modal should show up if the nav unification has been enabled for the current logged user after they signed up.

  • For the beta launch that is: is_a11n && registered_date <= January 21, 2021.
  • For the first rollout (5% of single site users) that is: site_count == 1 && user_id % 100 < 5 && registered_date <= <first_rollout_date>
  • Etc.

EDIT: I guess this can be simplified to:

function shouldDisplayNavUnificationModal() {
  if ( hasDismissedModal() ) {
    return false;
  }

  if ( isNavUnificationDisabled() ) {
    return false;
  }

  return getCurrentUserRegisteredDate() <= <LAST_ROLLOUT_DATE>;
}

And then update <LAST_ROLLOUT_DATE> every time we do a partial rollout.

@ghost
Copy link

ghost commented Feb 5, 2021

Looks like the modal that accidentally debuted earlier was using the wrong image asset. We should be using these since there's no current plan to implement the visual changes:
IMG.zip

@Aurorum
Copy link
Contributor

Aurorum commented Feb 8, 2021

Looks like the modal that accidentally debuted earlier was using the wrong image asset. We should be using these since there's no current plan to implement the visual changes:
IMG.zip

PR in #49827 to update those image assets. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Calypso & wp-admin Navigation All navigation in Calypso and wp-admin, and the unified transitions between the two. [Pri] BLOCKER Requires immediate attention. [Type] Task
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants