Skip to content

Conversation

@adrienne-deriv
Copy link
Contributor

@adrienne-deriv adrienne-deriv commented Oct 2, 2023

Changes:

Please provide a summary of the change.

We have proposed a new change to the modal structure:

  • Refactored WalletModal to be a modal wrapper. A modal wrapper is just a skeleton/template that only contains styling for a certain modal template. For instance, some modals have header and footer like MT5AccountTypeModal and JurisdictionModal, while some modals only have a close icon and no header/footer, such as CreatePasswordModal or success modal. Modal wrappers like ModalStepWrapper are templates which have header/footer included, while ModalWrapper is just a plain modal with a close icon.
  • The component structure for a modal is proposed to be as the following:
const DeezModal = (props) => {
   ...
   return (
       <Wrapper ...>
            ...modal contents
       </Wrapper>
   )
}

where <Wrapper> can be a wrapper like ModalStepWrapper and ModalWrapper! The important thing to note is that wrappers should not control the behaviour of the modal, only styling of reusable templates.

  • Added full-screen responsive wrapper for ModalStepWrapper

Screenshots:

Please provide some screenshots of the change.

@vercel
Copy link

vercel bot commented Oct 2, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
deriv-app ✅ Ready (Inspect) Visit Preview Oct 2, 2023 7:26am

@adrienne-deriv adrienne-deriv marked this pull request as ready for review October 2, 2023 06:56
@github-actions
Copy link
Contributor

github-actions bot commented Oct 2, 2023

A production App ID was automatically generated for this PR. (log)

Click here to copy & paste above information.
- **PR**: [https://github.com/binary-com/deriv-app/pull/10411](https://github.com/binary-com/deriv-app/pull/10411)
- **URLs**:
    - **w/ App ID + Server**: https://deriv-app-git-fork-adrienne-deriv-wide-mobile-wrapper.binary.sx?qa_server=red.binaryws.com&app_id=32134
    - **Original**: https://deriv-app-git-fork-adrienne-deriv-wide-mobile-wrapper.binary.sx
- **App ID**: `32134`

@github-actions
Copy link
Contributor

github-actions bot commented Oct 2, 2023

🚨 Lighthouse report for the changes in this PR:

Category Score
🔺 Performance 21
🟧 Accessibility 75
🟧 Best practices 83
🟧 SEO 85
🟢 PWA 90

Lighthouse ran with https://deriv-app-git-fork-adrienne-deriv-wide-mobile-wrapper.binary.sx/

@sonarqubecloud
Copy link

sonarqubecloud bot commented Oct 2, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug D 1 Bug
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
10.5% 10.5% Duplication

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint

@coveralls
Copy link

Coverage Status

coverage: 11.24%. remained the same when pulling 05a5d08 on adrienne-deriv:wide-mobile-wrapper into ce6d645 on binary-com:master.

@balakrishna-deriv balakrishna-deriv merged commit 6affb6b into deriv-com:master Oct 2, 2023
vinu-deriv pushed a commit that referenced this pull request Oct 10, 2023
…l wrappers (#10411)

* feat: added wide wrapper for jurisdiction modal

* feat: added desktop view for jurisdiction modal

* chore: updated package-lock.json for wallets

* chore: removed package-lock.json in wallets

* chore: updated component typings based on comments

* chore: updated comments based on reviews

* chore: fixed issues with circleci for wallets

* chore: removed duplicated packages in wallets

* refactor: added responsive view for step wrapper

* chore: removed unused component
vinu-deriv pushed a commit that referenced this pull request Oct 10, 2023
…l wrappers (#10411)

* feat: added wide wrapper for jurisdiction modal

* feat: added desktop view for jurisdiction modal

* chore: updated package-lock.json for wallets

* chore: removed package-lock.json in wallets

* chore: updated component typings based on comments

* chore: updated comments based on reviews

* chore: fixed issues with circleci for wallets

* chore: removed duplicated packages in wallets

* refactor: added responsive view for step wrapper

* chore: removed unused component
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.

5 participants