Skip to content

Commit

Permalink
fix: Hide disabled reason tooltips when the option is scrolled away
Browse files Browse the repository at this point in the history
  • Loading branch information
jperals committed Sep 27, 2024
1 parent 1a3ef83 commit 88f8a28
Show file tree
Hide file tree
Showing 8 changed files with 129 additions and 23 deletions.
43 changes: 29 additions & 14 deletions pages/select/disabled-reason.page.tsx
Original file line number Diff line number Diff line change
@@ -1,32 +1,47 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
import * as React from 'react';
import React, { useContext } from 'react';

import Box from '~components/box';
import Select from '~components/select';

import AppContext, { AppContextType } from '../app/app-context';
import ScreenshotArea from '../utils/screenshot-area';

const options = [
{
value: '1',
label: 'Option 1',
disabled: true,
disabledReason: 'disabled reason',
},
{
value: '2',
label: 'Option 2',
},
];
type PageContext = React.Context<
AppContextType<{
expandToViewport: boolean;
virtualScroll: boolean;
}>
>;

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() {
const { urlParams } = useContext(AppContext as PageContext);

return (
<ScreenshotArea>
<Box variant="h1">Select with disabled reason</Box>
<Box padding="l">
<div style={{ width: '400px' }}>
<Select placeholder="Choose option" selectedOption={null} options={options} />
<Select
expandToViewport={urlParams.expandToViewport}
virtualScroll={urlParams.virtualScroll}
placeholder="Choose option"
selectedOption={null}
options={options}
/>
</div>
</Box>
</ScreenshotArea>
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'];
hideOnTriggerOverflow?: boolean;
}

export default function Tooltip({
Expand All @@ -30,6 +31,7 @@ export default function Tooltip({
contentAttributes = {},
position = 'top',
size = 'small',
hideOnTriggerOverflow,
}: 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} />}
hideOnTriggerOverflow={hideOnTriggerOverflow}
>
<PopoverBody dismissButton={false} dismissAriaLabel={undefined} onDismiss={undefined} header={undefined}>
{value}
Expand Down
14 changes: 14 additions & 0 deletions src/internal/utils/scrollable-containers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,3 +123,17 @@ export function getFirstScrollableParent(element: HTMLElement): HTMLElement | un
}) || undefined
);
}

export function isInScroll(element: HTMLElement | SVGElement) {
if (element instanceof SVGElement) {
return true;

Check warning on line 129 in src/internal/utils/scrollable-containers.ts

View check run for this annotation

Codecov / codecov/patch

src/internal/utils/scrollable-containers.ts#L129

Added line #L129 was not covered by tests
}
const scrollableParent = getFirstScrollableParent(element);

Check warning on line 131 in src/internal/utils/scrollable-containers.ts

View check run for this annotation

Codecov / codecov/patch

src/internal/utils/scrollable-containers.ts#L131

Added line #L131 was not covered by tests
if (!scrollableParent) {
return true;

Check warning on line 133 in src/internal/utils/scrollable-containers.ts

View check run for this annotation

Codecov / codecov/patch

src/internal/utils/scrollable-containers.ts#L133

Added line #L133 was not covered by tests
}
const elementCenter = element.offsetTop + element.offsetHeight / 2;
const isAbove = elementCenter < scrollableParent.scrollTop;
const isBelow = elementCenter > scrollableParent.clientHeight - scrollableParent.scrollTop;

Check warning on line 137 in src/internal/utils/scrollable-containers.ts

View check run for this annotation

Codecov / codecov/patch

src/internal/utils/scrollable-containers.ts#L135-L137

Added lines #L135 - L137 were not covered by tests
return !isAbove && !isBelow;
}
20 changes: 15 additions & 5 deletions src/popover/container.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
import React, { useLayoutEffect, useRef } from 'react';
import React, { useLayoutEffect, useRef, useState } from 'react';
import clsx from 'clsx';

import { nodeContains } from '@cloudscape-design/component-toolkit/dom';
import { useResizeObserver } from '@cloudscape-design/component-toolkit/internal';

import { useVisualRefresh } from '../internal/hooks/use-visual-mode';
import { isInScroll } from '../internal/utils/scrollable-containers';
import { InternalPosition, PopoverProps } from './interfaces';
import usePopoverPosition from './use-popover-position.js';

Expand Down Expand Up @@ -39,6 +40,7 @@ export interface PopoverContainerProps {
// Do not use this if the popover is open on hover, in order to avoid unexpected movement.
allowScrollToFit?: boolean;
allowVerticalOverflow?: boolean;
hideOnTriggerOverflow?: boolean;
}

export default function PopoverContainer({
Expand All @@ -55,12 +57,15 @@ export default function PopoverContainer({
keepPosition,
allowScrollToFit,
allowVerticalOverflow,
hideOnTriggerOverflow,
}: PopoverContainerProps) {
const bodyRef = useRef<HTMLDivElement | null>(null);
const contentRef = useRef<HTMLDivElement | null>(null);
const popoverRef = useRef<HTMLDivElement | null>(null);
const arrowRef = useRef<HTMLDivElement | null>(null);

const [hideDueToOverScroll, setHideDueToOverscroll] = useState(false);

const isRefresh = useVisualRefresh();

// Updates the position handler.
Expand Down Expand Up @@ -113,8 +118,13 @@ export default function PopoverContainer({
};

const updatePositionOnResize = () => requestAnimationFrame(() => updatePositionHandler());
const refreshPosition = () => requestAnimationFrame(() => positionHandlerRef.current());

const refreshPosition = () => {
const hide = !!hideOnTriggerOverflow && !!trackRef.current && !isInScroll(trackRef.current);
setHideDueToOverscroll(hide);

Check warning on line 123 in src/popover/container.tsx

View check run for this annotation

Codecov / codecov/patch

src/popover/container.tsx#L123

Added line #L123 was not covered by tests
if (!hide) {
requestAnimationFrame(() => positionHandlerRef.current());

Check warning on line 125 in src/popover/container.tsx

View check run for this annotation

Codecov / codecov/patch

src/popover/container.tsx#L125

Added line #L125 was not covered by tests
}
};
window.addEventListener('click', onClick);
window.addEventListener('resize', updatePositionOnResize);
window.addEventListener('scroll', refreshPosition, true);
Expand All @@ -124,9 +134,9 @@ export default function PopoverContainer({
window.removeEventListener('resize', updatePositionOnResize);
window.removeEventListener('scroll', refreshPosition, true);
};
}, [keepPosition, positionHandlerRef, trackRef, updatePositionHandler]);
}, [hideOnTriggerOverflow, keepPosition, positionHandlerRef, trackRef, updatePositionHandler]);

return (
return hideDueToOverScroll ? null : (
<div
ref={popoverRef}
style={{ ...popoverStyle, zIndex }}
Expand Down
58 changes: 58 additions & 0 deletions src/select/__integ__/disabled-reason.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
import useBrowser from '@cloudscape-design/browser-test-tools/use-browser';

import createWrapper from '../../../lib/components/test-utils/selectors';
import SelectPageObject from './page-objects/select-page';

function setupTest(
{ expandToViewport, virtualScroll }: { expandToViewport: boolean; virtualScroll: boolean },
testFn: (page: SelectPageObject) => Promise<void>
) {
return useBrowser(async browser => {
await browser.url(
`/#/light/select/disabled-reason?expandToViewport=${expandToViewport}&virtualScroll=${virtualScroll}`
);
const select = createWrapper().findSelect();
const page = new SelectPageObject(browser, select);
await testFn(page);
});
}

describe('Disabled reasons', () => {
describe.each([false, true])('expandToViewport=%s', expandToViewport => {
describe.each([false, true])('virtualScroll=%s', virtualScroll => {
test(
'shows disabled reason on hover',
setupTest({ expandToViewport, virtualScroll }, async page => {
await page.pause(100);
const select = createWrapper().findSelect();
await page.clickSelect();
await page.assertDropdownOpen(true, expandToViewport);
const firstDisabledOption = select.findDropdown({ expandToViewport }).findOption(1);
await page.hoverElement(firstDisabledOption.toSelector());
const disabledTooltip = firstDisabledOption.findDisabledReason();
expect(await page.isDisplayed(disabledTooltip.toSelector())).toBe(true);
})
);

test(
'hides disabled reason when the disabled option is scrolled out of view',
setupTest({ expandToViewport, virtualScroll }, async page => {
await page.pause(100);
const select = createWrapper().findSelect();
await page.clickSelect();
await page.assertDropdownOpen(true, expandToViewport);
const dropdown = select.findDropdown({ expandToViewport });
const firstDisabledOption = select.findDropdown({ expandToViewport }).findOption(1);
await page.hoverElement(firstDisabledOption.toSelector());
const disabledTooltipSelector = firstDisabledOption.findDisabledReason().toSelector();
expect(await page.isDisplayed(disabledTooltipSelector)).toBe(true);
await page.elementScrollTo(dropdown.findOptionsContainer().toSelector(), { top: 500 });
await page.waitForJsTimers();
expect(await page.isDisplayed(disabledTooltipSelector)).toBe(false);
})
);
});
});
});
12 changes: 8 additions & 4 deletions src/select/__integ__/page-objects/select-page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,16 @@ export default class SelectPageObject<
await this.browser.releaseActions();
}

isDropdownOpen() {
return this.isExisting(this.wrapper.findDropdown().findOpenDropdown().toSelector());
isDropdownOpen(expandToViewport = false) {
return this.isExisting(this.wrapper.findDropdown({ expandToViewport }).findOpenDropdown().toSelector());
}

async assertDropdownOpen(isOpen: boolean) {
await assert.equal(await this.isDropdownOpen(), isOpen, `Select dropdown should ${isOpen ? '' : 'not '} be open`);
async assertDropdownOpen(isOpen: boolean, expandToViewport?: boolean) {
await assert.equal(
await this.isDropdownOpen(expandToViewport),
isOpen,
`Select dropdown should ${isOpen ? '' : 'not '} be open`
);
}

async ensureDropdownOpen() {
Expand Down
1 change: 1 addition & 0 deletions src/select/parts/item.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ const Item = (
trackRef={internalRef}
value={disabledReason!}
position="right"
hideOnTriggerOverflow={true}
/>
)}
</>
Expand Down
1 change: 1 addition & 0 deletions src/select/parts/multiselect-item.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ const MultiSelectItem = (
trackRef={internalRef}
value={disabledReason!}
position="right"
hideOnTriggerOverflow={true}
/>
)}
</>
Expand Down

0 comments on commit 88f8a28

Please sign in to comment.