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

feat(modal): viewport constrained fluid size #838

Merged
merged 4 commits into from
Nov 26, 2021

Conversation

varl
Copy link
Contributor

@varl varl commented Nov 10, 2021

Use case

There is a recurring need for larger Modals than our large variant (600px wide).

Potential solutions

  • height and width props that sets the size of the Modal
  • Override using className and !important
  • Unconstrained fluid Modal size that scrolls with the page
  • Constrained fluid Modal size that scrolls the ModalContent

Chosen solution

Constrained fluid Modal size that scrolls the ModalContent

Allowing the Modal to have a fluid prop allows the size of the Modal to be:

  • adaptable to the content of the ModalContent component
  • controlled by the app developer, by utilising an additional wrapper with defined sizes inside of the ModalContent

This provides enough flexibility without opening up the API too much.

The other solution's drawbacks that excluded them:

  • height/width props tied into ongoing discussion about overrides/extensions/global props
  • using className/!important is not recommended by us
  • scrolling the page because the Modal is very large is not an UX we want as it obscures the modality traits

Future

  • Consider changing the default from medium to fluid (breaking change)
  • If we want to allow page scroll, we need to open the Layer component to allow overflow.
  • The Card CSS can probably be simplified.

@dhis2-bot
Copy link
Contributor

dhis2-bot commented Nov 10, 2021

🚀 Deployed on https://pr-838--dhis2-ui.netlify.app

@cypress
Copy link

cypress bot commented Nov 10, 2021



Test summary

555 0 0 0


Run details

Project ui
Status Passed
Commit 14c3a73
Started Nov 26, 2021 9:45 AM
Ended Nov 26, 2021 9:55 AM
Duration 09:31 💡
OS Linux Ubuntu - 20.04
Browser Electron 89

View run in Cypress Dashboard ➡️


This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

@varl varl changed the base branch from LIBS-2-custom-modal-dimensions to master November 25, 2021 09:29
@varl varl force-pushed the viva-custom-modal-dimensions branch 5 times, most recently from 970c278 to 5063906 Compare November 25, 2021 11:36
@varl varl changed the title refactor(modal): allow constrained custom sizes feat(modal): viewport constrained fluid size Nov 25, 2021
@varl varl force-pushed the viva-custom-modal-dimensions branch from 5063906 to 4922669 Compare November 25, 2021 12:42
@varl varl requested review from a team and cooper-joe and removed request for cooper-joe November 25, 2021 12:43
HendrikThePendric and others added 2 commits November 26, 2021 10:14
The fluid modal is allowed to stretch to the entire viewport save for
some space around it to ensure that it is still perceived as a modal by
the user.

Within the ModalContent, a container (e.g. div or Box) can be placed
with either a specific size, or a dynamic size, and the Modal will
resize automatically to accomodate for the content.

If the content is small, the Modal will contract to fit it.
@dhis2-bot
Copy link
Contributor

dhis2-bot commented Nov 26, 2021

🚀 Deployed on https://pr-838--dhis2-ui.netlify.app

@dhis2-bot dhis2-bot temporarily deployed to netlify November 26, 2021 09:15 Inactive
@varl varl force-pushed the viva-custom-modal-dimensions branch from 0a44252 to 14c3a73 Compare November 26, 2021 09:35
@dhis2-bot dhis2-bot temporarily deployed to netlify November 26, 2021 09:40 Inactive
@varl varl merged commit 819301d into master Nov 26, 2021
@varl varl deleted the viva-custom-modal-dimensions branch November 26, 2021 10:07
@dhis2-bot
Copy link
Contributor

🎉 This PR is included in version 7.8.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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.

5 participants