-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
[DOM] Support for :top-layer content, e.g. <dialog> and [popup] #1842
Comments
+1 on this, I was hoping to use floating-ui in combination with the native |
I'm testing out this fairly naive middleware, based on the import { Middleware, MiddlewareArguments } from "@floating-ui/dom";
export const topLayer = (): Middleware => ({
name: 'topLayer',
async fn(middlewareArguments: MiddlewareArguments) {
const { x, y, elements: { reference, floating } } = middlewareArguments;
let onTopLayer = false;
const diffCoords = {
x: 0,
y: 0,
}
// browsers will throw when they do not support the following selectors, catch the errors.
try {
onTopLayer = onTopLayer || floating.matches(':open');
// eslint-disable-next-line no-empty
} catch (e) {}
try {
onTopLayer = onTopLayer || floating.matches(':modal');
// eslint-disable-next-line no-empty
} catch (e) {}
if (onTopLayer) {
// :top-layer content will be positioned from `window` get the reference's position from there, too.
const rect = reference.getBoundingClientRect()
diffCoords.x = rect.x;
diffCoords.y = rect.y;
}
return {
x: x + diffCoords.x,
y: y + diffCoords.y,
data: diffCoords,
};
},
}); This feels like a situation that will come up more and more as these specs solidify and expect. It feels like this should be core library functionality, or at least a core library middleware. |
That looks surprisingly small and simple so it should be straightforward to support in the core I'm confused why is this line necessary to make it work? div {
transform: translateZ(0px);
} |
Oh, this is what it's specifically correcting for but I didn't test that as a gate of the middleware. I'll look closer on that, needing style rules on the trigger will be another good reason to push this into the main lib, however. |
@Westbrook are you up to make a PR with that CodeSandbox you made? Seems good |
Can do! My team and I are specifically looking into all of this right now, so hopefully I can get your something this week. |
@Westbrook did you make any progress on this? I've been prototyping with the I created a simple replication here (requires Chrome Canary with experimental web features enabled at time of writing): I included your test middleware in that example as well (commented out by default), and while it's not complete of course, I was able to adjust it to give me the proper positioning values in this scenario as well. With that being said, It seems like for any elements in the top-layer that we could bypass the containing block logic completely. I know |
I've continued to make progress in my local implementation, however the testing harness and deep requirement for test coverage for this feature tend to turn me away every time I get close to making a contribution on this. Here's where I've got to to date: https://codesandbox.io/s/floating-ui-dom-template-forked-fkk8yj?file=/src/topLayerOverTransforms.js My work to consume this is coming to a close to an end, at which point I'll no longer have an excuse to not dig into the React code underlying the test harness and get coverage ready for this feature so I can submit a PR. |
Thanks for the update! I also started digging into it a bit, I'll see what else I can find out. Additionally, I adjusted my codepen example with your latest middleware changes, and also added in the |
Any updates on this? The Popover API is scheduled for launch in Chrome 114. Thanks for all the hard work! 🙏 |
I was able to adapt the original solution that @Westbrook posted above to work for me with the I'm curious if this also works for others? If so, it's fairly simple, and as @Westbrook mentioned it is likely something that should be built into the core functionality. The gist of the changes I made were to apply the following middleware that now accounts for the offset of the reference element in the coordinate calculation after all other middleware: const topLayerOverTransforms = () => ({
name: 'topLayer',
async fn({ x, y, elements: { reference, floating }}) {
let onTopLayer = false;
const diffCoords = { x: 0, y: 0 };
// browsers will throw when they do not support the following selectors, catch the errors.
try {
onTopLayer = onTopLayer || floating.matches(':modal') || floating.matches(':popover-open');
} catch (e) {}
if (!onTopLayer) {
return { x, y, data: diffCoords };
}
const containingBlock = getContainingBlock(reference);
const inContainingBlock = containingBlock && !isWindow(containingBlock);
if (onTopLayer && inContainingBlock) {
const rect = reference.getBoundingClientRect();
diffCoords.x = Math.trunc(rect.x - reference.offsetLeft);
diffCoords.y = Math.trunc(rect.y - reference.offsetTop);
}
return {
x: x + diffCoords.x,
y: y + diffCoords.y,
data: diffCoords
};
}
}); |
I keep getting caught up in the complexities of the testing surface but still intend to make a PR for this. A couple of previous contributions I've made had skipped on tests and ended up getting regressed in future releases, so I know I can't go without them at the complexity of this addition. However, I'd be happy for anyone else to skip the line, if they had time to do it sooner. My current version includes a lot of edge case coverage for my extensive use of shadow DOM and the complexities that come along with that. It looks like the following, but without tests and what not, it'll be hard to guarantee anything here. export const topLayerOverTransforms = () => ({
name: 'topLayer',
async fn(middlewareArguments: MiddlewareArguments) {
const {
x,
y,
elements: { reference, floating },
} = middlewareArguments;
let onTopLayer = false;
let topLayerIsFloating = false;
const diffCoords = {
x: 0,
y: 0,
};
try {
onTopLayer = onTopLayer || floating.matches(':popover-open');
// eslint-disable-next-line no-empty
} catch (e) {}
try {
onTopLayer = onTopLayer || floating.matches(':open');
// eslint-disable-next-line no-empty
} catch (e) {}
try {
onTopLayer = onTopLayer || floating.matches(':modal');
// eslint-disable-next-line no-empty
} catch (e) {}
topLayerIsFloating = onTopLayer;
if (!onTopLayer) {
const dialogAncestorQueryEvent = new Event(
'floating-ui-dialog-test',
{ composed: true, bubbles: true }
);
floating.addEventListener(
'floating-ui-dialog-test',
(event: Event) => {
(event.composedPath() as unknown as Element[]).forEach(
(el) => {
if (el === floating || el.localName !== 'dialog')
return;
try {
onTopLayer = onTopLayer || el.matches(':modal');
if (onTopLayer) {
// console.log(el);
}
// eslint-disable-next-line no-empty
} catch (e) {}
}
);
},
{ once: true }
);
floating.dispatchEvent(dialogAncestorQueryEvent);
}
let overTransforms = false;
const containingBlock = getContainingBlock(reference as Element);
if (containingBlock !== null && !isWindow(containingBlock)) {
overTransforms = true;
}
if (onTopLayer && overTransforms) {
const rect = containingBlock!.getBoundingClientRect();
diffCoords.x = rect.x;
diffCoords.y = rect.y;
}
if (onTopLayer && topLayerIsFloating) {
return {
x: x + diffCoords.x,
y: y + diffCoords.y,
data: diffCoords,
};
}
if (onTopLayer) {
return {
x,
y,
data: diffCoords,
};
}
return {
x: x - diffCoords.x,
y: y - diffCoords.y,
data: diffCoords,
};
},
}); |
Wanted to make sure everyone interested saw #2351... the further I get into this the more I wonder "is this a middleware, or is it just a thing that should be a part of the library?", but it's hard to say. Some more work for testing upcoming... |
As someone who's now starting to build popover components on top of the newly released popover feature in chrome, it makes sense to me that this should just be built into the library. re transform I think it's not uncommon to use transform: translateY(-50%) for positioning that takes the size of the element into account. |
|
The workaround @DRiFTy17 made works really nicely (for now 😉 ) but are there still plans to include this fix in future versions of Floating-ui? I noticed that the same logic that is applied in the workaround needs to be applied to the positioning of the arrow. There it uses a |
For those that are interested, my version of this plugin looks as follows with the new exports: import type { Middleware, MiddlewareState } from '@floating-ui/dom';
import {
getContainingBlock,
getWindow,
isContainingBlock,
} from '@floating-ui/utils/dom';
import { VirtualTrigger } from './VirtualTrigger.js';
export const topLayerOverTransforms = (): Middleware => ({
name: 'topLayer',
async fn(middlewareArguments: MiddlewareState) {
const {
x,
y,
elements: { reference, floating },
} = middlewareArguments;
let onTopLayer = false;
let topLayerIsFloating = false;
let withinReference = false;
const diffCoords = {
x: 0,
y: 0,
};
try {
onTopLayer = onTopLayer || floating.matches(':popover-open');
// eslint-disable-next-line no-empty
} catch (error) {}
try {
onTopLayer = onTopLayer || floating.matches(':open');
// eslint-disable-next-line no-empty
} catch (error) {}
try {
onTopLayer = onTopLayer || floating.matches(':modal');
// eslint-disable-next-line no-empty
/* c8 ignore next 3 */
} catch (error) {}
topLayerIsFloating = onTopLayer;
const dialogAncestorQueryEvent = new Event('floating-ui-dialog-test', {
composed: true,
bubbles: true,
});
floating.addEventListener(
'floating-ui-dialog-test',
(event: Event) => {
(event.composedPath() as unknown as Element[]).forEach((el) => {
withinReference = withinReference || el === reference;
if (el === floating || el.localName !== 'dialog') return;
try {
onTopLayer = onTopLayer || el.matches(':modal');
// eslint-disable-next-line no-empty
/* c8 ignore next */
} catch (error) {}
});
},
{ once: true }
);
floating.dispatchEvent(dialogAncestorQueryEvent);
let overTransforms = false;
if (!(reference instanceof VirtualTrigger)) {
const root = (withinReference ? reference : floating) as Element;
const containingBlock = isContainingBlock(root)
? root
: getContainingBlock(root);
if (
containingBlock !== null &&
getWindow(containingBlock) !==
(containingBlock as unknown as Window)
) {
const css = getComputedStyle(containingBlock);
overTransforms =
css.transform !== 'none' || css.filter
? css.filter !== 'none'
: false;
}
if (onTopLayer && overTransforms && containingBlock) {
const rect = containingBlock.getBoundingClientRect();
diffCoords.x = rect.x;
diffCoords.y = rect.y;
}
}
if (onTopLayer && topLayerIsFloating) {
return {
x: x + diffCoords.x,
y: y + diffCoords.y,
data: diffCoords,
};
}
if (onTopLayer) {
return {
x,
y,
data: diffCoords,
};
}
return {
x: x - diffCoords.x,
y: y - diffCoords.y,
data: diffCoords,
};
},
}); There's definitely some edge cases around Some of the |
I am using popover without transforms and it still doesn't work. It seems if triggering button is inside the parent which is containing block positioning completely breaks. Reproduction. |
There hasn't been a fix/solution added to the package yet. @Westbrook's comment above is the current solution, and as you can see requires some hacks to cover various cases. |
Proposed solution in that comment did not work for me :/ |
This is our current version of the plugin in Typescript: import type { Middleware, MiddlewareState } from '@floating-ui/dom';
import {
getContainingBlock,
getWindow,
isContainingBlock,
} from '@floating-ui/utils/dom';
import { VirtualTrigger } from './VirtualTrigger.js';
export const topLayerOverTransforms = (): Middleware => ({
name: 'topLayer',
async fn(middlewareArguments: MiddlewareState) {
const {
x,
y,
elements: { reference, floating },
} = middlewareArguments;
let onTopLayer = false;
let topLayerIsFloating = false;
let withinReference = false;
const diffCoords = {
x: 0,
y: 0,
};
try {
onTopLayer = onTopLayer || floating.matches(':popover-open');
// eslint-disable-next-line no-empty
} catch (error) {}
try {
onTopLayer = onTopLayer || floating.matches(':open');
// eslint-disable-next-line no-empty
} catch (error) {}
try {
onTopLayer = onTopLayer || floating.matches(':modal');
// eslint-disable-next-line no-empty
/* c8 ignore next 3 */
} catch (error) {}
topLayerIsFloating = onTopLayer;
const dialogAncestorQueryEvent = new Event('floating-ui-dialog-test', {
composed: true,
bubbles: true,
});
floating.addEventListener(
'floating-ui-dialog-test',
(event: Event) => {
(event.composedPath() as unknown as Element[]).forEach((el) => {
withinReference = withinReference || el === reference;
if (el === floating || el.localName !== 'dialog') return;
try {
onTopLayer = onTopLayer || el.matches(':modal');
// eslint-disable-next-line no-empty
/* c8 ignore next */
} catch (error) {}
});
},
{ once: true }
);
floating.dispatchEvent(dialogAncestorQueryEvent);
let overTransforms = false;
if (!(reference instanceof VirtualTrigger)) {
const root = (withinReference ? reference : floating) as Element;
const containingBlock = isContainingBlock(root)
? root
: getContainingBlock(root);
let css: CSSStyleDeclaration | Record<string, string> = {};
if (
containingBlock !== null &&
getWindow(containingBlock) !==
(containingBlock as unknown as Window)
) {
css = getComputedStyle(containingBlock);
// The overlay is "over transforms" when the containing block uses specific CSS...
overTransforms =
// the `transform` property
css.transform !== 'none' ||
// the `translate` property
css.translate !== 'none' ||
// the `containerType` property
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
(css.containerType
? // eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
css.containerType !== 'none'
: false) ||
// the `backdropFilter` property
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
(css.backdropFilter
? // eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
css.backdropFilter !== 'none'
: false) ||
// the `filter` property for anything other than "none"
(css.filter ? css.filter !== 'none' : false) ||
// the `transform` property "will-change"
css.willChange.search('transform') > -1 ||
// the `transform` property "will-change"
css.willChange.search('translate') > -1 ||
// a value of "paint", "layout", "strict", or "content" for `contain`
['paint', 'layout', 'strict', 'content'].some((value) =>
(css.contain || '').includes(value)
);
}
if (onTopLayer && overTransforms && containingBlock) {
const rect = containingBlock.getBoundingClientRect();
// Margins are not included in the bounding client rect and need to be handled separately.
const { marginInlineStart = '0', marginBlockStart = '0' } = css;
diffCoords.x = rect.x + parseFloat(marginInlineStart);
diffCoords.y = rect.y + parseFloat(marginBlockStart);
}
}
if (onTopLayer && topLayerIsFloating) {
return {
x: x + diffCoords.x,
y: y + diffCoords.y,
data: diffCoords,
};
}
if (onTopLayer) {
return {
x,
y,
data: diffCoords,
};
}
return {
x: x - diffCoords.x,
y: y - diffCoords.y,
data: diffCoords,
};
},
}); It works for our use cases in https://opensource.adobe.com/spectrum-web-components/, but there's always a new CSS property to add to the test buffer 🙃. It would be nice if a solution to this was part of the main library as the expansion of |
@Westbrook, thank you for your efforts towards this solution. I was wondering what Many thanks. |
Practically, it’s this https://github.com/adobe/spectrum-web-components/blob/main/packages/overlay/src/VirtualTrigger.ts However, if you never use virtual elements then you could omit that gating. |
Changed this issue to a bug. @Westbrook what's the status of this PR: #2351? Since this has a high number of 👍 votes, we should try to get this merged in |
I can look to get it cleaned up and go from there. However, if this is a "bug" and with the larger amount of interaction with the overall element stack, I wonder if it shouldn't be part of the default calculations rather than a plugin. |
Ideally it would be part of the default calculations indeed, however the amount of code it takes to get working is a bit high to be a default in terms of bundle size impact for those not using it. At the very least, I think it's a bug in terms of needing to support this increasingly-used web platform feature, even if it requires a middleware be manually added to work. |
working on a web component, should i wait for this too merge or should i go for the workaround? also could not find how to use the workaround, do you have an example somewhere? |
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [@floating-ui/dom](https://floating-ui.com) ([source](https://togithub.com/floating-ui/floating-ui/tree/HEAD/packages/dom)) | [`1.5.4` -> `1.6.0`](https://renovatebot.com/diffs/npm/@floating-ui%2fdom/1.5.4/1.6.0) | [![age](https://developer.mend.io/api/mc/badges/age/npm/@floating-ui%2fdom/1.6.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/@floating-ui%2fdom/1.6.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/@floating-ui%2fdom/1.5.4/1.6.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/@floating-ui%2fdom/1.5.4/1.6.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>floating-ui/floating-ui (@​floating-ui/dom)</summary> ### [`v1.6.0`](https://togithub.com/floating-ui/floating-ui/blob/HEAD/packages/dom/CHANGELOG.md#160) [Compare Source](https://togithub.com/floating-ui/floating-ui/compare/@floating-ui/dom@1.5.4...@floating-ui/dom@1.6.0) ##### Minor Changes - fix: handle CSS `:top-layer` elements inside containing blocks. It's no longer necessary to implement the middleware workaround outlined[floating-ui/floating-ui#1842 (comment). ##### Patch Changes - Update dependencies: `@floating-ui/core@1.6.0` </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/ariakit/ariakit). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4xMzUuMCIsInVwZGF0ZWRJblZlciI6IjM3LjEzNS4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [@floating-ui/dom](https://floating-ui.com) ([source](https://togithub.com/floating-ui/floating-ui/tree/HEAD/packages/dom)) | [`1.5.3` -> `1.6.3`](https://renovatebot.com/diffs/npm/@floating-ui%2fdom/1.5.3/1.6.3) | [![age](https://developer.mend.io/api/mc/badges/age/npm/@floating-ui%2fdom/1.6.3?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/@floating-ui%2fdom/1.6.3?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/@floating-ui%2fdom/1.5.3/1.6.3?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/@floating-ui%2fdom/1.5.3/1.6.3?slim=true)](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>floating-ui/floating-ui (@​floating-ui/dom)</summary> ### [`v1.6.3`](https://togithub.com/floating-ui/floating-ui/blob/HEAD/packages/dom/CHANGELOG.md#163) [Compare Source](https://togithub.com/floating-ui/floating-ui/compare/@floating-ui/dom@1.6.2...@floating-ui/dom@1.6.3) ##### Patch Changes - fix: calculate reference element offset relative to `offsetParent` iframe. Fixes issue with positioning in nested iframes, such as the following: ```html <html> <iframe> <div>floating</div> <iframe> <div>reference</div> </iframe> </iframe> </html> ``` ### [`v1.6.2`](https://togithub.com/floating-ui/floating-ui/blob/HEAD/packages/dom/CHANGELOG.md#162) [Compare Source](https://togithub.com/floating-ui/floating-ui/compare/@floating-ui/dom@1.6.1...@floating-ui/dom@1.6.2) ##### Patch Changes - fix: top layer element positioning and collision detection when using `absolute` strategy ### [`v1.6.1`](https://togithub.com/floating-ui/floating-ui/blob/HEAD/packages/dom/CHANGELOG.md#161) [Compare Source](https://togithub.com/floating-ui/floating-ui/compare/@floating-ui/dom@1.6.0...@floating-ui/dom@1.6.1) ##### Patch Changes - perf: avoid `getContainingBlock` call for non-top layer elements ### [`v1.6.0`](https://togithub.com/floating-ui/floating-ui/blob/HEAD/packages/dom/CHANGELOG.md#160) [Compare Source](https://togithub.com/floating-ui/floating-ui/compare/@floating-ui/dom@1.5.4...@floating-ui/dom@1.6.0) ##### Minor Changes - fix: handle CSS `:top-layer` elements inside containing blocks. It's no longer necessary to implement the middleware workaround outlined[floating-ui/floating-ui#1842 (comment). ##### Patch Changes - Update dependencies: `@floating-ui/core@1.6.0` ### [`v1.5.4`](https://togithub.com/floating-ui/floating-ui/blob/HEAD/packages/dom/CHANGELOG.md#154) [Compare Source](https://togithub.com/floating-ui/floating-ui/compare/@floating-ui/dom@1.5.3...@floating-ui/dom@1.5.4) ##### Patch Changes - [`4c04669`](https://togithub.com/floating-ui/floating-ui/commit/4c04669): chore: exports .d.mts types, solves [#​2472](https://togithub.com/floating-ui/floating-ui/issues/2472) - [`0d18e37`](https://togithub.com/floating-ui/floating-ui/commit/0d18e37): refactor: avoid $ appearing in rects dimensions - Updated dependencies \[[`4c04669`](https://togithub.com/floating-ui/floating-ui/commit/4c04669)] - Updated dependencies \[[`afb7e5e`](https://togithub.com/floating-ui/floating-ui/commit/afb7e5e)] - [@​floating-ui/utils](https://togithub.com/floating-ui/utils)[@​0](https://togithub.com/0).2.0 - [@​floating-ui/core](https://togithub.com/floating-ui/core)[@​1](https://togithub.com/1).5.3 </details> --- ### Configuration 📅 **Schedule**: Branch creation - "before 5am every weekday" in timezone Europe/Berlin, Automerge - "after 10pm every weekday,before 5am every weekday" in timezone Europe/Berlin. 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Never, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/solid-design-system/solid). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4yMjcuMiIsInVwZGF0ZWRJblZlciI6IjM3LjIyNy4yIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Content in a
<dialog>
that is opened withshowModal()
orshowPopUp()
via thepopup
attribute will not actually have "offsetParents" or "clippingAncestors". How do you think it is best for the library to support positioning that content?Particularly, if a reference element and a floating element are both bound in an element with a matrix transform, a la https://codesandbox.io/s/floating-ui-dom-template-forked-es3dfl?file=/src/index.js, do you foresee adding extra checks for this reality, or surfacing a flag so a use can opt-out of ancestor math or something else.
The text was updated successfully, but these errors were encountered: