MAS UI integration v2 #189

r-czajkowski · 2022-09-12T14:55:00Z

Depends on: #178

This PR adds support for multi-staking apps authorization.

Changes

display new apps to authorize when a user connects to a wallet and at least one staking provider has an unauthorized staking application,

Screenshots

display authorized status in staking application card,

Screenshots

add validation for multi app staking form- if a user selects an application to authorize they must fill the form otherwise we should display validation messages,

Screenshots

multi staking applications authorization flow- pre-submit tx modal and success modal.

Screenshots

Open a modal to authorize apps for a given staking provider. This modal should be displayed when a user connects to a wallet and at least one staking provider doesn't have authorized at least one staking application.

Display correct statuses for authorized app such as: - check icon, - `authorized` label, - total authorized balance.

github-actions · 2022-09-13T11:13:51Z

Preview uploaded to https://preview.dashboard.test.threshold.network/mas-integration-v2/index.html.

Add `FlexProps` so we can change the styles for the main wrapper.

Create a modal that will be displayed when a user wants to authorize the staking application.

Open a `AuthorizeStakingApps` modal when a user wants to authorize staking applications. We want to display this modal only if the application is not authorized. For increasing authroization amount we want to display a different modal.

github-actions · 2022-09-13T12:51:21Z

Preview uploaded to https://preview.dashboard.test.threshold.network/mas-integration-v2/index.html.

Add validation befor submitting the `Authorize selected apps`.If a user wants to authorize multiple apps they have to select apps using checkboxes and fill the form for each selected app. Before submitting form the dapp triggers validation of selected forms to make sure all forms have been completed by the user.

github-actions · 2022-09-13T14:11:59Z

Preview uploaded to https://preview.dashboard.test.threshold.network/mas-integration-v2/index.html.

If more than one app was selected the validation didn't work correctly. This commit fixes it.

Create a `useSendTransactionFromFn` hook that allows sending transacion from funcion instance so we can pass the instance of funcion from threshold ts library. The `useSendTransacion` use the newly created hook so api of `useSendTransaction` hook remains unchanged.

`appNameToAppService` -> `stakingAppNameToThresholdAppService`. We want to export this variable because will be useful in other places.

Create the new hook that returns the staking application address based on the staking app name.

Just to define type we will implement this modal in follow-up work.

Add hook that authorizes multiple applications and increases the authorization for application.

Add submit function to authorize selected applications.

`displayNewAuthToAuthrozieAppModalEffect` -> `displayNewAppsToAuthrozieModalEffect` `shouldDisplayNewAuthToAuthrozieAppModal` -> `shouldDisplayNewAppsToAuthrozieModal`

github-actions · 2022-09-14T15:57:03Z

Preview uploaded to https://preview.dashboard.test.threshold.network/mas-integration-v2/index.html.

github-actions · 2022-09-14T16:08:26Z

Preview uploaded to https://preview.dashboard.test.threshold.network/mas-integration-v2/index.html.

The `arrow` icon color is set to `currentColor` so will always match to the text color because an arrow is a child of `Link` component. The previous impl won't work in some cases eg: ``` <Button variant="outline" as={ExternalLink} mr={2} href={link} text="text" withArrow /> ``` This will render the arrow in default color even if we pass `color` prop, an arrow won't inherit color from parent.

Open success modal.

Pass the staking application address to a `AuthorizeStakingApps` as a prop- the staking app address is required to authrize app.

github-actions · 2022-09-15T12:55:41Z

Preview uploaded to https://preview.dashboard.test.threshold.network/mas-integration-v2/index.html.

georgeweiler · 2022-09-20T21:28:49Z

...ages/Staking/AuthorizeStakingApps/AuthorizeApplicationsCardCheckbox/AppAuthorizationInfo.tsx

@@ -60,6 +76,16 @@ export const AppAuthorizationInfo: FC<AppAuthorizationInfoProps> = ({
          {`${formatPercentage(slashingPercentage, 0, true)}`}
        </BoxLabel>
      </HStack>
+      {isAuthorizationRequired && isAuthorized && (


I'm running into an issue where my authorizedStake value is undefined for a moment before it loads. This causes formatTokenAmount to crash the app because BigNumber can't handle undefined. Maybe it's a local config issue? The value eventually loads for me, so adding this fixed the issue. Up to you if you think it's worth it. Do you think we we see this bug in production?

{isAuthorizationRequired && isAuthorized && !!authorizedStake && <>content</>}

I think it will be addressed once we add support for fetching data when a user goes to the auth page directly see #189 (comment). Does it work for you if I address it in a separate PR?

yes that works

georgeweiler · 2022-09-20T21:36:36Z

src/pages/Staking/AuthorizeStakingApps/index.tsx

+  }
+
+  const onAuthorizeApps = async () => {
+    const isTbtcSelected = isAppSelected("tbtc")


if think you pull these out into their own variables in the component state such as:

const isTbtcSelected = useMemo(() => isAppSelected("tbtc"))

then you can reference them in this function and in the JSX below without re-rendering the component an 2 extra times when that value is calculated.

The same thing as here #189 (comment). It won't be memoized value because isAppSelected function uses the selectedApps array. To achieve that we should refactor this component to be more "static"- I mean store the boolean for each app in a separate variable like that instead of storing selected apps in an array:

const [isTbtcAppSelected, setTbtcAppSelected] = useState(false) const [isRandomBeaconAppSelected, setRandomBeaconAppSelected] = useState(false) ... <AuthorizeApplicationsCardCheckbox <...otherProps> onCheckboxClick={(_, isChecked: boolean) => setTbtcAppSelected(isChecked)} isSelected={isTbtcAppSelected} />

I think this approach is OK, at least for now, because we have only 2 staking apps. Let me know what you think and maybe let's refactor in a separate PR.

georgeweiler · 2022-09-20T21:36:51Z

src/pages/Staking/AuthorizeStakingApps/index.tsx

          onCheckboxClick={onCheckboxClick}
-          isSelected={selectedApps.map((app) => app.label).includes("tBTC")}
+          isSelected={isAppSelected("tbtc")}


see comment above about extracting this to a variable

georgeweiler · 2022-09-20T21:40:25Z

src/components/Modal/StakingApplications/StakingApplicationsAuthorized.tsx

+  const navigate = useNavigate()
+  const onAuthorizeOtherApps = async () => {
+    closeModal()
+    navigate(`/staking/${stakingProvider}/authorize`)


I think this is OK for now because I am doing the same in many modals, and I don't have a good solution to implement before the launch. But it's going to throw a warning, see:

https://dev.to/jexperton/how-to-fix-the-react-memory-leak-warning-d4i

also nit: why async method here?

Yeah we need to get rid of the warning some day. Reagrding async- it is unnecessary. I'll remove.

`hasAnyUnauthroizedStakes` -> `hasAnyUnauthorizedStakes`

To match Figma designs.

github-actions · 2022-09-21T13:28:34Z

Preview uploaded to https://preview.dashboard.test.threshold.network/mas-integration-v2/index.html.

Print the lower than and bigger than signs. For example when the authorize stake is `44.900 T` for the stake that has `45.000 T` staked it will display that `>99%` is authorized instead of `100%`.

Register MAS listeners only if the `MULTI_APP_STAKING` feature flag is enabled.

github-actions · 2022-09-22T08:30:37Z

Preview uploaded to https://preview.dashboard.test.threshold.network/mas-integration-v2/index.html.

github-actions · 2022-09-22T08:38:48Z

Preview uploaded to https://preview.dashboard.test.threshold.network/mas-integration-v2/index.html.

The effect should be triggered once we get data for staking providers for random beacon and tbtc app.

github-actions · 2022-09-22T09:14:27Z

Preview uploaded to https://preview.dashboard.test.threshold.network/mas-integration-v2/index.html.

Format percentage value more precisely- print the lower than and bigger than signs.

github-actions · 2022-09-22T09:35:55Z

Preview uploaded to https://preview.dashboard.test.threshold.network/mas-integration-v2/index.html.

michalsmiarowski

Left few minor comments that I would like to be addressed before the merge

src/components/Modal/StakingApplications/StakingApplicationsAuthorized.tsx

Wrap the `ExternalLink` component with `forwardRef` to make the `as` prop to work as well.

github-actions · 2022-09-22T11:28:51Z

Preview uploaded to https://preview.dashboard.test.threshold.network/mas-integration-v2/index.html.

To display `100%` if calculated percentage value is 100 instead of `>99%` and `0%` if the calculated percentage value is 0 instead of `<1%`.

github-actions · 2022-09-22T12:01:02Z

Preview uploaded to https://preview.dashboard.test.threshold.network/mas-integration-v2/index.html.

michalsmiarowski

LGTM 🏋️

Open a modal to authorize apps

0bce397

Open a modal to authorize apps for a given staking provider. This modal should be displayed when a user connects to a wallet and at least one staking provider doesn't have authorized at least one staking application.

r-czajkowski requested review from georgeweiler and michalsmiarowski September 12, 2022 14:55

r-czajkowski changed the base branch from main to mas-integration September 12, 2022 14:55

Update staking application card

0bc9be4

Display correct statuses for authorized app such as: - check icon, - `authorized` label, - total authorized balance.

r-czajkowski added 3 commits September 13, 2022 14:23

Update StakeAddressInfo component

11151f0

Add `FlexProps` so we can change the styles for the main wrapper.

Create AuthorizeStakingApps modal

bfea1a2

Create a modal that will be displayed when a user wants to authorize the staking application.

Open AuthorizeStakingApps modal

c703438

Open a `AuthorizeStakingApps` modal when a user wants to authorize staking applications. We want to display this modal only if the application is not authorized. For increasing authroization amount we want to display a different modal.

r-czajkowski added 8 commits September 14, 2022 14:56

Fix submitting apps authorization form

e87c3b1

If more than one app was selected the validation didn't work correctly. This commit fixes it.

Rename and export variable

86c4141

`appNameToAppService` -> `stakingAppNameToThresholdAppService`. We want to export this variable because will be useful in other places.

Add new hook

794edce

Create the new hook that returns the staking application address based on the staking app name.

Define the new modal type

dd148e7

Just to define type we will implement this modal in follow-up work.

Add new staking apps hooks

2f9458f

Add hook that authorizes multiple applications and increases the authorization for application.

Update authorize staking apps modal

6c9d8bc

Add submit function to authorize selected applications.

Fix typos

27617ab

`displayNewAuthToAuthrozieAppModalEffect` -> `displayNewAppsToAuthrozieModalEffect` `shouldDisplayNewAuthToAuthrozieAppModal` -> `shouldDisplayNewAppsToAuthrozieModal`

r-czajkowski added 💎 components 📜 contract integration labels Sep 14, 2022

r-czajkowski added 4 commits September 15, 2022 14:07

Create a success modal for apps authorization

c8ca9a4

Update hook that authrozie staking apps

d37dbd2

Open success modal.

Open auth staking app modal correctly

55a954b

Pass the staking application address to a `AuthorizeStakingApps` as a prop- the staking app address is required to authrize app.

r-czajkowski marked this pull request as ready for review September 15, 2022 13:02

georgeweiler reviewed Sep 20, 2022

View reviewed changes

r-czajkowski added 2 commits September 21, 2022 14:12

Fix typo

bd465ca

`hasAnyUnauthroizedStakes` -> `hasAnyUnauthorizedStakes`

Update copy in multi app staking modal flow

303b6ab

To match Figma designs.

georgeweiler previously approved these changes Sep 21, 2022

View reviewed changes

r-czajkowski added 2 commits September 22, 2022 09:22

Format percentage value more precisely

da5ad3e

Print the lower than and bigger than signs. For example when the authorize stake is `44.900 T` for the stake that has `45.000 T` staked it will display that `>99%` is authorized instead of `100%`.

Conditionally register MAS listeners

9e4e044

Register MAS listeners only if the `MULTI_APP_STAKING` feature flag is enabled.

r-czajkowski dismissed georgeweiler’s stale review via 9e4e044 September 22, 2022 08:13

Remove unnecessary async

fec8758

r-czajkowski added 2 commits September 22, 2022 10:46

Fix typo

d1ea737

Fix listener effect predicate function

f82d8d7

The effect should be triggered once we get data for staking providers for random beacon and tbtc app.

Format percentage value in stake card

3ed47b0

Format percentage value more precisely- print the lower than and bigger than signs.

michalsmiarowski requested changes Sep 22, 2022

View reviewed changes

src/components/Modal/StakingApplications/StakingApplicationsAuthorized.tsx Outdated Show resolved Hide resolved

r-czajkowski added 2 commits September 22, 2022 12:59

Add missing key prop

1c939e8

Fix function component cannot be given refs err

0ca4fe5

Wrap the `ExternalLink` component with `forwardRef` to make the `as` prop to work as well.

Update formatPercentage fn

37a06fb

To display `100%` if calculated percentage value is 100 instead of `>99%` and `0%` if the calculated percentage value is 0 instead of `<1%`.

michalsmiarowski self-requested a review September 22, 2022 12:18

michalsmiarowski approved these changes Sep 22, 2022

View reviewed changes

michalsmiarowski merged commit 4a3568e into main Sep 22, 2022

michalsmiarowski deleted the mas-integration-v2 branch September 22, 2022 12:35

r-czajkowski added this to the v1.3.0 milestone Oct 3, 2022

MAS UI integration v2 #189

MAS UI integration v2 #189

Uh oh!

Conversation

r-czajkowski commented Sep 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Uh oh!

github-actions bot commented Sep 13, 2022

Uh oh!

github-actions bot commented Sep 13, 2022

Uh oh!

github-actions bot commented Sep 13, 2022

Uh oh!

github-actions bot commented Sep 14, 2022

Uh oh!

github-actions bot commented Sep 14, 2022

Uh oh!

github-actions bot commented Sep 15, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

georgeweiler Sep 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Sep 21, 2022

Uh oh!

github-actions bot commented Sep 22, 2022

Uh oh!

github-actions bot commented Sep 22, 2022

Uh oh!

github-actions bot commented Sep 22, 2022

Uh oh!

github-actions bot commented Sep 22, 2022

Uh oh!

michalsmiarowski left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented Sep 22, 2022

Uh oh!

github-actions bot commented Sep 22, 2022

Uh oh!

michalsmiarowski left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

r-czajkowski commented Sep 12, 2022 •

edited

Loading

georgeweiler Sep 20, 2022 •

edited

Loading