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

Handling modal windows #70

Merged
merged 25 commits into from
Dec 20, 2023
Merged

Handling modal windows #70

merged 25 commits into from
Dec 20, 2023

Conversation

kkosiorowska
Copy link
Contributor

@kkosiorowska kkosiorowska commented Dec 8, 2023

This PR inits the stake/ustake flow by handling modal windows. After opening the modal, the sidebar should also be visible. From the sidebar user can also open the docs drawer. This PR adds these components and sets the correct property z-index for elements. The stake and unstake action require several modal windows to be opened in sequence. Let's manage this state with the context. A minor note, let's use context at this point and if we decide to use redux add it in a separate PR.

Screen.Recording.2023-12-14.at.13.02.57.mov

@kkosiorowska kkosiorowska added the 🎨 dapp dApp label Dec 8, 2023
@kkosiorowska kkosiorowska added this to the MVP milestone Dec 8, 2023
@kkosiorowska kkosiorowska self-assigned this Dec 8, 2023
This was referenced Dec 8, 2023
Copy link
Contributor

@r-czajkowski r-czajkowski left a comment

Choose a reason for hiding this comment

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

First pass

dapp/src/theme/utils/zIndices.ts Show resolved Hide resolved
dapp/src/DApp.tsx Outdated Show resolved Hide resolved
dapp/src/theme/Drawer.ts Outdated Show resolved Hide resolved
dapp/src/theme/Drawer.ts Outdated Show resolved Hide resolved
@kkosiorowska kkosiorowska marked this pull request as draft December 12, 2023 20:05
@kkosiorowska
Copy link
Contributor Author

This PR has been updated. Previously, each modal was opened separately. Currently, the modal is one and we only replace the content in it.

@kkosiorowska kkosiorowska marked this pull request as ready for review December 14, 2023 11:57
dapp/src/components/Modals/StakingOverviewModal.tsx Outdated Show resolved Hide resolved
dapp/src/components/Modals/StakeModal.tsx Outdated Show resolved Hide resolved
dapp/src/components/Overview/PositionDetails.tsx Outdated Show resolved Hide resolved
dapp/src/components/Overview/PositionDetails.tsx Outdated Show resolved Hide resolved
dapp/src/components/Overview/PositionDetails.tsx Outdated Show resolved Hide resolved
dapp/src/components/Modals/StakeModal.tsx Outdated Show resolved Hide resolved
dapp/src/components/Header/index.tsx Outdated Show resolved Hide resolved
dapp/src/components/DocsDrawer/index.tsx Outdated Show resolved Hide resolved
dapp/src/components/Overview/PositionDetails.tsx Outdated Show resolved Hide resolved
dapp/src/components/Overview/PositionDetails.tsx Outdated Show resolved Hide resolved
dapp/src/components/Sidebar/index.tsx Outdated Show resolved Hide resolved
dapp/src/components/Sidebar/index.tsx Outdated Show resolved Hide resolved
openSideBar()
} else {
closeSidebar()
timeout = setTimeout(resetState, 100)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need to reset state in timeout?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After closing the modal, we should reset the active step state. However, if we do this without timeout we will see the default state in the modal for one moment. See the video below.

Screen.Recording.2023-12-19.at.16.25.28.mov

dapp/src/components/Sidebar/index.tsx Outdated Show resolved Hide resolved
isOpen: boolean
onClose: () => void
numberOfSteps: number
// eslint-disable-next-line react/require-default-props
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Minor comment.
Let's remove this comment after the merge #74. This rule is disabled there.

acre/dapp/.eslintrc

Lines 10 to 18 in 4d01503

// TypeScript allows us to declare props that are non-optional internally
// but are interpreted as optional externally if they have defaultProps
// defined; the following two adjustments disable eslint-plugin-react
// checks that predate this ability for TS and that no longer apply.
"react/default-props-match-prop-types": [
2,
{ "allowRequiredDefaults": true }
],
"react/require-default-props": [0]

dapp/src/components/Modals/StakingOverviewModal.tsx Outdated Show resolved Hide resolved
dapp/src/components/Modals/StakeModal.tsx Outdated Show resolved Hide resolved
dapp/src/components/Overview/PositionDetails.tsx Outdated Show resolved Hide resolved
dapp/src/components/Overview/PositionDetails.tsx Outdated Show resolved Hide resolved
dapp/src/components/DocsDrawer/index.tsx Outdated Show resolved Hide resolved
dapp/src/components/Header/index.tsx Outdated Show resolved Hide resolved
openSideBar()
} else {
closeSidebar()
timeout = setTimeout(resetState, 100)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

After closing the modal, we should reset the active step state. However, if we do this without timeout we will see the default state in the modal for one moment. See the video below.

Screen.Recording.2023-12-19.at.16.25.28.mov

dapp/src/components/Overview/PositionDetails.tsx Outdated Show resolved Hide resolved
dapp/src/components/Overview/PositionDetails.tsx Outdated Show resolved Hide resolved
@r-czajkowski r-czajkowski merged commit b670566 into main Dec 20, 2023
9 checks passed
@r-czajkowski r-czajkowski deleted the handling-modal-window branch December 20, 2023 14:33
@nkuba nkuba mentioned this pull request Jan 11, 2024
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.

2 participants