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

fix: Adjust modal alignment to middle at breakpoint #821

Merged
merged 1 commit into from
Apr 23, 2024

Conversation

rfgamaral
Copy link
Member

@rfgamaral rfgamaral commented Apr 18, 2024

Short description

Some of our modals have enough content that when viewed at certain viewport heights, the content is cropped from view, hiding important elements (such as CTA buttons). This happens because we use a dynamic value for "padding" to optimize the placement more towards the top of the viewport, in a way to place modals more or less at eyesight level, while having enough room for dropdowns.

To work around this mechanism for certain modals when the viewport height is not enough to display all the content, this PR adjusts modal alignment to the middle of the screen when a certain breakpoint is reached.

Important

We have an ongoing discussion to decide whether this is the way forward or not, but I'll be OOO for more than a week, and won't be around to bring this to the finish line. However, I'm leaving this PR ready if the design team decides to go with this.

PR Checklist

  • Executed npm run validate and made sure no errors / warnings were shown
  • Described changes in CHANGELOG.md
  • Bumped version in package.json and package-lock.json (npm --no-git-tag-version version <major|minor|patch>) ref
  • Reviewed and approved Chromatic visual regression tests in CI

📸 Demo

firefox_e3afGnycSk.mp4

Versioning

Patch. A release would be nice to help with https://github.com/Doist/Issues/issues/13441.

@rfgamaral rfgamaral requested a review from a team April 18, 2024 16:19
@rfgamaral rfgamaral self-assigned this Apr 18, 2024
@rfgamaral rfgamaral requested review from pedroalves0 and removed request for a team April 18, 2024 16:19
@rfgamaral rfgamaral force-pushed the ricardo/dynamic-modal-alignment branch from 7015c1c to 6b710e2 Compare April 18, 2024 16:20
@pedroalves0 pedroalves0 merged commit 3aefefb into main Apr 23, 2024
5 checks passed
@pedroalves0 pedroalves0 deleted the ricardo/dynamic-modal-alignment branch April 23, 2024 15:46
nats12 added a commit that referenced this pull request May 31, 2024
* feat: Add 'medium' weight to Heading (#814)

* feat: Add 'medium' weight to Heading

* Update changelog

* Bump version

* fix: Adjust modal alignment to middle at breakpoint (#821)

* revert: Modal alignment to middle at breakpoint adjustment (#823)

ref: 3aefefb

* feat: Add warning tone for Badge (#826)

## Short description

Adds a new `warning` variant for the Badge component's `tone` option.


<img width="281" alt="Screenshot 2024-05-28 at 11 30 36" src="https://github.com/Doist/reactist/assets/15801768/b829b743-6356-4932-8b00-ab4a34d44769">


## PR Checklist

<!--
Feel free to leave unchecked or remove the lines that are not applicable.
-->

-   [x] Added tests for bugs / new features
-   [x] Updated docs (storybooks, readme)
-   [x] Executed `npm run validate` and made sure no errors / warnings were shown
-   [x] Described changes in `CHANGELOG.md`
-   [x] Bumped version in `package.json` and `package-lock.json` (`npm --no-git-tag-version version <major|minor|patch>`) [ref](https://docs.npmjs.com/cli/v6/commands/npm-version)
-   [x] Reviewed and approved Chromatic visual regression tests in CI

## Versioning

Minor

* chore: Bump up version to v23.3.0 (#827)

---------

Co-authored-by: Frankie Yan <[email protected]>
Co-authored-by: Ricardo Amaral <[email protected]>
Co-authored-by: Ernesto García <[email protected]>
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