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

Improve staking form user experience #643

Merged
merged 10 commits into from
Nov 21, 2023

Conversation

kpyszkowski
Copy link
Contributor

@kpyszkowski kpyszkowski commented Oct 5, 2023

Ref: #634

Added validation if user wallet is empty, simplified and made validation error messages more descriptive, improved readability of validation function, disabled form if wallet is not connected.

Some of the issues were also addressed in #666, which was merged into this PR.

Added validation if user wallet is empty, simplified and made validation error messages more descriptive, improved readability of validation function, disabled form if wallet is not connected.
@github-actions
Copy link

github-actions bot commented Oct 5, 2023

Copy link
Contributor

@michalsmiarowski michalsmiarowski left a comment

Choose a reason for hiding this comment

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

Left some comments to look at before the merge

src/pages/Staking/NewStakeCard.tsx Outdated Show resolved Hide resolved
src/pages/Staking/NewStakeCard.tsx Outdated Show resolved Hide resolved
src/utils/forms.ts Outdated Show resolved Hide resolved
src/utils/forms.ts Outdated Show resolved Hide resolved
src/utils/forms.ts Outdated Show resolved Hide resolved
Renamed variables:
defaultLessThanMsg -> defaultStakingLessThanMessage
defaultGreaterThanMsg -> defaultStakingGreaterThanMessage
defaultValidationOptions -> defaultStakingValidationOptions
ValidationMsg -> AmountValidationMessage
ValidationOptions -> AmountValidationOptions
Applied refinemets, added getAmountInRangeValidationMessage function
@github-actions
Copy link

1 similar comment
@github-actions
Copy link

@michalsmiarowski michalsmiarowski added this to the v1.13.0 milestone Oct 30, 2023
Copy link
Contributor

@michalsmiarowski michalsmiarowski left a comment

Choose a reason for hiding this comment

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

Left two comments to look at before the merge.

Also let's sync it with the main branch

src/pages/Staking/NewStakeCard.tsx Outdated Show resolved Hide resolved
src/pages/Staking/NewStakeCard.tsx Outdated Show resolved Hide resolved
src/components/Forms/TokenAmountForm.tsx Outdated Show resolved Hide resolved
In 42bd970 we've changed the default messages
in a way that they are related to Token staking. This is bad because now such
messages were displayed in tbtc section (for example).

This commit changes the default error message back to where it was before this
commit.
The property name change is not needed because we usually use default `active`
property from `useWeb3React` hook anyway.
The previosu commit introduces `shouldDisableButton` property which might get
a little confusing, because when it's set to true someone might expect that the
button is disabled no matter what, when in reality it relies on `isDisabled`
property also.

Instead of this, I've added `isDisabled` property for `Connect Wallet` button in
SubmitTxButton component and set it to false. This  overrides the `isDisabled`
property from `...buttonProp`, because in this case we don't want ot have this
button disabled anyway.

This means that we can use `isDisabled` to disable token amount form if the
account is not connected but keep the `Connect Wallet` button active.
@michalsmiarowski michalsmiarowski self-requested a review November 21, 2023 17:25
Copy link

1 similar comment
Copy link

Copy link
Contributor

@michalsmiarowski michalsmiarowski left a comment

Choose a reason for hiding this comment

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

LGTM 🔥

@michalsmiarowski michalsmiarowski merged commit 462be69 into main Nov 21, 2023
5 checks passed
@michalsmiarowski michalsmiarowski deleted the feature/staking-form-ux-improvements branch November 21, 2023 17:39
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.

2 participants