Skip to content

fix: Require less cursor precision when moving from tooltip target to tooltip content #3359

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

Merged
merged 3 commits into from
Apr 9, 2025
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@
{
"path": "lib/components/internal/widget-exports.js",
"brotli": false,
"limit": "752 kB"
"limit": "753 kB"
}
],
"browserslist": [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import { ButtonProps } from '../../../../button/interfaces';
import { IconProps } from '../../../../icon/interfaces';
import Icon from '../../../../icon/internal';
import Tooltip from '../../../../internal/components/tooltip';
import { registerTooltip } from '../../../../internal/components/tooltip/registry';

import testutilStyles from '../../../test-classes/styles.css.js';
import styles from './styles.css.js';
Expand Down Expand Up @@ -185,15 +184,6 @@ function TriggerButton(
}
}, [containerRef, hasTooltip, tooltipValue]);

useEffect(() => {
if (tooltipVisible) {
return registerTooltip(() => {
setShowTooltip(false);
setSupressTooltip(false);
});
}
}, [tooltipVisible]);

return (
<div
ref={containerRef}
Expand Down Expand Up @@ -238,7 +228,10 @@ function TriggerButton(
trackRef={containerRef}
value={tooltipValue}
className={testutilStyles['trigger-tooltip']}
onDismiss={() => setShowTooltip(false)}
onDismiss={() => {
setShowTooltip(false);
setSupressTooltip(false);
}}
/>
)}
</div>
Expand Down
56 changes: 22 additions & 34 deletions src/breadcrumb-group/item/item.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
import React, { useEffect, useRef, useState } from 'react';
import React, { useRef, useState } from 'react';
import clsx from 'clsx';

import InternalIcon from '../../icon/internal';
import Tooltip from '../../internal/components/tooltip';
import { registerTooltip } from '../../internal/components/tooltip/registry';
import { fireCancelableEvent, isPlainLeftClick } from '../../internal/events';
import { BreadcrumbGroupProps, BreadcrumbItemProps } from '../interfaces';
import { getEventDetail } from '../utils';
Expand All @@ -28,40 +27,29 @@
itemAttributes,
children,
}: BreadcrumbItemWithPopoverProps<T>) => {
const [showPopover, setShowPopover] = useState(false);
const textRef = useRef<HTMLElement>(null);
const popoverContent = (
<Tooltip trackRef={textRef} value={item.text} size="medium" onDismiss={() => setShowPopover(false)} />
);

useEffect(() => {
if (showPopover) {
return registerTooltip(() => {
setShowPopover(false);
});
}
}, [showPopover]);
const [showTooltip, setShowTooltip] = useState(false);
const textRef = useRef<HTMLElement | null>(null);

return (
<>
<Item
ref={textRef}
isLast={isLast}
onFocus={() => {
setShowPopover(true);
}}
onBlur={() => setShowPopover(false)}
onMouseEnter={() => {
setShowPopover(true);
}}
onMouseLeave={() => setShowPopover(false)}
anchorAttributes={anchorAttributes}
{...itemAttributes}
>
{children}
</Item>
{showPopover && popoverContent}
</>
<Item
ref={textRef}
isLast={isLast}
onFocus={() => {
setShowTooltip(true);
}}
onBlur={() => setShowTooltip(false)}
onMouseEnter={() => {
setShowTooltip(true);
}}
onMouseLeave={() => setShowTooltip(false)}

Check warning on line 44 in src/breadcrumb-group/item/item.tsx

View check run for this annotation

Codecov / codecov/patch

src/breadcrumb-group/item/item.tsx#L44

Added line #L44 was not covered by tests
anchorAttributes={anchorAttributes}
{...itemAttributes}
>
{children}
{showTooltip && (
<Tooltip trackRef={textRef} value={item.text} size="medium" onDismiss={() => setShowTooltip(false)} />
)}
</Item>
);
};

Expand Down
37 changes: 0 additions & 37 deletions src/internal/components/tooltip/__tests__/registry.test.ts

This file was deleted.

33 changes: 0 additions & 33 deletions src/internal/components/tooltip/registry.ts

This file was deleted.

48 changes: 48 additions & 0 deletions src/popover/container.scss
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,60 @@
@use '../internal/styles' as styles;
@use '../internal/styles/tokens' as awsui;

$arrow-height: 10px;

.container {
display: inline-block;
position: fixed;
inset-block-start: -9999px;
inset-inline-start: -9999px;
z-index: 2000;

// A little pseudoelement to extend the container to the space between the
// popover and the trigger to avoid hover-based popovers/tooltips from
// closing too eagerly when the cursor goes between them.
&::before {
content: '';
position: absolute;
}

&:has(&-arrow-position-bottom-left),
&:has(&-arrow-position-bottom-center),
&:has(&-arrow-position-bottom-right) {
&::before {
inset-inline: 0;
inset-block-start: -$arrow-height;
block-size: $arrow-height;
}
}

&:has(&-arrow-position-top-left),
&:has(&-arrow-position-top-center),
&:has(&-arrow-position-top-right) {
&::before {
inset-inline: 0;
inset-block-end: -$arrow-height;
block-size: $arrow-height;
}
}

&:has(&-arrow-position-right-top),
&:has(&-arrow-position-right-bottom) {
&::before {
inset-block: 0;
inset-inline-start: -$arrow-height;
inline-size: $arrow-height;
}
}

&:has(&-arrow-position-left-top),
&:has(&-arrow-position-left-bottom) {
&::before {
inset-block: 0;
inset-inline-end: -$arrow-height;
inline-size: $arrow-height;
}
}
}

.container-body {
Expand Down
18 changes: 16 additions & 2 deletions src/slider/__tests__/slider.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,19 @@ describe('Slider events', () => {
expect(screen.queryByText('50')).not.toBeInTheDocument();
});

test('show tooltip on touch start', () => {
const wrapper = renderSlider({
min: 0,
max: 100,
value: 50,
});
fireEvent.touchStart(wrapper.findNativeInput()!.getElement());
expect(screen.queryByText('50')).toBeInTheDocument();

fireEvent.touchEnd(wrapper.findNativeInput()!.getElement());
expect(screen.queryByText('50')).not.toBeInTheDocument();
});

test('close tooltip on Esc keydown', () => {
const wrapper = renderSlider({
min: 0,
Expand Down Expand Up @@ -326,7 +339,8 @@ describe('Slider i18n', () => {
});

describe('Slider a11y', () => {
test('Validates a11y', () => {
// FIXME: This test was never functional. This will be fixed in a subsequent PR.
test.skip('Valides a11y', async () => {
const wrapper = renderSlider({
min: 0,
max: 100,
Expand All @@ -337,7 +351,7 @@ describe('Slider a11y', () => {
ariaLabel: 'aria label',
});

expect(wrapper.getElement()).toValidateA11y();
await expect(wrapper.getElement()).toValidateA11y();
});

test('Renders correct aria label', () => {
Expand Down
Loading
Loading