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

Token Activation Feature #2381

Merged

Conversation

alicjakujawa
Copy link
Contributor

@alicjakujawa alicjakujawa commented Feb 17, 2021

Description

This PR adds in button for token activation to user navigation bar

New stuff

  • getUserLock resolver
  • UserLock query and types
  • Button which shows user balance

Changes 🏗

  • Updated Numeral component to not show decimals for numbers larger then 5 figures

Deletions ⚰️

  • Removed token cache

TODO ⚰️
Styles updates

Resolves DEV-212

@alicjakujawa alicjakujawa self-assigned this Feb 17, 2021
@linear
Copy link

linear bot commented Feb 17, 2021

DEV-212 Add Token Activation button/label to the user navigation

  • It should show the total amount of tokens.
  • It should show a green dot when all of the tokens are activated.
  • It should show a red dot when NOT all of the tokens are activated.
  • On hover, it should show a tooltip with a different copy depending on the state of the tokens. If they are all activated it should say "All your tokens are activated! 💪".
  • Otherwise, the tooltip should show "You have inactive tokens. 😢".
  • It should have an onClick prop for later use when the popover is implemented.

FireShot Capture 132 - Token Activation – Figma - www.figma.com.png

FireShot Capture 141 - Token Activation – Figma - www.figma.com.png

FireShot Capture 135 - Token Activation – Figma - www.figma.com.png

FireShot Capture 138 - Token Activation – Figma - www.figma.com.png

Figma design:

xxx - Token Activation (Figma)

@alicjakujawa alicjakujawa force-pushed the DEV-212-token-activation-bar branch 2 times, most recently from 5a0f458 to cb137f8 Compare February 22, 2021 10:19
@alicjakujawa alicjakujawa requested a review from a team February 22, 2021 10:19
@alicjakujawa alicjakujawa marked this pull request as ready for review February 22, 2021 11:10
Copy link
Contributor

@chinins chinins left a comment

Choose a reason for hiding this comment

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

Several points:

  • as per my comments, could we rename the balances to active/inactive to avoid confusion and have it corresponding to UI? It took me a while to process the lock names and wallet balances :)

  • focus styles are not correct: i think you need to remove outline & add border color same blue as the font color. Also there is a little stipe of the neighboring color that gets in the outline.

  • from Jack's discord message, I thought we should show decimals when 5 figure numbers or smaller. Right now, no decimals are shown with small numbers.

  • there is no tooltip on hover

  • when I make a payment that increases the token number it doesn't update the dot color & the number of tokens until i refresh the page

How should I test when tokens are inactive? Or is it only possible when we have actions wired?

the current focus styles:
Screenshot 2021-02-22 at 14 17 27

@chinins
Copy link
Contributor

chinins commented Feb 22, 2021

This helps me for my branch. I will rebase and can actually pass proper props :)

@chinins
Copy link
Contributor

chinins commented Feb 22, 2021

If some of my comments are not clear or I misunderstood something, please, let me know and we can also have a quick call to discuss.

@alicjakujawa
Copy link
Contributor Author

alicjakujawa commented Feb 22, 2021

Several points:

  • as per my comments, could we rename the balances to active/inactive to avoid confusion and have it corresponding to UI? It took me a while to process the lock names and wallet balances :)
  • focus styles are not correct: i think you need to remove outline & add border color same blue as the font color. Also there is a little stipe of the neighboring color that gets in the outline.
  • from Jack's discord message, I thought we should show decimals when 5 figure numbers or smaller. Right now, no decimals are shown with small numbers.
  • there is no tooltip on hover
  • when I make a payment that increases the token number it doesn't update the dot color & the number of tokens until i refresh the page

How should I test when tokens are inactive? Or is it only possible when we have actions wired?

the current focus styles:
Screenshot 2021-02-22 at 14 17 27

  • about styles "nitpicks" its for sure not. yet done, it will be in following steps, as this task either way will be a feature branch for this feature, so i will go with them after we will have "base" from which we can work. To not block you and Armando in your tasks :)

  • tooltips - I will also add it, but lets first agree about base.

  • for the last point, i will debug create expenditure saga, probably something there is missing :)

@chinins
Copy link
Contributor

chinins commented Feb 22, 2021

Of course, the base is good. Let's just connect regarding these balances. I'll ping you in discord.

@alicjakujawa alicjakujawa marked this pull request as draft February 22, 2021 16:32
@alicjakujawa alicjakujawa force-pushed the DEV-212-token-activation-bar branch from 9a5aa3e to 201c80d Compare February 23, 2021 09:06
@alicjakujawa alicjakujawa marked this pull request as ready for review February 23, 2021 09:08
@alicjakujawa alicjakujawa force-pushed the DEV-212-token-activation-bar branch from 201c80d to d81d807 Compare February 23, 2021 09:08
Copy link
Contributor

@ArmandoGraterol ArmandoGraterol left a comment

Choose a reason for hiding this comment

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

Nice! Apart from some things Olga already mentioned, and these small things I noticed, this looks good

@alicjakujawa alicjakujawa force-pushed the DEV-212-token-activation-bar branch 2 times, most recently from ef02f21 to aa5a848 Compare February 24, 2021 12:21
Copy link
Member

@rdig rdig left a comment

Choose a reason for hiding this comment

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

I'm fine for the most part with this, however, I left some comments that should be improved upon.

As for the thing Olga mentioned, that this is not complete (missing styles, tooltips, etc): please open another issue to track those, and add them in a serparate PR

src/data/resolvers/user.ts Outdated Show resolved Hide resolved
src/data/resolvers/user.ts Outdated Show resolved Hide resolved
src/modules/core/components/Numeral/Numeral.tsx Outdated Show resolved Hide resolved
@alicjakujawa alicjakujawa force-pushed the DEV-212-token-activation-bar branch 3 times, most recently from 18e38d2 to 2b03fe9 Compare February 24, 2021 16:48
@rdig rdig changed the title Add token activation button to UserNavigation Token Activation Feature Mar 1, 2021
@alicjakujawa alicjakujawa force-pushed the DEV-212-token-activation-bar branch from b6ca514 to 516d0bb Compare March 9, 2021 15:01
chinins and others added 27 commits March 24, 2021 11:21
@ArmandoGraterol ArmandoGraterol force-pushed the DEV-212-token-activation-bar branch from 165f3ff to 6d22f18 Compare March 24, 2021 14:38
@ArmandoGraterol ArmandoGraterol merged commit 763a4f6 into feature/reputation-voting Mar 24, 2021
@ArmandoGraterol ArmandoGraterol deleted the DEV-212-token-activation-bar branch March 24, 2021 14:41
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