Skip to content

fix: fixing invalid containing block calculation on IE for fixed strategy #1268

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Apr 1, 2021
Merged

fix: fixing invalid containing block calculation on IE for fixed strategy #1268

merged 1 commit into from
Apr 1, 2021
+10 −0

Conversation

wszydlak
Copy link
Contributor

@wszydlak wszydlak commented Mar 29, 2021 •
edited
Loading

According to CSS 2.1 (https://www.w3.org/TR/CSS21/visudet.html#containing-block-details) which is used by Internet Explorer for containing block calculation (https://docs.microsoft.com/en-us/previous-versions/windows/internet-explorer/ie-developer/platform-apis/aa768717(v=vs.85)) i prepared a fix which is obtaining correct containing block (same as IE will use for displaying content on page).

This fix will ensure that we use correct containing blocks for IE 9, 10 and 11.

I cannot paste codepen, cause it has some errors on IE.

However You can check new behavior using code below. I also added core-js to polyfill ES6 functions

<!DOCTYPE html>
<title>Transform Visual Test</title>

<style>
  #transform {
    position: fixed;
    transform: translate(200px, 200px);
  }
  
  #tooltip {
    background-color: #663399;
    padding: 20px;
    width: 200px;
  }
</style>

<div id="transform">
  <button type="button" id="button">I'm a button</button>
  <div id="tooltip">I'm a tooltip</div>
</div>

<script src="dist/umd/popper.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/core-js/3.9.1/minified.js"></script>
<script>
  const button = document.querySelector("#button");
  const tooltip = document.querySelector("#tooltip");

  // Pass the button, the tooltip, and some options, and Popper will do the
  // magic positioning for you:
  Popper.createPopper(button, tooltip, {
    placement: "bottom",
    strategy: "fixed"
  });
</script>

@rollingversions
Copy link

rollingversions bot commented Mar 29, 2021 •
edited
Loading

Change Log for @popperjs/core (2.9.1 → 2.9.2)

Bug Fixes

  • fixing invalid containing block calculation on IE for fixed strategy

Edit changelog

FezVrasta
FezVrasta reviewed Mar 29, 2021
View reviewed changes
src/dom-utils/getOffsetParent.js Outdated
@@ -22,6 +22,16 @@ function getTrueOffsetParent(element: Element): ?Element {
// return the containing block
function getContainingBlock(element: Element) {
const isFirefox = navigator.userAgent.toLowerCase().indexOf('firefox') !== -1;
const isIE11 = navigator.userAgent.indexOf('Trident') !== -1;
Copy link
Member

Choose a reason for hiding this comment

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

Isn't this more like a generic isIE?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Trident applies only for IE11, if we want to check older IE versions we need to have more if’s, do we need this?

Copy link
Member

Choose a reason for hiding this comment

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

No, I thought Trident was in all IE versions, thanks for the info.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So we could resolve this?

Copy link
Contributor Author

@wszydlak wszydlak Mar 30, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I checked again - you were right, Trident is more generic like isIE, it applies for IE 9, 10 and 11

Copy link
Contributor Author

Choose a reason for hiding this comment

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

According to this i pushed a change

Copy link
Contributor

@ptu14 ptu14 Mar 30, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I guess @FezVrasta is right about Trident and it can be isIE variable. I don't have old IE to test this but found something like this: https://stackoverflow.com/a/27069089

FezVrasta
FezVrasta reviewed Mar 30, 2021
View reviewed changes
fix: fixing invalid containing block calculation on IE for fixed stra…

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
3fcd81b 
…tegy
@wszydlak wszydlak changed the title fix: fixing invalid containing block calculation on IE11 for fixed strategy fix: fixing invalid containing block calculation on IE for fixed strategy Mar 30, 2021
@wszydlak wszydlak requested a review from FezVrasta March 30, 2021 11:20
@wszydlak
Copy link
Contributor Author

@FezVrasta are there any chances to get this checked and confirmed soon to be released as soon as possible?

@FezVrasta FezVrasta merged commit 4800b37 into floating-ui:master Apr 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ptu14 ptu14

@FezVrasta FezVrasta

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@wszydlak @FezVrasta @ptu14