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

Fix conditions for red color text in remaining display widgets #3020

Merged
merged 2 commits into from
Dec 10, 2021

Conversation

chinins
Copy link
Contributor

@chinins chinins commented Dec 8, 2021

Description

This PR fixes the conditions for color coding of the remaining display widgets (time & tokens)

Changes 🏗

  • simplify props passing in RemainingTokensValue
  • update isWarning conditions in RemainingTokens & RemainingTime

Resolves #3012

@chinins chinins added the bug label Dec 8, 2021
@chinins chinins requested a review from a team December 8, 2021 15:54
@chinins chinins self-assigned this Dec 8, 2021
Copy link
Member

@arrenv arrenv left a comment

Choose a reason for hiding this comment

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

Working as expected, nice work!

red-updating-less-10-percent

time-colour-10per-red-change

I do have a couple of other things I noticed that you may also be able to include in this PR. However, let me know if you think these should be raised in separate issues.

#1 - Sold out warning on batches is not displaying if the total tokens in the coin machine are sold out.

also-sold-out

#2 - Available tokens on batches not correct.

change-available-on-batch

@chinins
Copy link
Contributor Author

chinins commented Dec 9, 2021

@arrenv, regarding the sold out issue I will fix it in this PR.

Regarding the second issue with incorrect amounts, I think it should be a different issue. Do you have any per user purchase limit that might intervene with this or anything else? I didn't experience this issue.

@arrenv
Copy link
Member

arrenv commented Dec 9, 2021

It is all default, as in the per user limit is 100% and batch size is 400,000.

You do have to let the next batch start though, which is why I skipped forward an hour. I ran the following in terminal.

curl -H "Content-Type: application/json" -X POST --data '{"jsonrpc":"2.0","method":"evm_increaseTime","params":[3600],"id": 1}' localhost:8545 && curl -H "Content-Type: application/json" -X POST --data '{"jsonrpc":"2.0","method":"evm_mine","params":[]}' localhost:8545

@chinins chinins force-pushed the fix/3012-remaining-tokens-text-color-rules branch from ff97f86 to e0a60f7 Compare December 9, 2021 21:18
@chinins
Copy link
Contributor Author

chinins commented Dec 9, 2021

@arrenv, can you please test the token numbers & soldout issues again? Maybe there is something wrong with pulling data from the contracts... The logic is working properly for me (and I had less tokens left than the target):
Screenshot 2021-12-09 at 22 25 52

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.

For me, the color of all 3 widgets is working as expected! And I agree that the second issue should be separated as it's not related to this PR and it may not be a simple, small fix.

@arrenv
Copy link
Member

arrenv commented Dec 10, 2021

@chinins I have been trying to replicate again today with a fresh environment and have not been able to replicate. So, like you said, probably just an environment issue.

Regarding #2 this still seems to be an issue, so I will raise that separately.

@chinins chinins force-pushed the fix/3012-remaining-tokens-text-color-rules branch from 994e64f to 147e3e6 Compare December 10, 2021 15:33
@chinins chinins force-pushed the fix/3012-remaining-tokens-text-color-rules branch from 147e3e6 to 8061e15 Compare December 10, 2021 15:41
@chinins chinins merged commit c541a9a into master Dec 10, 2021
@chinins chinins deleted the fix/3012-remaining-tokens-text-color-rules branch December 10, 2021 17:21
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.

Coin Machine: Widget text is red when it should not be.
3 participants