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

Side panel #525

Merged
merged 5 commits into from
Jul 29, 2021
Merged

Side panel #525

merged 5 commits into from
Jul 29, 2021

Conversation

pavloMandryk
Copy link
Collaborator

This PR contains the reorganization of the steps, so we can have the same order as shows on issue #217 , and the implementation of substeps.

@vercel
Copy link

vercel bot commented Jul 25, 2021

@pavloMandryk is attempting to deploy a commit to the 1hive Team on Vercel.

A member of the Team first needs to authorize it.

Copy link
Member

@rperez89 rperez89 left a comment

Choose a reason for hiding this comment

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

Awesome man! just some small comments

src/components/Onboarding/Screens/config.js Outdated Show resolved Hide resolved
src/components/Onboarding/Steps/Step.js Show resolved Hide resolved
src/components/Onboarding/Steps/StepsItem.js Outdated Show resolved Hide resolved
Copy link
Member

@0xGabi 0xGabi left a comment

Choose a reason for hiding this comment

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

@pavloMandryk great work. Super smooth. I left several cosmetic changes to the name descriptions.

One thing that I am not convinced is about the CircleGraph component showing both the % of the progress and also the 1/6 step we are. Wonder if we shoudl keep the steps there 🤔

src/components/Onboarding/Screens/config.js Outdated Show resolved Hide resolved
src/components/Onboarding/Screens/config.js Outdated Show resolved Hide resolved
src/components/Onboarding/Screens/config.js Outdated Show resolved Hide resolved
src/components/Onboarding/Screens/config.js Outdated Show resolved Hide resolved
src/components/Onboarding/Screens/config.js Outdated Show resolved Hide resolved
src/components/Onboarding/Screens/config.js Outdated Show resolved Hide resolved
src/components/Onboarding/Screens/config.js Outdated Show resolved Hide resolved
src/components/Onboarding/Screens/config.js Outdated Show resolved Hide resolved
src/components/Onboarding/Screens/config.js Outdated Show resolved Hide resolved
src/components/Onboarding/Steps/StepsPanel.js Show resolved Hide resolved
@pavloMandryk
Copy link
Collaborator Author

I have made some changes based on your suggestions:

  • Changed key to parent in the Screens array. Found a bit confusing the key naming.
  • Took off the 1/6 from the <CircleGraph />. It wasn't providing useful info as we already have the side menu to show in which step the user currently is. Also it wasn't taking into account substeps, so I think it is a good idea to get rid of it.
  • Changed the steps and substeps naming.
  • Had to ajust the font size of substeps so the new names could fit better.
  • Created a constant isTypeStep to check the type of the input to <StepsItem />.

Copy link
Member

@0xGabi 0xGabi left a comment

Choose a reason for hiding this comment

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

Well done! Looking great. Let's merge it.

src/components/Onboarding/Steps/StepsPanel.js Outdated Show resolved Hide resolved
@0xGabi 0xGabi merged commit 995c717 into 1Hive:gardens Jul 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants