Skip to content

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

Merged
FezVrasta merged 1 commit into
floating-ui:masterfrom
wszydlak:master
Apr 1, 2021
Merged

fix: fixing invalid containing block calculation on IE for fixed strategy#1268
FezVrasta merged 1 commit into
floating-ui:masterfrom
wszydlak:master

Conversation

@wszydlak

@wszydlak wszydlak commented Mar 29, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

rollingversions Bot commented Mar 29, 2021 •
edited
Loading

Copy link
Copy Markdown

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
Comment thread src/dom-utils/getOffsetParent.js Outdated
// return the containing block
function getContainingBlock(element: Element) {
const isFirefox = navigator.userAgent.toLowerCase().indexOf('firefox') !== -1;
const isIE11 = navigator.userAgent.indexOf('Trident') !== -1;

@FezVrasta FezVrasta Mar 29, 2021

Copy link
Copy Markdown
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?

@wszydlak wszydlak Mar 29, 2021

Copy link
Copy Markdown
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?

@FezVrasta FezVrasta Mar 29, 2021

Copy link
Copy Markdown
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.

@wszydlak wszydlak Mar 29, 2021

Copy link
Copy Markdown
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?

@wszydlak wszydlak Mar 30, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

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

@wszydlak wszydlak Mar 30, 2021

Copy link
Copy Markdown
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

@ptu14 ptu14 Mar 30, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor

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
Comment thread src/dom-utils/getOffsetParent.js Outdated
@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

wszydlak commented Mar 31, 2021

Copy link
Copy Markdown
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

@FezVrasta FezVrasta Awaiting requested review from FezVrasta
1 more reviewer
@ptu14 ptu14 ptu14 left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@wszydlak @FezVrasta @ptu14