-
Notifications
You must be signed in to change notification settings - Fork 295
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
[DDW-569] Move ITN redemption to Daedalus application menu #2401
[DDW-569] Move ITN redemption to Daedalus application menu #2401
Conversation
Hi @yakovkaravelov. Tested on build Mainnet 16676. Functionality works well although I found if the user changes the language to Japanese the button reverts back to being greyed out and inactive. See video. If Daedalus is closed and reopened the button is active again |
Good catch @ManusMcCole! This is obviously a bug that most likely affects develop and production builds too. |
@yakovkaravelov please merge latest develop. |
This bug was not introduced by this PR actually. It is already existing in develop branch as well, anyway, will fix it. |
@yakovkaravelov please fix it regardless of it's origin. |
@tomislavhoracek @nikolaglumac @daniloprates @miorsufianiohk @ManusMcCole Please review/test, thanks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yakovkaravelov I left one comment. Can you please take a look?
And one more question. Why you removed isUpdateAvailable
checks?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good @yakovkaravelov
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. Tested on 16833. Good job @yakovkaravelov 👍
Reviewing this now |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yakovkaravelov The screen can be accessed while Daedalus is loading and it looks like it hasn't any wallet. It changes after the loading is complete, but I think it looks odd. Can this be improved? For reference, the settings and wallet settings screens can't be accessed in that state.
Good point @gabriela-ponce - I have modified the logic so that we show the "syncing" status message instead of no-wallets in case Daedalus is not fully synced. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
This PR moved ITN redemption to Daedalus application menu
Jira Ticket
Todos
Test Cases
Scenario 1 - Validate "Redeem ITN rewards" option is visible on the menu
Scenario 2 - Access to "Redeem ITN rewards" screen
Testing Checklist
App
Daedalus
=>Redeem ITN rewards
and make sure ITN redemption dialog is shown.Screenshots
English
Japanese
Test Cases
Review Checklist
Basics
feature
/bug
/chore
,release-x.x.x
)yarn test
)yarn dev
)yarn package
/ CI builds)yarn flow:test
)yarn lint
)yarn prettier:check
)yarn manage:translations
produces no changes)yarn storybook
)yarn.lock
file is updatedCode Quality
Testing
After Review
done
column on the YouTrack board