Create and implement a Stepper component. #2246

fairlighteth · 2022-01-21T11:41:25Z

Summary

Adds a Stepper component and uses this in our Claim flow

Screen.Recording.2022-01-21.at.12.38.08.mov

Screen.Recording.2022-01-21.at.12.37.40.mov

Note

Stepper not optimized for mobile/responsive

fairlighteth · 2022-01-21T11:48:36Z

Update:

Changed some styles, where only the active step has a bold step text title:

- Cleaned up unused styled components, in relation to our previous 'stepper'.

github-actions · 2022-01-21T11:51:32Z

🔭 GP Swap: CoW Protocol v2 Swap UI

elena-zh

@biocom , great changes!
A small question: will we change name 'Approve tokens' button to the 'Next' (or so) here or in a separate PR?

fairlighteth · 2022-01-21T12:48:53Z

@elena-zh Ah yes I recall the discussion. Changed it to 'Continue'. WDYT?

elena-zh · 2022-01-21T12:55:53Z

@biocom , sounds great!
Thanks!

fairlighteth · 2022-01-21T12:57:09Z

@biocom , sounds great! Thanks!

This has been committed to this PR. Thank you.

anxolin

Looks great, its a big difference in UX ! 🔥

Approved with minor comments.

One suggestion for another PR is to change this to:

With some text like:

You have chosen to exercise some investment opportunities. Investing will give you the chance to get vCOW token at at fixed price. 

This process will consist in two steps. The first one will be to define the investment amounts and set the required allowances. This means, that you will need to approve the investment for the investing tokens. 

The last step is to execute all the claiming opportunities on-chain. This step will send the vCOW tokens for both Airdrop and the selected investment opportunities. Additionally, this step will take the invested tokens.

For more details on this, please read [BLOG POST]

src/custom/components/Stepper/index.tsx

src/custom/pages/Claim/InvestmentFlow/index.tsx

fairlighteth · 2022-01-21T14:30:11Z

@anxolin Thank for the suggestions. As for the text I updated it to this:

What do you think?

As for the Stepper component I've refactored things :)

W3stside · 2022-01-21T14:55:32Z

src/custom/components/Stepper/index.tsx

+        const completedStep = activeStep > id
+        const isActiveStep = activeStep === id
+        return (
+          <Step key={id} totalSteps={steps.length} isActiveStep={isActiveStep} completedStep={completedStep}>


bit overkill as there are only 3 steps but cool anyhow

Just keeping in mind, this was setup as a re-usable component (outside Claim purposes).

W3stside · 2022-01-21T14:56:03Z

src/custom/pages/Claim/InvestmentFlow/index.tsx

+    title: 'Start',
+  },
+  {
+    id: 1,


why do you need an id if the index/order is deterministic? it's an array

@W3stside My thinking was that this would be a more fool proof approach. Order of an Array in the context of this small Array of objects const indeed is deterministic.

In context of a general purpose Stepper component, my understanding is that in scenarios where you might have missing (object) values in the Array or externally imported data, one could argue setting explicit step numbers might be a defensive way of setting it up, given the order/index number might be affected in those scenarios. But correct me if I'm wrong.

Happy to do it another way, but just to highlight.

Feel free to review post merge. For now I reverted logic back to use index for key.

Then make id optional as a prop and use the index as a fallback

Sure Michel, makes sense if you plan to feed this with dynamic data which doesn't have order guarantees.
My thinking was, it's just using a static deterministic array, and in practical terms it will always be the case. I don't see the stepper consuming a dynamic list from an untrusted REST API :P

Anyways, it's all good, i consider the comment a ubernit :)
Fine to leave it, fine to remove it, fine to make it optional

I see, could also be an option. A bit messy potentially in the end where index and id are thrown together and mixed up? I think I leave it as per your first suggestion and use only index for now. Feel free to modify post merge otherwise.

@anxolin Yeah, probably I'm over thinking it at this moment :)

src/custom/components/Stepper/index.tsx

W3stside · 2022-01-21T14:57:32Z

src/custom/components/Stepper/index.tsx

+  &::before {
+    left: 0;
+    right: 50%;
+    margin-right: ${({ circleSize }) => (circleSize ? `${circleSize}px` : '42px')};


could be useful to cache 42px here as you're clearly setting it in several places. would make tweaking this a breeze later and prevent 1 change from skewing the others

Agreed. Will set up CSS vars in the parent.

@W3stside Updated to reflect this now.

W3stside · 2022-01-21T15:00:02Z

src/custom/pages/Claim/InvestmentFlow/index.tsx

+          The first step allows you to define the investment amounts and set the required allowances for the tokens you
+          will use to invest with. <br />
+          <br />
+          The last step executes all claiming opportunities on-chain. In addition it sends the tokens you will use to


Suggested change

The last step executes all claiming opportunities on-chain. In addition it sends the tokens you will use to

The last step executes all claiming opportunities on-chain. Additionally, it sends the tokens you will use to

Preference (both are correct). Will update otherwise.

Nah not really, "In addition" is awkward like that

src/custom/pages/Claim/InvestmentFlow/index.tsx

W3stside

check comments and address first please

anxolin

Nice changes, i really like this new component

fairlighteth · 2022-01-21T15:40:30Z

Updated text with a wording suggestion by @W3stside but also the latest text provided by @avsavsavs

elena-zh · 2022-01-21T16:30:45Z

@biocom , could you please remove this exceeding period?

Or move it to the end of the previous sentence..

alfetopito

Excellent, looks amazing!

One tiny suggestion regarding a period and this:

I find it funny that there are clearly 3 steps on top but we say there are two steps 😆

I know it was there before, and you could say it's 0 indexed hehe, but still sounds weird.

src/custom/pages/Claim/InvestmentFlow/index.tsx

Co-authored-by: Leandro Boscariol <[email protected]>

fairlighteth · 2022-01-21T17:25:56Z

@alfetopito

I find it funny that there are clearly 3 steps on top but we say there are two steps 😆
I know it was there before, and you could say it's 0 indexed hehe, but still sounds weird.

Yeah... I see your point. At the end, it is still accurate in a way (1 step done, 2 more to go? 😆 ). Open to address it, maybe in a next PR to keep this one going.

biocom added 2 commits January 21, 2022 10:45

Fix duplicate CSS properties.

aee5ea5

Create and implement a Stepper component.

f5e2dc4

fairlighteth requested review from a team January 21, 2022 11:41

biocom added 2 commits January 21, 2022 12:42

Cleanup comment

8a44672

Further styling of active step text.

54952c4

elena-zh approved these changes Jan 21, 2022

View reviewed changes

Change button to use 'continue' text.

3bfe1bb

elena-zh mentioned this pull request Jan 21, 2022

Do not show 'Approve' tokens screen when ETH option is selected/tokens are approved #2168

Closed

anxolin approved these changes Jan 21, 2022

View reviewed changes

src/custom/components/Stepper/index.tsx Outdated Show resolved Hide resolved

src/custom/pages/Claim/InvestmentFlow/index.tsx Outdated Show resolved Hide resolved

Modify text + refactor stepper.

98b26be

W3stside reviewed Jan 21, 2022

View reviewed changes

src/custom/components/Stepper/index.tsx Show resolved Hide resolved

W3stside reviewed Jan 21, 2022

View reviewed changes

src/custom/pages/Claim/InvestmentFlow/index.tsx Outdated Show resolved Hide resolved

W3stside approved these changes Jan 21, 2022

View reviewed changes

anxolin approved these changes Jan 21, 2022

View reviewed changes

biocom added 3 commits January 21, 2022 16:25

Use a CSS var to store the default iconSize

6d2c04e

Use a CSS var to store the default iconSize

9ea99b9

Wording suggestions.

439f761

Base automatically changed from claim-styleImprove-27 to develop January 21, 2022 16:41

alfetopito approved these changes Jan 21, 2022

View reviewed changes

src/custom/pages/Claim/InvestmentFlow/index.tsx Outdated Show resolved Hide resolved

fairlighteth and others added 2 commits January 21, 2022 18:22

Update src/custom/pages/Claim/InvestmentFlow/index.tsx

bd4e4a5

Co-authored-by: Leandro Boscariol <[email protected]>

Fix code style issues with Prettier

0817c89

biocom added 2 commits January 21, 2022 18:28

Revert key id to use index instead.

b3025a6

Revert key id to use index instead.

4ad4781

fairlighteth merged commit 56fbeb1 into develop Jan 21, 2022

alfetopito mentioned this pull request Jan 25, 2022

[RELEASE ⛰️ ] v1.10.0-RC.0 #2289

Merged

alfetopito deleted the claim-styleImprove-28 branch February 1, 2022 17:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create and implement a Stepper component. #2246

Create and implement a Stepper component. #2246

fairlighteth commented Jan 21, 2022

fairlighteth commented Jan 21, 2022

github-actions bot commented Jan 21, 2022

elena-zh left a comment

fairlighteth commented Jan 21, 2022

elena-zh commented Jan 21, 2022

fairlighteth commented Jan 21, 2022

anxolin left a comment •

edited

Loading

fairlighteth commented Jan 21, 2022

W3stside Jan 21, 2022

fairlighteth Jan 21, 2022

W3stside Jan 21, 2022

fairlighteth Jan 21, 2022 •

edited

Loading

fairlighteth Jan 21, 2022

W3stside Jan 21, 2022

anxolin Jan 21, 2022

fairlighteth Jan 21, 2022 •

edited

Loading

fairlighteth Jan 21, 2022

W3stside Jan 21, 2022

fairlighteth Jan 21, 2022

fairlighteth Jan 21, 2022

W3stside Jan 21, 2022

fairlighteth Jan 21, 2022

W3stside Jan 21, 2022

W3stside left a comment

anxolin left a comment

fairlighteth commented Jan 21, 2022

elena-zh commented Jan 21, 2022 •

edited

Loading

alfetopito left a comment •

edited

Loading

fairlighteth commented Jan 21, 2022

	The last step executes all claiming opportunities on-chain. In addition it sends the tokens you will use to
	The last step executes all claiming opportunities on-chain. Additionally, it sends the tokens you will use to

Create and implement a Stepper component. #2246

Create and implement a Stepper component. #2246

Conversation

fairlighteth commented Jan 21, 2022

Summary

Note

fairlighteth commented Jan 21, 2022

github-actions bot commented Jan 21, 2022

elena-zh left a comment

Choose a reason for hiding this comment

fairlighteth commented Jan 21, 2022

elena-zh commented Jan 21, 2022

fairlighteth commented Jan 21, 2022

anxolin left a comment • edited Loading

Choose a reason for hiding this comment

fairlighteth commented Jan 21, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fairlighteth Jan 21, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fairlighteth Jan 21, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

W3stside left a comment

Choose a reason for hiding this comment

anxolin left a comment

Choose a reason for hiding this comment

fairlighteth commented Jan 21, 2022

elena-zh commented Jan 21, 2022 • edited Loading

alfetopito left a comment • edited Loading

Choose a reason for hiding this comment

fairlighteth commented Jan 21, 2022

anxolin left a comment •

edited

Loading

fairlighteth Jan 21, 2022 •

edited

Loading

fairlighteth Jan 21, 2022 •

edited

Loading

elena-zh commented Jan 21, 2022 •

edited

Loading

alfetopito left a comment •

edited

Loading