Skip to content

Commit

Permalink
fix: Hide disabled reason tooltip when the option is scrolled away (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
jperals authored Oct 1, 2024
1 parent 1686229 commit a2e7df7
Show file tree
Hide file tree
Showing 14 changed files with 413 additions and 156 deletions.
10 changes: 10 additions & 0 deletions pages/multiselect/disabled-reason.page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,14 @@ import Multiselect, { MultiselectProps } from '~components/multiselect';

import ScreenshotArea from '../utils/screenshot-area';

const extraOptions = [...Array(30).keys()].map(n => {
const numberToDisplay = (n + 5).toString();
return {
value: numberToDisplay,
label: `Option ${n + 5}`,
};
});

const options: MultiselectProps.Options = [
{ value: 'first', label: 'Simple' },
{ value: 'second', label: 'With small icon', iconName: 'folder' },
Expand All @@ -24,6 +32,8 @@ const options: MultiselectProps.Options = [
disabled: true,
disabledReason: 'disabled reason',
},
...extraOptions,
{ label: 'Last option', disabled: true, disabledReason: 'disabled reason' },
];

export default function MultiselectPage() {
Expand Down
23 changes: 11 additions & 12 deletions pages/select/disabled-reason.page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,17 @@ import Select from '~components/select';

import ScreenshotArea from '../utils/screenshot-area';

const options = [
{
value: '1',
label: 'Option 1',
disabled: true,
disabledReason: 'disabled reason',
},
{
value: '2',
label: 'Option 2',
},
];
const options = [...Array(50).keys()].map(n => {
const numberToDisplay = (n + 1).toString();
const baseOption = {
value: numberToDisplay,
label: `Option ${numberToDisplay}`,
};
if (n === 0 || n === 24 || n === 49) {
return { ...baseOption, disabled: true, disabledReason: 'disabled reason' };
}
return baseOption;
});

export default function SelectPage() {
return (
Expand Down
3 changes: 3 additions & 0 deletions src/internal/components/tooltip/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export interface TooltipProps {
className?: string;
contentAttributes?: React.HTMLAttributes<HTMLDivElement>;
size?: PopoverProps['size'];
hideOnOverscroll?: boolean;
}

export default function Tooltip({
Expand All @@ -30,6 +31,7 @@ export default function Tooltip({
contentAttributes = {},
position = 'top',
size = 'small',
hideOnOverscroll,
}: TooltipProps) {
if (!trackKey && (typeof value === 'string' || typeof value === 'number')) {
trackKey = value;
Expand All @@ -48,6 +50,7 @@ export default function Tooltip({
position={position}
zIndex={7000}
arrow={position => <PopoverArrow position={position} />}
hideOnOverscroll={hideOnOverscroll}
>
<PopoverBody dismissButton={false} dismissAriaLabel={undefined} onDismiss={undefined} header={undefined}>
{value}
Expand Down
60 changes: 60 additions & 0 deletions src/popover/__tests__/positions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import {
calculatePosition,
intersectRectangles,
isCenterOutside,
PRIORITY_MAPPING,
} from '../../../lib/components/popover/utils/positions';

Expand Down Expand Up @@ -199,3 +200,62 @@ describe('intersectRectangles', () => {
expect(intersectRectangles([])).toEqual(null);
});
});

describe('isCenterOutside', () => {
const parentRect = {
blockSize: 50,
inlineSize: 10,
insetBlockStart: 15,
insetBlockEnd: 65,
insetInlineStart: 0,
insetInlineEnd: 10,
};

test('returns true if the block-level center of the first rect is smaller than the block-level start of the second rect', () => {
expect(
isCenterOutside(
{
blockSize: 20,
inlineSize: 10,
insetBlockStart: 0,
insetBlockEnd: 20,
insetInlineStart: 0,
insetInlineEnd: 10,
},
parentRect
)
).toBe(true);
});

test('returns true if the block-level center of the first rect is bigger than the block-level start of the second rect', () => {
expect(
isCenterOutside(
{
blockSize: 20,
inlineSize: 10,
insetBlockStart: 60,
insetBlockEnd: 80,
insetInlineStart: 0,
insetInlineEnd: 10,
},
parentRect
)
).toBe(true);
});

test('returns false if the block-level center of the first rect is between the block-level start and the block-level end of the second rect', () => {
expect(
isCenterOutside(
{
blockSize: 20,
inlineSize: 10,
insetBlockStart: 10,
insetBlockEnd: 30,
insetInlineStart: 0,
insetInlineEnd: 10,
},
parentRect
)
).toBe(false);
});
});
33 changes: 19 additions & 14 deletions src/popover/container.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ export interface PopoverContainerProps {
// Do not use this if the popover is open on hover, in order to avoid unexpected movement.
allowScrollToFit?: boolean;
allowVerticalOverflow?: boolean;
// Whether the popover should be hidden when the trigger is scrolled away.
hideOnOverscroll?: boolean;
}

export default function PopoverContainer({
Expand All @@ -55,6 +57,7 @@ export default function PopoverContainer({
keepPosition,
allowScrollToFit,
allowVerticalOverflow,
hideOnOverscroll,
}: PopoverContainerProps) {
const bodyRef = useRef<HTMLDivElement | null>(null);
const contentRef = useRef<HTMLDivElement | null>(null);
Expand All @@ -64,18 +67,20 @@ export default function PopoverContainer({
const isRefresh = useVisualRefresh();

// Updates the position handler.
const { updatePositionHandler, popoverStyle, internalPosition, positionHandlerRef } = usePopoverPosition({
popoverRef,
bodyRef,
arrowRef,
trackRef,
contentRef,
allowScrollToFit,
allowVerticalOverflow,
preferredPosition: position,
renderWithPortal,
keepPosition,
});
const { updatePositionHandler, popoverStyle, internalPosition, positionHandlerRef, isOverscrolling } =
usePopoverPosition({
popoverRef,
bodyRef,
arrowRef,
trackRef,
contentRef,
allowScrollToFit,
allowVerticalOverflow,
preferredPosition: position,
renderWithPortal,
keepPosition,
hideOnOverscroll,
});

// Recalculate position when properties change.
useLayoutEffect(() => {
Expand Down Expand Up @@ -124,9 +129,9 @@ export default function PopoverContainer({
window.removeEventListener('resize', updatePositionOnResize);
window.removeEventListener('scroll', refreshPosition, true);
};
}, [keepPosition, positionHandlerRef, trackRef, updatePositionHandler]);
}, [hideOnOverscroll, keepPosition, positionHandlerRef, trackRef, updatePositionHandler]);

return (
return isOverscrolling ? null : (
<div
ref={popoverRef}
style={{ ...popoverStyle, zIndex }}
Expand Down
5 changes: 5 additions & 0 deletions src/popover/interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,11 @@ export interface Dimensions {

export type BoundingBox = Dimensions & Offset;

export type Rect = BoundingBox & {
insetBlockEnd: number;
insetInlineEnd: number;
};

export namespace PopoverProps {
export type Position = 'top' | 'right' | 'bottom' | 'left';
export type Size = 'small' | 'medium' | 'large';
Expand Down
33 changes: 28 additions & 5 deletions src/popover/use-popover-position.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ import {
getFirstScrollableParent,
scrollRectangleIntoView,
} from '../internal/utils/scrollable-containers';
import { BoundingBox, InternalPosition, Offset, PopoverProps } from './interfaces';
import { calculatePosition, getDimensions, getOffsetDimensions } from './utils/positions';
import { BoundingBox, InternalPosition, Offset, PopoverProps, Rect } from './interfaces';
import { calculatePosition, getDimensions, getOffsetDimensions, isCenterOutside } from './utils/positions';

export default function usePopoverPosition({
popoverRef,
Expand All @@ -26,6 +26,7 @@ export default function usePopoverPosition({
preferredPosition,
renderWithPortal,
keepPosition,
hideOnOverscroll,
}: {
popoverRef: React.RefObject<HTMLDivElement | null>;
bodyRef: React.RefObject<HTMLDivElement | null>;
Expand All @@ -37,14 +38,18 @@ export default function usePopoverPosition({
preferredPosition: PopoverProps.Position;
renderWithPortal?: boolean;
keepPosition?: boolean;
hideOnOverscroll?: boolean;
}) {
const previousInternalPositionRef = useRef<InternalPosition | null>(null);
const [popoverStyle, setPopoverStyle] = useState<Partial<Offset>>({});
const [internalPosition, setInternalPosition] = useState<InternalPosition | null>(null);
const [isOverscrolling, setIsOverscrolling] = useState(false);

// Store the handler in a ref so that it can still be replaced from outside of the listener closure.
const positionHandlerRef = useRef<() => void>(() => {});

const scrollableContainerRectRef = useRef<Rect | null>(null);

const updatePositionHandler = useCallback(
(onContentResize = false) => {
if (!trackRef.current || !popoverRef.current || !bodyRef.current || !contentRef.current || !arrowRef.current) {
Expand Down Expand Up @@ -152,15 +157,32 @@ export default function usePopoverPosition({
scrollRectangleIntoView(rect, scrollableParent);
}

if (hideOnOverscroll && trackRef.current instanceof HTMLElement) {
const scrollableContainer = getFirstScrollableParent(trackRef.current);
if (scrollableContainer) {
scrollableContainerRectRef.current = getLogicalBoundingClientRect(scrollableContainer);
}
}

positionHandlerRef.current = () => {
const trackRect = getLogicalBoundingClientRect(track);

const newTrackOffset = toRelativePosition(
getLogicalBoundingClientRect(track),
trackRect,
containingBlock ? getLogicalBoundingClientRect(containingBlock) : viewportRect
);

setPopoverStyle({
insetBlockStart: newTrackOffset.insetBlockStart + trackRelativeOffset.insetBlockStart,
insetInlineStart: newTrackOffset.insetInlineStart + trackRelativeOffset.insetInlineStart,
});

if (hideOnOverscroll && scrollableContainerRectRef.current) {
// Assuming the arrow tip is at the vertical center of the popover trigger.
// This is good enough for disabled reason tooltip in select and multiselect.
// Can be further refined to take the exact arrow position into account if hideOnOverscroll is to be used in other cases.
setIsOverscrolling(isCenterOutside(trackRect, scrollableContainerRectRef.current));
}
};
},
[
Expand All @@ -170,13 +192,14 @@ export default function usePopoverPosition({
contentRef,
arrowRef,
keepPosition,
allowScrollToFit,
preferredPosition,
renderWithPortal,
allowVerticalOverflow,
allowScrollToFit,
hideOnOverscroll,
]
);
return { updatePositionHandler, popoverStyle, internalPosition, positionHandlerRef };
return { updatePositionHandler, popoverStyle, internalPosition, positionHandlerRef, isOverscrolling };
}

function getBorderWidth(element: HTMLElement) {
Expand Down
9 changes: 8 additions & 1 deletion src/popover/utils/positions.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
import { BoundingBox, Dimensions, InternalPosition, PopoverProps } from '../interfaces';
import { BoundingBox, Dimensions, InternalPosition, PopoverProps, Rect } from '../interfaces';

// A structure describing how the popover should be positioned
interface CalculatedPosition {
Expand Down Expand Up @@ -336,3 +336,10 @@ export function getDimensions(element: HTMLElement) {
function isTopOrBottom(internalPosition: InternalPosition) {
return ['top', 'bottom'].includes(internalPosition.split('-')[0]);
}

export function isCenterOutside(child: Rect, parent: Rect) {
const childCenter = child.insetBlockStart + child.blockSize / 2;
const overflowsBlockStart = childCenter < parent.insetBlockStart;
const overflowsBlockEnd = childCenter > parent.insetBlockEnd;
return overflowsBlockStart || overflowsBlockEnd;
}
Loading

0 comments on commit a2e7df7

Please sign in to comment.