Skip to content

Guided Transfer: Notices for ineligibility #7270

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

Merged
merged 10 commits into from
Aug 23, 2016

Conversation

jordwest
Copy link
Contributor

@jordwest jordwest commented Aug 4, 2016

This PR was rebased and depends on the refactoring work in #7347 (Merged)

This PR adds notices informing the user when there are potential issues impeding a Guided Transfer.

Notices:

  • GT unavailable (error)
  • GT on vacation (error)
  • Premium theme (warning)
  • Custom font (warning)

@jordwest
Copy link
Contributor Author

jordwest commented Aug 4, 2016

After implementing the 'unavailable' notice, I'm not sure whether the notice really belongs on that page:
ineligibility

Should we move that notice (and the vacation notice) to the Export page?

@jordwest
Copy link
Contributor Author

jordwest commented Aug 5, 2016

@dllh @mrheston: I've just made some changes to the way the GT ineligibility information is displayed. Would love some design review on this idea:

If there's an issue blocking transfer (vacation/GT disabled) then we'll display a card where the first step would have been:

screen shot 2016-08-05 at 4 44 30 pm

screen shot 2016-08-05 at 4 29 12 pm

Until we have premium theme/custom font auto-fixing implemented:
screen shot 2016-08-05 at 4 43 46 pm

If the GT process can auto-resolve the premium theme/custom font issues, we'll instead display the issues as notices as originally planned:

screen shot 2016-08-05 at 4 56 48 pm

@jordwest jordwest added [Status] Needs Design Review Add this when you'd like to get a review / feedback from the Design team on your PR [Status] In Progress and removed [Status] In Progress labels Aug 5, 2016
@folletto
Copy link
Contributor

folletto commented Aug 5, 2016

Should we move that notice (and the vacation notice) to the Export page?

If possibile (I'm not sure it's feasible) would be ideal to change the message on the first page, without having the user to click "Purchase" first.

Here's a quick mock:

guided-back

guided-fixes

Make sure that each of the action in the "fixes" list has a link to where actually that bit can be fixed. :)

@dllh
Copy link
Member

dllh commented Aug 5, 2016

I think this looks great, and @folletto's feedback seems right as well.

I note that on the fonts notice (the yellow one), we still refer to switching the theme.

@folletto
Copy link
Contributor

folletto commented Aug 5, 2016

I note that on the fonts notice (the yellow one), we still refer to switching the theme.

Cool!

I'd say to adjust it as needed. I think the important bit is to provide a link for each bit — you're more aware than me on what's right in each of the bits. :)

@dllh
Copy link
Member

dllh commented Aug 5, 2016

Oh, I meant in the original yellow notice (not your mockup), which
presumably we'll still show in case the user doesn't resolve the issue in
the step you've kindly suggested.

On Fri, Aug 5, 2016 at 8:02 AM, Davide Casali notifications@github.com
wrote:

I note that on the fonts notice (the yellow one), we still refer to
switching the theme.

Cool!

I'd say to adjust it as needed. I think the important bit is to provide a
link for each bit — you're more aware than me on what's right in each of
the bits. :)


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#7270 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ACnITAwr2WAVuDokyEul3UkmvTPAA30Sks5qcyZtgaJpZM4JchKF
.

@jordwest jordwest force-pushed the add/guided-transfer/ineligibility-notices branch 2 times, most recently from 8558f00 to ffb2285 Compare August 9, 2016 11:36
@jordwest
Copy link
Contributor Author

jordwest commented Aug 9, 2016

Those designs look great ✨

One thing I'm concerned about is that users might not know what a Guided Transfer is, and might be reluctant to rectify those issues without knowing what they're doing it for. How about displaying the issues card above/below the normal description card?

It was becoming difficult to modify the Export and Guided Transfer cards, so I've also done some refactoring work to organize the export page components better in #7347.

I've also started implementing the changes suggested above, but they're still a work in progress.

@folletto
Copy link
Contributor

folletto commented Aug 9, 2016

One thing I'm concerned about is that users might not know what a Guided Transfer is, and might be reluctant to rectify those issues without knowing what they're doing it for. How about displaying the issues card above/below the normal description card?

I'd try to avoid stacking a full-height card and a full-height explanation at the same time, but I think you're right about the lack of explanation there.

Then maybe we can just use the "not available" message displayed immediately (with a line at the end saying "For more details click here." pointing to the support page), and instead use the "checklist" inside as you had before, so one can still read the full description before clicking.

@evilluendas
Copy link
Contributor

What if we reduce a little bit the text on the unavailability info and display it in a popover? Something like this, that would display when hovering the deactivated button:

gt-0c-unavailable

Then we can have the checklist in the second screen, like this:

gt-1c-upgrades

In a next iteration we could consider deactivating the conflicting customizations automatically when you purchase the GT instead of having those two buttons to manually make the changes. (Although having the user to consciously change the theme makes it less prone to unpleasant surprises afterwards, so maybe it's better this way.)

@jordwest
Copy link
Contributor Author

@mrheston That looks great. On the second screen, we'll also need to hide or disable the 'Set up Guided Transfer' card to prevent the user moving to the next step while there are issues.

@jordwest jordwest force-pushed the add/guided-transfer/ineligibility-notices branch 2 times, most recently from aa57ea8 to 4d94c27 Compare August 11, 2016 06:30
@jordwest
Copy link
Contributor Author

Popover and disabled button implemented in 4d94c27:

screen shot 2016-08-11 at 4 26 53 pm

@folletto
Copy link
Contributor

would display when hovering the deactivated button

It's a good solution. We need to make sure it works on mobile, which doesn't have hover tho. Maybe we can swap the disabled button (which is also not really readable) with a notification rectangle with the i icon, and make that both clickable and hoverable?

@jordwest jordwest force-pushed the add/guided-transfer/ineligibility-notices branch 5 times, most recently from c2d8d93 to 6443176 Compare August 22, 2016 05:01
@jordwest
Copy link
Contributor Author

Just finished implementing these suggestions:

screen shot 2016-08-22 at 5 37 42 pm

screen shot 2016-08-22 at 5 39 34 pm

screen shot 2016-08-22 at 5 23 44 pm

I spent more time than I care to admit trying to get that little i aligned with the text 😄

I've left those auto-resolving buttons out for now, there are a whole bunch of UI and API considerations in that (eg, confirmation dialog or undo, the sequence of API calls required) which we should take to another PR. It's pretty simple to resolve the issues manually for now though, here's a quick GIF:

ineligibility_fix

@jordwest jordwest added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. and removed [Status] In Progress labels Aug 22, 2016
@dllh
Copy link
Member

dllh commented Aug 22, 2016

I gave this a test and was able to force GT to be unavailable so that I could see the little gray button and explanatory bubble, but I couldn't repro the yellow notice. I tried making GT available to click through to the host listing screen, then making GT unavailable and clicking a host logo, but I was then able to see the credentials page. Steps to test would be really helpful here.

// Fallback for unknown issue - user should never see this
return <div>
<p>{ translate( `Howdy! It looks like there's something stopping us from being able
to transfer your site. Please contact support and we'll sort it out!` ) }</p>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe link to /help/contact here?

@jordwest
Copy link
Contributor Author

@dllh Was the screen below the one you couldn't reproduce?

screen shot 2016-08-22 at 5 23 44 pm

To reach that screen:

  1. GTs should be enabled generally (not disabled or on vacation)
  2. Switch to a site with a premium theme and/or custom fonts set
  3. Click the Purchase a Guided Transfer button
  4. You should see the yellow notice and issues list.
  5. Fix the issues, then return to that screen.
  6. The host selection screen should appear.

@dllh
Copy link
Member

dllh commented Aug 23, 2016

Ah, yeah, that was it. I tested that flow, disabled the offending upgrades, and was able to move through the flow to the next step. The code looks pretty uncontroversial to me, so let's 🚢, maybe with the addition of the link I've suggested.

@jordwest jordwest force-pushed the add/guided-transfer/ineligibility-notices branch from b6336b4 to d22184b Compare August 23, 2016 05:19
@jordwest
Copy link
Contributor Author

Rebased and added the support link in 391ba34

@jordwest jordwest merged commit 237c357 into master Aug 23, 2016
@jordwest jordwest deleted the add/guided-transfer/ineligibility-notices branch August 23, 2016 07:22
@jordwest jordwest removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Aug 23, 2016
@lancewillett lancewillett removed the [Status] Needs Design Review Add this when you'd like to get a review / feedback from the Design team on your PR label Oct 19, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants