Added skip button to WP.com checklist

[fditrapani]

Changes proposed in this Pull Request

This PR introduces a skip button to the WP.com checklist. The Jetpack checklist currently doesn't offer a way to skip tasks so I added it in such a way that it should not impact Woo or Jetpack.

For the time being, we will have two ways to skip a task. I will be replace the circle next to the title with an icon in another PR in the future.

Before

After

Jetpack

Testing instructions

• Create a new site on WP.com
• Visit /checklist/[url]
• Click on the skip button and confirm that it is marked complete

Something to note, tasks don't automatically collapse when you mark them complete. Since this is the existing behaviour, and it's a little beyond my expertise, I'd suggest we tackle that in a separate PR.

cc @Automattic/zelda @simison, @jeffgolenski

Fixes # https://github.com/Automattic/zelda-private/issues/56

[matticbot]

Test live: https://calypso.live/?branch=add/skip-button-to-checklist

[matticbot]

Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:

Sections (~85 bytes added 📈 [gzipped])

name         parsed_size           gzip_size
woocommerce       +130 B  (+0.0%)      +42 B  (+0.0%)
plans             +130 B  (+0.0%)      +43 B  (+0.0%)

Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to.

Async-loaded Components (~197 bytes added 📈 [gzipped])

name                                                         parsed_size           gzip_size
async-load-my-sites-checklist-wpcom-checklist-component-jsx       +334 B  (+0.3%)      +65 B  (+0.2%)
async-load-design-playground                                      +130 B  (+0.0%)      +43 B  (+0.0%)
async-load-design-blocks                                          +130 B  (+0.0%)      +51 B  (+0.0%)
async-load-design                                                 +130 B  (+0.0%)      +38 B  (+0.0%)

React components that are loaded lazily, when a certain part of UI is displayed for the first time.

Legend

What is parsed and gzip size?

Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory.
Gzip Size: Compressed size of the JS and CSS files. This much data needs to be downloaded over network.

Generated by performance advisor bot at iscalypsofastyet.com.

[p-jackson]

Tests well, the changes LGTM.

I was a little surprised by the behaviour; that clicking skip marked the step as complete. I was expecting the skip button to just proceed to the next step and leave it unchecked. My knee jerk reaction was "now I've skipped it forever and I can't go back to it!" which of course isn't true.

In saying that I don't think it hurts to try it this way :) It'll be interesting to see if there's any feedback on it if there's more user testing coming up.

[ramonjd]

Just need to wrap Skip in a translate call

[fditrapani]

Thanks for the feedback @ramonjd, @Aurorum, and @p-jackson. I got the fixes in.

I was a little surprised by the behaviour; that clicking skip marked the step as complete. I was expecting the skip button to just proceed to the next step and leave it unchecked. My knee jerk reaction was "now I've skipped it forever and I can't go back to it!" which of course isn't true.

Good point. I think that bug I mentioned where the task stays opened might work in favour of this but you're right — it's something for us to keep an eye on.

[fditrapani]

Looks like the mobile e2e tests are unrelated (or are they?) I don't have write permissions to rerun them, can anyone else? cc @p-jackson @ramonjd

[ramonjd]

I don't have write permissions to rerun them, can anyone else?

I kicked them off again.

Normally you can also remove the Needs e2e testing label and re-add it to trigger a rerun of all the e2e tests.

[fditrapani]

Normally you can also remove the Needs e2e testing label and re-add it to trigger a rerun of all the e2e tests.

Thanks for the tip!

[ockham]

On mobile right now, so I can't really test this PR, but did you verify that 'Skipped' state is persisted after a reload? AFAIK, the backend implementations differ for WP.com and Jetpack, with Jetpack not actually storing any 'skipped' state bool on the server side, so I'd imagine that it would be lost upon reload/coming back to the checklist.

[fditrapani]

Thanks for checking @ockham. You're right that the implementations are different and for that reason, I didn't include the skip button for Jetpack. On the WP.com side, it is handled on the backend and the button basically replicates the existing checkmark next to each task. This means that the complete status is being honoured.

[fditrapani]

This is what the test is reporting as failed ^ which doesn't seem to be related (I find these things really hard to read). Also noting that the reason the test fails isn't consistent. It's mostly around the test timing out.

[simison]

Test failures seem identical to what I saw in #34401 so seems unrelated and safe to merge. 👍

[ockham]

Thanks for checking @ockham. You're right that the implementations are different and for that reason, I didn't include the skip button for Jetpack. On the WP.com side, it is handled on the backend and the button basically replicates the existing checkmark next to each task. This means that the complete status is being honoured.

🤦‍♂️ Uh yeah, my bad. I glanced over the PR on my phone and somehow managed to miss the fact that it was about the non-Jetpack checklist.