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: remove animation fill mode for modals #1517

Merged
merged 1 commit into from
Feb 18, 2022
Merged

Conversation

miguelpeixe
Copy link
Member

All Submissions:

Changes proposed in this Pull Request:

This is a temporary fix until WordPress/gutenberg#38716 is not reviewed/released.

Fixes the positioning of Popovers inside Modals for Chrome and Safari.

With the animation fill mode set to forwards, the final step sets transform to a defined value. This makes position: fixed child elements position from the Modal's edge instead of the viewport.

More on position and containing blocks: https://developer.mozilla.org/en-US/docs/Web/CSS/Containing_block#identifying_the_containing_block

Master This PR
Captura de Tela 2022-02-16 às 14 25 24 image

How to test the changes in this Pull Request:

  1. Test a Modal with an inner Popover (available at feat(header-bidding): multiple gam orders newspack-ads#310 testing instructions)
  2. Confirm that on master it's misplaced (invisible due to overflow: hidden of Modal container)
  3. Check out this branch and confirm it's properly placed

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully ran tests with your changes locally?

@miguelpeixe miguelpeixe added the [Status] Needs Review The issue or pull request needs to be reviewed label Feb 16, 2022
@miguelpeixe miguelpeixe requested a review from a team as a code owner February 16, 2022 17:28
@miguelpeixe miguelpeixe self-assigned this Feb 16, 2022
Copy link
Contributor

@dkoo dkoo left a comment

Choose a reason for hiding this comment

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

👍

@github-actions github-actions bot added [Status] Approved The pull request has been reviewed and is ready to merge and removed [Status] Needs Review The issue or pull request needs to be reviewed labels Feb 18, 2022
@miguelpeixe miguelpeixe merged commit a5c0459 into master Feb 18, 2022
matticbot pushed a commit that referenced this pull request Feb 24, 2022
# [1.78.0-alpha.1](v1.77.1...v1.78.0-alpha.1) (2022-02-24)

### Bug Fixes

* initial theme mods setting ([#1500](#1500)) ([2d3de6b](2d3de6b)), closes [#1093](#1093)
* remove animation fill mode for modals ([#1517](#1517)) ([a5c0459](a5c0459))
* **setup-wizard:** hide navigation on welcome screen ([a6fad4e](a6fad4e))

### Features

* **ad-units:** move edit/archive links to popover ([#1505](#1505)) ([5177fe6](5177fe6))
* **stripe:** donate flow; location code ([#1483](#1483)) ([8cd28f9](8cd28f9))
@matticbot
Copy link
Contributor

🎉 This PR is included in version 1.78.0-alpha.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

matticbot pushed a commit that referenced this pull request Mar 8, 2022
# [1.78.0](v1.77.3...v1.78.0) (2022-03-08)

### Bug Fixes

* initial theme mods setting ([#1500](#1500)) ([2d3de6b](2d3de6b)), closes [#1093](#1093)
* remove animation fill mode for modals ([#1517](#1517)) ([a5c0459](a5c0459))
* **setup-wizard:** hide navigation on welcome screen ([a6fad4e](a6fad4e))
* stripe data setting ([ceec544](ceec544))

### Features

* **ad-units:** move edit/archive links to popover ([#1505](#1505)) ([5177fe6](5177fe6))
* **stripe:** donate flow; location code ([#1483](#1483)) ([8cd28f9](8cd28f9))
@matticbot
Copy link
Contributor

🎉 This PR is included in version 1.78.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@miguelpeixe miguelpeixe deleted the fix/modal-popover branch September 28, 2022 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released on @alpha released [Status] Approved The pull request has been reviewed and is ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants