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

Stepper: Added FlowProgress component to stepper #63413

Merged
merged 3 commits into from May 11, 2022

Conversation

nelsonec87
Copy link
Contributor

@nelsonec87 nelsonec87 commented May 9, 2022

Changes proposed in this Pull Request

  • Added a new component, FlowProgress, that shows information like "Step 2 of 10".

Testing instructions

  • If you want to show the FlowProgress in a step you should add a new prop to StepContainer:
...
const stepProgress = { flowLength: 10, currentStep: 4 };
...

<StepContainer
   ...
   stepProgress={ stepProgress }
   ...
/>

image

In this example the values are hard-coded, but the idea is to store this information in the ONBOARD_STORE, so it's manageable from the flow configuration instead on the step (since the step may be used in different flows).
The store changes are on a different PR.

Related to #63355

@nelsonec87 nelsonec87 requested a review from a team May 9, 2022 12:03
@nelsonec87 nelsonec87 self-assigned this May 9, 2022
@matticbot matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label May 9, 2022
@matticbot
Copy link
Contributor

matticbot commented May 9, 2022

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

App Entrypoints (~97 bytes added 📈 [gzipped])

name           parsed_size           gzip_size
entry-stepper       +301 B  (+0.0%)      +97 B  (+0.0%)

Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used.

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.

@andres-blanco
Copy link
Contributor

andres-blanco commented May 9, 2022

I was looking at this other PR that implements this feature and I find it strange that the data management is not done as part of this PR. Maybe we should add a key prop so that the store can handle many different FlowProgress components?

We could have only one selector that receives a key as parameter and returns the specific count for that key.

@nelsonec87
Copy link
Contributor Author

I was looking at this other PR that implements this feature and I find it strange that the data management is not done as part of this PR. Maybe we should add a key prop so that the store can handle many different FlowProgress components?

We could have only one selector that receives a key as parameter and returns the specific count for that key.

I just created a separate PR for the sake of having the smallest self-contained PR, and in this case the step progress can technically be used without the store part. And we could be easier to review.

When you say a 'key', do you mean in the data management side, right? Like we pass the siteId for most SITE_STORE stuff?
I thought about it, but I didn't see the need for it. Did I get your comment right?

@nelsonec87 nelsonec87 force-pushed the add/step-progress-indicator branch from e6e907f to f89ed45 Compare May 10, 2022 21:18
Copy link
Contributor

@andres-blanco andres-blanco left a comment

Choose a reason for hiding this comment

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

LGTM! Just verify why the tests are failing to be sure if it's safe to merge.

@nelsonec87 nelsonec87 merged commit 0e0cefd into trunk May 11, 2022
@nelsonec87 nelsonec87 deleted the add/step-progress-indicator branch May 11, 2022 17:57
@github-actions github-actions bot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label May 11, 2022
@a8ci18n
Copy link

a8ci18n commented May 11, 2022

This Pull Request is now available for translation here: https://translate.wordpress.com/deliverables/7307319

Thank you @nelsonec87 for including a screenshot in the description! This is really helpful for our translators.

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.

None yet

4 participants