Skip to content
Open
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
Comment thread
PaulGMardling marked this conversation as resolved.
"type": "minor",
"comment": "feat: add a hideBoundary positioning option for customizing escaped and reference-hidden detection",
"packageName": "@fluentui/react-positioning",
"email": "paulmardling@microsoft.com",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "fix: allow tooltips inside non-scrolling overflow:hidden containers to remain visible",
"packageName": "@fluentui/react-tooltip",
"email": "paulmardling@microsoft.com",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ export type PositioningImperativeRef = {
};

// @public
export interface PositioningProps extends Pick<PositioningOptions, 'align' | 'arrowPadding' | 'autoSize' | 'coverTarget' | 'fallbackPositions' | 'flipBoundary' | 'offset' | 'overflowBoundary' | 'overflowBoundaryPadding' | 'pinned' | 'position' | 'strategy' | 'useTransform' | 'matchTargetSize' | 'onPositioningEnd' | 'disableUpdateOnResize' | 'shiftToCoverTarget'> {
export interface PositioningProps extends Pick<PositioningOptions, 'align' | 'arrowPadding' | 'autoSize' | 'coverTarget' | 'fallbackPositions' | 'flipBoundary' | 'hideBoundary' | 'offset' | 'overflowBoundary' | 'overflowBoundaryPadding' | 'pinned' | 'position' | 'strategy' | 'useTransform' | 'matchTargetSize' | 'onPositioningEnd' | 'disableUpdateOnResize' | 'shiftToCoverTarget'> {
positioningRef?: React_2.Ref<PositioningImperativeRef>;
target?: TargetElement | null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ describe('PositioningProps', () => {
autoSize: 'always',
coverTarget: true,
flipBoundary: null,
hideBoundary: 'scrollParent',
offset: 0,
overflowBoundary: null,
overflowBoundaryPadding: 0,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,9 @@ export interface PositioningOptions {
/** The element which will define the boundaries of the positioned element for the overflow behavior. */
overflowBoundary?: PositioningBoundary | null;

/** The element which will define the boundaries for detecting whether the positioned element is hidden. */
hideBoundary?: PositioningBoundary | null;

/**
* Applies a padding to the overflow bounadry, so that overflow is detected earlier before the
* positioned surface hits the overflow boundary.
Expand Down Expand Up @@ -277,6 +280,7 @@ export interface PositioningProps
| 'coverTarget'
| 'fallbackPositions'
| 'flipBoundary'
| 'hideBoundary'
| 'offset'
| 'overflowBoundary'
| 'overflowBoundaryPadding'
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
'use client';

import { devtools } from '@floating-ui/devtools';
import { hide as hideMiddleware, arrow as arrowMiddleware } from '@floating-ui/dom';
import { arrow as arrowMiddleware, hide as hideMiddleware } from '@floating-ui/dom';
import type { Middleware, Placement, Strategy } from '@floating-ui/dom';
import { useFluent_unstable as useFluent } from '@fluentui/react-shared-contexts';
import * as React from 'react';
Expand All @@ -17,7 +17,7 @@ import {
matchTargetSize as matchTargetSizeMiddleware,
} from './middleware';
import type { PositioningConfigurationFn, PositioningConfigurationFnOptions, PositioningOptions } from './types';
import { toFloatingUIPlacement, hasScrollParent, normalizeAutoSize } from './utils';
import { getBoundary, toFloatingUIPlacement, hasScrollParent, normalizeAutoSize } from './utils';
import { devtoolsCallback } from './utils/devtools';
import { usePositioningConfiguration } from './PositioningConfigurationContext';

Expand All @@ -38,6 +38,7 @@ function usePositioningConfigFn(
coverTarget,
disableUpdateOnResize,
flipBoundary,
hideBoundary,
offset,
overflowBoundary,
pinned,
Expand Down Expand Up @@ -65,6 +66,7 @@ function usePositioningConfigFn(
strategy,
coverTarget,
flipBoundary,
hideBoundary,
overflowBoundary,
useTransform,
overflowBoundaryPadding,
Expand All @@ -87,6 +89,7 @@ function usePositioningConfigFn(
strategy,
coverTarget,
flipBoundary,
hideBoundary,
overflowBoundary,
useTransform,
overflowBoundaryPadding,
Expand Down Expand Up @@ -136,6 +139,7 @@ export function usePositioningOptions(options: PositioningOptions): (
offset,
coverTarget,
flipBoundary,
hideBoundary,
overflowBoundary,
useTransform,
overflowBoundaryPadding,
Expand All @@ -150,6 +154,8 @@ export function usePositioningOptions(options: PositioningOptions): (
unstable_disableTether,
} = optionsAfterEnhancement;
const normalizedAutoSize = normalizeAutoSize(autoSize);
const normalizedHideBoundary = getBoundary(container, hideBoundary ?? undefined);
const hideBoundaryOptions = normalizedHideBoundary ? { boundary: normalizedHideBoundary } : {};

const middleware = [
normalizedAutoSize && resetMaxSizeMiddleware(normalizedAutoSize),
Expand All @@ -170,8 +176,8 @@ export function usePositioningOptions(options: PositioningOptions): (
maxSizeMiddleware(normalizedAutoSize, { container, overflowBoundary, overflowBoundaryPadding, isRtl }),
intersectingMiddleware(),
arrow && arrowMiddleware({ element: arrow, padding: arrowPadding }),
hideMiddleware({ strategy: 'referenceHidden' }),
hideMiddleware({ strategy: 'escaped' }),
Comment thread
PaulGMardling marked this conversation as resolved.
hideMiddleware({ strategy: 'referenceHidden', ...hideBoundaryOptions }),
hideMiddleware({ strategy: 'escaped', ...hideBoundaryOptions }),
process.env.NODE_ENV !== 'production' &&
targetDocument &&
devtools(targetDocument, devtoolsCallback(optionsAfterEnhancement)),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
import { getBoundary } from './getBoundary';

describe('getBoundary', () => {
it('returns undefined when boundary is undefined', () => {
const element = document.createElement('div');

expect(getBoundary(element, undefined)).toBeUndefined();
});

it("returns the document element for 'window' boundary", () => {
const element = document.createElement('div');
document.body.appendChild(element);

expect(getBoundary(element, 'window')).toBe(document.documentElement);
});

it("returns 'clippingAncestors' for 'clippingParents' boundary", () => {
const element = document.createElement('div');

expect(getBoundary(element, 'clippingParents')).toBe('clippingAncestors');
});

it('returns the boundary itself when it is already a floating-ui boundary', () => {
const element = document.createElement('div');
const customBoundary = document.createElement('div');

expect(getBoundary(element, customBoundary)).toBe(customBoundary);
});

// Regression coverage for https://github.com/microsoft/fluentui/issues/36604
//
// Before the fix, the hide middleware always used 'clippingAncestors' as its boundary, which meant any static
// (non-scrolling) `overflow: hidden` ancestor was treated the same as a real scroll container, causing
// `referenceHidden` to report true even though nothing was actually scrolled out of view. `getBoundary` with
// `'scrollParent'` is what the fix now uses instead, and it must only stop at ancestors that can actually scroll.
describe("'scrollParent' boundary", () => {
it('skips a static overflow:hidden ancestor that cannot scroll, falling back to the document element', () => {
const staticHiddenContainer = document.createElement('div');
const trigger = document.createElement('button');

jest.spyOn(window, 'getComputedStyle').mockReturnValue({
overflow: 'hidden',
overflowX: '',
overflowY: '',
} as CSSStyleDeclaration);

staticHiddenContainer.appendChild(trigger);
document.body.appendChild(staticHiddenContainer);

expect(getBoundary(trigger, 'scrollParent')).toBe(document.documentElement);
});

it('resolves to the nearest real scroll parent, ignoring an intermediate static overflow:hidden container', () => {
const scrollableAncestor = document.createElement('div');
const staticHiddenContainer = document.createElement('div');
const trigger = document.createElement('button');

staticHiddenContainer.appendChild(trigger);
scrollableAncestor.appendChild(staticHiddenContainer);
document.body.appendChild(scrollableAncestor);

jest.spyOn(window, 'getComputedStyle').mockImplementation(
(node: Element) =>
({
overflow: node === scrollableAncestor ? 'scroll' : 'hidden',
overflowX: '',
overflowY: '',
} as CSSStyleDeclaration),
);

expect(getBoundary(trigger, 'scrollParent')).toBe(scrollableAncestor);
});

it('returns the document element when the resolved scroll parent is BODY', () => {
const trigger = document.createElement('button');
document.body.appendChild(trigger);

jest.spyOn(window, 'getComputedStyle').mockReturnValue({
overflow: 'visible',
overflowX: '',
overflowY: '',
} as CSSStyleDeclaration);

expect(getBoundary(trigger, 'scrollParent')).toBe(document.documentElement);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -44,4 +44,59 @@ describe('Tooltip', () => {
});
});
});

// Verifies the fix for regression reported in https://github.com/microsoft/fluentui/issues/36604
describe('static non-scrolling overflow:hidden container', () => {
it('still shows the tooltip when its trigger sits in a tightly-fitted overflow:hidden container', () => {
mount(
<div style={{ overflow: 'hidden', display: 'flex' }}>
<Tooltip content="I should still appear" relationship="label">
<Button id="trigger">Hover me</Button>
</Tooltip>
</div>,
);

cy.get('#trigger').realHover();

cy.get('[role="tooltip"]').should('be.visible').and('have.text', 'I should still appear');
});
});

// Verifies a static overflow:hidden wrapper nested inside a real scroll parent doesn't interfere with either
// the #36604 fix or the #32882 scroll-hide behavior.
describe('static overflow:hidden nested inside a scrollable ancestor', () => {
it('shows the tooltip while in view, then hides it once its trigger scrolls out of view', () => {
mount(
<div
id="scroll-container"
style={{
height: '100px',
width: '200px',
overflow: 'hidden scroll',
position: 'relative',
}}
>
<div style={{ height: '400px', paddingTop: '8px' }}>
<div style={{ overflow: 'hidden', display: 'flex' }}>
<Tooltip content="Nested tooltip" relationship="label">
<Button id="trigger">Hover me</Button>
</Tooltip>
</div>
</div>
</div>,
);

cy.get('#trigger').realHover();

cy.get('[role="tooltip"]')
.should('be.visible')
.then($tooltip => {
cy.get('#scroll-container').scrollTo(0, 300);
cy.wrap($tooltip).should('not.be.visible');

cy.get('#scroll-container').scrollTo(0, 0);
cy.wrap($tooltip).should('be.visible');
});
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
import * as React from 'react';
import { act, fireEvent, render } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import { Tooltip } from './Tooltip';
import { resetIdsForTests } from '@fluentui/react-utilities';

// Regression coverage for https://github.com/microsoft/fluentui/issues/36604.
//
// JSDOM doesn't run a real layout engine, so every element reports a zero-size
// `getBoundingClientRect` by default. Floating UI's `hide` middleware treats a
// zero-size reference/floating rect as fully clipped, which would make this test
// "pass" for the wrong reason (tooltip always hidden, regardless of the fix).
// To exercise the actual regression, we give the DOM nodes involved
// browser-realistic, non-zero rects that reproduce the reported layout:
// a trigger sitting in a tightly-fitted `overflow: hidden` wrapper, itself
// nested inside a real scrollable ancestor.
function mockRect(rect: Partial<DOMRect>): DOMRect {
return {
x: rect.left ?? 0,
y: rect.top ?? 0,
top: 0,
left: 0,
right: 0,
bottom: 0,
width: (rect.right ?? 0) - (rect.left ?? 0),
height: (rect.bottom ?? 0) - (rect.top ?? 0),
toJSON: () => ({}),
...rect,
} as DOMRect;
}

describe('Tooltip overflow:hidden regression (#36604)', () => {
let spies: jest.SpyInstance[];

afterEach(() => {
spies.forEach(spy => spy.mockRestore());
resetIdsForTests();
});

it('still shows the tooltip when its trigger sits in a tightly-fitted, static overflow:hidden container nested inside a real scroll parent', async () => {
const result = render(
<div
data-testid="scroll-ancestor"
style={{ height: '600px', width: '800px', overflow: 'auto', position: 'relative' }}
>
<div data-testid="static-hidden" style={{ overflow: 'hidden', display: 'flex' }}>
<Tooltip content="I should still appear" relationship="label" showDelay={0} hideDelay={0}>
<button data-testid="trigger">Hover me</button>
</Tooltip>
</div>
</div>,
);

const scrollAncestor = result.getByTestId('scroll-ancestor');
const staticHidden = result.getByTestId('static-hidden');
const trigger = result.getByTestId('trigger');

// A tight box around the trigger - the static `overflow: hidden` wrapper hugs it exactly,
// matching the "tightly-fitted toolbar/card" repro from the issue.
const triggerRect = { top: 300, left: 300, right: 380, bottom: 332 };
const scrollAncestorRect = { top: 0, left: 0, right: 800, bottom: 600 };
const viewportRect = { top: 0, left: 0, right: 1024, bottom: 768 };
const tooltipContentRect = { top: 0, left: 0, right: 100, bottom: 32 };

function rectFor(element: Element) {
if (element === scrollAncestor) {
return scrollAncestorRect;
}
if (element === staticHidden || element === trigger) {
return triggerRect;
}
if (element === document.documentElement || element === document.body) {
return viewportRect;
}
// The tooltip bubble itself (and anything else, e.g. the arrow): give it a modest,
// non-zero size so Floating UI's placement math has something realistic to work with.
return tooltipContentRect;
}

// JSDOM has no real layout engine: `getBoundingClientRect` always returns a zero rect, and
// `offsetWidth`/`offsetHeight` (which Floating UI's dimension measurement actually reads via
// `getCssDimensions`) are always 0 too. Both need mocking in lockstep, or Floating UI falls
// back to treating every element as zero-size regardless of the rects above.
spies = [
Comment thread
PaulGMardling marked this conversation as resolved.
Outdated
jest.spyOn(HTMLElement.prototype, 'getBoundingClientRect').mockImplementation(function (this: HTMLElement) {
return mockRect(rectFor(this));
}),
jest.spyOn(HTMLElement.prototype, 'offsetWidth', 'get').mockImplementation(function (this: HTMLElement) {
const rect = rectFor(this);
return rect.right - rect.left;
}),
jest.spyOn(HTMLElement.prototype, 'offsetHeight', 'get').mockImplementation(function (this: HTMLElement) {
const rect = rectFor(this);
return rect.bottom - rect.top;
}),
// Floating UI's viewport/clipping-ancestor rects (`getViewportRect`, `getInnerBoundingClientRect`) read
// `clientWidth`/`clientHeight`, not `getBoundingClientRect` - these need mocking too, or the resolved
// boundary (however correctly it resolves) collapses to a zero-size rect regardless.
jest.spyOn(HTMLElement.prototype, 'clientWidth', 'get').mockImplementation(function (this: HTMLElement) {
const rect = rectFor(this);
return rect.right - rect.left;
}),
jest.spyOn(HTMLElement.prototype, 'clientHeight', 'get').mockImplementation(function (this: HTMLElement) {
const rect = rectFor(this);
return rect.bottom - rect.top;
}),
];

await userEvent.hover(trigger);

// Let Floating UI's async `computePosition` (and our positioning-end event) settle.
await act(async () => {
await new Promise(resolve => setTimeout(resolve, 50));
});

const tooltip = result.baseElement.querySelector('[role="tooltip"]') as HTMLElement;
expect(tooltip).not.toBeNull();
expect(tooltip.textContent).toBe('I should still appear');
expect(getComputedStyle(tooltip).visibility).not.toBe('hidden');

await act(async () => {
fireEvent.pointerLeave(trigger);
await new Promise(resolve => setTimeout(resolve, 50));
});
});
});
Loading
Loading