Importers: Move action buttons to the bottom of each screen #31364

spen · 2019-03-11T19:35:12Z

This PR addresses issues with buttons in both the importers that expect the user to upload a file and the ones that expect them to input a URL, namely:

Inconsistencies in button positioning.
Unclear button text.
Clashing button colours.

At the moment, buttons appear in different places in the importer pane, depending on the type of importer and stage of the import. @dezzie's Issues #31092 and #30564 propose a more consistent placement of all buttons in the bottom left corner of the importer pane. They also recommend some small changes to button text, and suggest removing the "scary" style from some buttons whose colour seems to clash with the magenta of the new Muriel primary style buttons.

Testing

Run through the whole importing flow with both a Wix import and a file-based import (WordPress).
Try cancelling at various points in the flow - does the button do what you expected in each place? Does the copy best represent the action?
Take the the flows through to completion - did the loading states look right? Did you get stuck at any point?

Note

The approach used here isn't perfect - because of limitations around state and certain actions being baked in to components (rather than abstracted in to the state layer) I've had to place the ActionButtons wherever they have access to to the actions they need to trigger: In the AuthorMappingPane, SiteImporterInputPane , SiteImporterSitePreview , ImportingPane and UploadingPane components.
The alternative would be to properly move out various pieces of state and state-changing actions from the components and in to redux but that has proven tricky in the past and wouldn't be worthwhile at this point.

Suggestion for extra work:

Between entering a URL & hitting enter, and the site-preview being generated there are essentially 2 loading states: The continue button shows it's 'busy' state (while we check the URL's validity), then there's just a spinner (while we generate the mShot preview).
It would be a smoother experience if we could have the busy state of the button represent both the validation and the mShot call.

These screenshots show how the UI of the file upload importers would look with this PR applied.

Import start

Import file uploading
(Ignore the excessive width of the progress bar in these screenshots - we'll fix that in a separate PR.)

Import file uploaded

Author mapping

Import started

Importing resources progress

Import done

matticbot · 2019-03-11T19:35:13Z

Test live: https://calypso.live/?branch=fix/importer-ui-fixes-for-clashing-buttons--try-render-props--2

andfinally · 2019-03-13T17:40:09Z

The code looks good to me - I did run into a weird state issue locally with my Wix import though, so I'd like to give this some more testing tomorrow. I'm not sure if the issue was related to these changes or if it was a pre-existing glitch.

WP import

Start import opens importer
Cancel closes importer
Cancel is greyed out during upload
Cancel after successful upload clears uploaded file from file pane
Upload pane successfully uploads new file after Chrome file upload error interrupts big upload
Continue after upload goes to author mapping view
Cancel leaves author mapping view and returns to importers list
Start Import starts import - button changes to Importing with busy state
Done after import finished returns to importers list.

Wix Import

Start import opens importer, Continue is greyed out
Cancel closes importer
Continue starts processing
Cancel during processing returns to importers list
Cancel on preview pane returns to URL input pane
Yes! Start import on preview pane starts import
Importing with busy state appears (shortly after spinner) during import
Cancel on importer failure goes to importers list

Wix weirdness

My Wix import got stuck. It seemed to be taking a long time. When I eventually refreshed the page, I got the importers list with all importers greyed out - the ImporterStatus for all of them was importer-disabled.
On refreshing again I got just the "stuck" view of the active Wix importer again with the status importer-importing.
I reset the importer state in my blog RC and started a new import with a different URL.
I got through the preview pane and started the import.
The view jumped back to the URL input pane.
Eventually it changed again to the empty importer with the cancel button. (Status was importer-import-failure.) This was the kind of error state I came across earlier today.

andfinally · 2019-03-13T16:10:05Z

client/my-sites/importer/importer-action-buttons/close-button.jsx

-				{ translate( 'Close', { context: 'verb, to Close a dialog' } ) }
-			</Button>
+			<ActionButton disabled={ disabled } onClick={ this.handleClick }>
+				{ translate( 'Cancel', { context: 'verb, to Close a dialog' } ) }


Maybe the context here should be like to Cancel an operation.

andfinally · 2019-03-13T16:13:00Z

client/my-sites/importer/importer-header/index.jsx

-		const { importerStatus, icon, isEnabled, title, description } = this.props;
-		const ButtonComponent = this.getButtonComponent();
+		const { importerStatus, icon, title, description, site } = this.props;
+		const { importerState } = this.props.importerStatus;


Would this be simpler?

const { importerState } = importerStatus;

andfinally · 2019-03-13T16:29:21Z

client/my-sites/importer/uploading-pane.jsx

@@ -121,6 +118,8 @@ class UploadingPane extends React.PureComponent {
 	};

 	render() {
+		const { importerStatus, site, isEnabled } = this.props;
+		const { importerState, importerId } = this.props.importerStatus;


This could be simplified too.

const { importerState, importerId } = importerStatus;

Good spotting on these! Copy pasta I swear!
Those should be handled in 7617ce8 :)

creativecoder

First, very nice work making these changes within this tangled part of our code base and not making it any worse... thank you!

I tested Wix and WordPress imports and everything worked as expected in terms of cancel buttons and stepping through the import. I left a comment about component naming to consider--your call if you think that's worth doing and whether it's worth doing now.

creativecoder · 2019-03-13T22:22:42Z

client/my-sites/importer/author-mapping-pane.jsx

+import ActionButtonContainer from 'my-sites/importer/importer-action-buttons/container';
+import ActionButton from 'my-sites/importer/importer-action-buttons/action-button';
+import CloseButton from 'my-sites/importer/importer-action-buttons/close-button';
+


These components seem odd to me, in that they have generic names but are importing specific. Could we have more descriptive names, like ImportActionContainer, ImportActionButton, ImportCancelButton, etc?

(I know this issue isn't new to this PR)

creativecoder · 2019-03-13T22:23:44Z

client/my-sites/importer/importer-action-buttons/close-button.jsx

-				{ translate( 'Close', { context: 'verb, to Close a dialog' } ) }
-			</Button>
+			<ActionButton disabled={ disabled } onClick={ this.handleClick }>
+				{ translate( 'Cancel', { context: 'verb, to Close a dialog' } ) }


creativecoder · 2019-03-13T22:43:13Z

Pinging @dezzie for a design review.

andfinally · 2019-03-14T09:02:54Z

I don't know if it's just me, but it seems like one effect of the change in the upload pane is to make it less apparent when a file has successfully uploaded - previously, a magenta button popped into view in the middle of the pane, making it very clear the upload had completed. I think the upload success is less obvious now.

andfinally · 2019-03-14T14:17:21Z

I've done a bunch of imports - several Wix, an Importer 6, a Medium - and all went well, so I reckon we're good to go. I'll hold off approving for now in case Spen wants to push any changes.

spen · 2019-03-14T17:48:45Z

The code looks good to me - I did run into a weird state issue locally with my Wix import though, so I'd like to give this some more testing tomorrow. I'm not sure if the issue was related to these changes or if it was a pre-existing glitch.

Sadly, this is all existing weirdness, the only remedy would be to properly address the weird state set up we have there :/

I don't know if it's just me, but it seems like one effect of the change in the upload pane is to make it less apparent when a file has successfully uploaded...

I see what you're saying. I wonder if a nice ✅ icon or similar would be a good indicator here?
Another improvement I can see would be to have the continue button present throughout the upload process, but have it disabled until the file is uploaded, similar to how it is in the URL input pane before it has a value.

dezzie · 2019-03-14T21:24:04Z

@spen, @andfinally, @creativecoder, yay! I took a look, and ran some imports, and I think we are good to merge. I hear you on:

Having something that indicates a successful file upload has taken place in the upload area. The text message is better than nothing, so I would go with what we have now if doing something new would hold us up a few days.
@spen's note about the suggested extra work to smooth out the progress indicators.
that progress bar is a little wide :) but we can address this later since it's visible for only a small amount of time.
Removing the "Back" affordance from the secondary navigation once an import has started, to prevent user errors. for this one, if you'd be comfortable doing it in a separate PR, maybe that'd be smarter, in the event that we get pushback for doing that and we have to undo work.

If we can follow-up on those things and put them in a separate PR for efficiency that'd be great. That also gives me time to run this stuff by the Muriel team without holding stuff up.

spen · 2019-03-14T23:53:24Z

I'll hold off approving for now in case Spen wants to push any changes.

All planned changes are up - the component naming and destructuring fixes, so ready for another 👀!
Thanks a bunch for the thorough reviews @andfinally :)

Designer research indicates that Cancel is a better word to use in the context of “cancel this Import”. See #30564

Addresses #30564

andfinally

Thanks for having a look @dezzie and thanks for the changes Spen! This looks good to me - I've done a couple more successful imports with the latest version and experienced no hiccups. I get what you mean about the tangled state being the cause of occasional weirdness!

I've created Importers: Make file upload success more obvious #31481 for the file upload success UI.
The outsize progress bar was fixed in Importers: restore correct width of importer progress bar #31428.

spen · 2019-03-15T10:48:22Z

I've created #31481 for the file upload success UI.

Awesome, thanks @andfinally :) And thanks for testing again! 🙏

bsessions85 · 2019-03-15T13:37:26Z

@spen It looks like this change broke an e2e test. Can you take a shot at updating the Import a site while signing up test? I would guess it is probably just a change in a css selector

spen · 2019-03-15T13:45:31Z

Thanks for the heads up @bsessions85 - I'll get on this right away :)

spen force-pushed the fix/importer-ui-fixes-for-clashing-buttons--try-render-props--2 branch 3 times, most recently from 97dc9ab to b6e86b8 Compare March 12, 2019 18:33

andfinally force-pushed the fix/importer-ui-fixes-for-clashing-buttons--try-render-props--2 branch from e38a2ef to a6beaba Compare March 13, 2019 11:14

spen force-pushed the fix/importer-ui-fixes-for-clashing-buttons--try-render-props--2 branch 4 times, most recently from f6f98c6 to fa3e55c Compare March 13, 2019 13:53

spen marked this pull request as ready for review March 13, 2019 14:22

spen changed the title ~~Fix/importer ui fixes for clashing buttons try render props 2~~ Importers: Move action buttons to the bottom of each screen Mar 13, 2019

spen requested a review from a team March 13, 2019 14:22

spen added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. Import Site Importer Site Importer related issues labels Mar 13, 2019

spen mentioned this pull request Mar 13, 2019

Importers: More seamless file upload transitions #31433

Merged

andfinally reviewed Mar 13, 2019

View reviewed changes

creativecoder reviewed Mar 13, 2019

View reviewed changes

getdave and others added 5 commits March 14, 2019 23:56

Updates button wording to better convey action in context

7c81521

Designer research indicates that Cancel is a better word to use in the context of “cancel this Import”. See #30564

Importers: Move all action buttons in to consistent space

7684a8b

Addresses #30564

Importers: Render close-button in importing pane if errored

7108ffd

Importers: Prefix action button names

2db9e1e

Importers: Update a translate context message, small code style tweaks

b7e4705

spen force-pushed the fix/importer-ui-fixes-for-clashing-buttons--try-render-props--2 branch from 7617ce8 to b7e4705 Compare March 15, 2019 00:01

andfinally mentioned this pull request Mar 15, 2019

Importers: Make file upload success more obvious #31481

Closed

andfinally approved these changes Mar 15, 2019

View reviewed changes

andfinally removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Mar 15, 2019

spen merged commit 0b72c8c into master Mar 15, 2019

spen mentioned this pull request Mar 15, 2019

e2e Tests: Fix failure introduced in #31364 #31492

Merged

This was referenced Mar 15, 2019

WordPress, Squarespace, Blogger, and Medium site importer UI fixes #31092

Closed

Wix Site Importer: UI fixes for addressing clash of buttons #30564

Closed

andfinally mentioned this pull request Mar 15, 2019

Importer UI fixes - make cancel button secondary to fix colour clashes #31317

Closed

lancewillett deleted the fix/importer-ui-fixes-for-clashing-buttons--try-render-props--2 branch March 9, 2020 22:38

This was referenced May 15, 2021

[Snyk] Security upgrade @wordpress/edit-post from 1.0.4 to 4.0.0 ajesse11x/wp-calypso#50

Open

[Snyk] Fix for 1 vulnerabilities awaisab172/wp-calypso#76

Open

snyk-bot mentioned this pull request Aug 3, 2021

[Snyk] Security upgrade @wordpress/edit-post from 3.5.0 to 4.0.0 awaisab172/wp-calypso#88

Open

snyk-bot mentioned this pull request Sep 5, 2021

[Snyk] Fix for 3 vulnerabilities ajesse11x/wp-calypso#61

Open

ajesse11x mentioned this pull request Oct 1, 2023

[Snyk] Fix for 1 vulnerabilities ajesse11x/wp-calypso#92

Open

awaisab172 mentioned this pull request Oct 2, 2023

[Snyk] Fix for 1 vulnerabilities awaisab172/wp-calypso#160

Open

awaisab172 mentioned this pull request Oct 31, 2023

[Snyk] Fix for 2 vulnerabilities awaisab172/wp-calypso#163

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Importers: Move action buttons to the bottom of each screen #31364

Importers: Move action buttons to the bottom of each screen #31364

spen commented Mar 11, 2019 •

edited by andfinally

Loading

matticbot commented Mar 11, 2019

andfinally commented Mar 13, 2019

andfinally Mar 13, 2019

creativecoder Mar 13, 2019

andfinally Mar 13, 2019

andfinally Mar 13, 2019

spen Mar 14, 2019

creativecoder left a comment

creativecoder Mar 13, 2019

creativecoder Mar 13, 2019

creativecoder commented Mar 13, 2019

andfinally commented Mar 14, 2019

andfinally commented Mar 14, 2019

spen commented Mar 14, 2019

dezzie commented Mar 14, 2019 •

edited

Loading

spen commented Mar 14, 2019

andfinally left a comment •

edited

Loading

spen commented Mar 15, 2019

bsessions85 commented Mar 15, 2019

spen commented Mar 15, 2019

Importers: Move action buttons to the bottom of each screen #31364

Importers: Move action buttons to the bottom of each screen #31364

Conversation

spen commented Mar 11, 2019 • edited by andfinally Loading

Testing

Note

Suggestion for extra work:

matticbot commented Mar 11, 2019

andfinally commented Mar 13, 2019

andfinally Mar 13, 2019

Choose a reason for hiding this comment

creativecoder Mar 13, 2019

Choose a reason for hiding this comment

andfinally Mar 13, 2019

Choose a reason for hiding this comment

andfinally Mar 13, 2019

Choose a reason for hiding this comment

spen Mar 14, 2019

Choose a reason for hiding this comment

creativecoder left a comment

Choose a reason for hiding this comment

creativecoder Mar 13, 2019

Choose a reason for hiding this comment

creativecoder Mar 13, 2019

Choose a reason for hiding this comment

creativecoder commented Mar 13, 2019

andfinally commented Mar 14, 2019

andfinally commented Mar 14, 2019

spen commented Mar 14, 2019

dezzie commented Mar 14, 2019 • edited Loading

spen commented Mar 14, 2019

andfinally left a comment • edited Loading

Choose a reason for hiding this comment

spen commented Mar 15, 2019

bsessions85 commented Mar 15, 2019

spen commented Mar 15, 2019

spen commented Mar 11, 2019 •

edited by andfinally

Loading

dezzie commented Mar 14, 2019 •

edited

Loading

andfinally left a comment •

edited

Loading