Skip to content
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { render, screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import { ClipboardCopy, ClipboardCopyVariant } from '../ClipboardCopy';

// This test file uses the real Truncate component for integration testing, instead of a mock
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It also feels weird in general having this test of ClipboardCopy integration for a change that we're making in Truncate 🤔

That being said I don't dislike this test, and I don't know better way of doing it after some quick poking around, so I won't block over it.


test('Tooltip appears on keyboard focus when using inline-compact variant with truncation', async () => {
const user = userEvent.setup();
const longText = 'This is a very long piece of content that should be truncated when the container is too small';

render(
<ClipboardCopy
variant={ClipboardCopyVariant.inlineCompact}
truncation={{ maxCharsDisplayed: 20 }}
data-testid="clipboard-copy"
>
{longText}
</ClipboardCopy>
);

expect(screen.queryByText(longText)).not.toBeInTheDocument();
expect(screen.queryByRole('tooltip')).not.toBeInTheDocument();

await user.tab();

const clipboardCopy = screen.getByTestId('clipboard-copy');
expect(clipboardCopy).toHaveFocus();

const tooltip = screen.getByRole('tooltip');
expect(tooltip).toBeInTheDocument();
expect(tooltip).toHaveTextContent(longText);
});
9 changes: 5 additions & 4 deletions packages/react-core/src/components/Truncate/Truncate.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { css } from '@patternfly/react-styles';
import { Tooltip, TooltipPosition, TooltipProps } from '../Tooltip';
import { getReferenceElement } from '../../helpers';
import { getResizeObserver } from '../../helpers/resizeObserver';
import { debounce } from '../../helpers/util';

export enum TruncatePosition {
start = 'start',
Expand Down Expand Up @@ -130,12 +131,12 @@ const TruncateBase: React.FunctionComponent<TruncateProps> = ({
const totalTextWidth = calculateTotalTextWidth(textElement, trailingNumChars, content);
const textWidth = position === 'middle' ? totalTextWidth : textElement.scrollWidth;

const handleResize = () => {
const debouncedHandleResize = debounce(() => {
const parentWidth = getActualWidth(parentElement);
setIsTruncated(textWidth >= parentWidth);
};
}, 500);

const observer = getResizeObserver(parentElement, handleResize);
const observer = getResizeObserver(parentElement, debouncedHandleResize);

return () => {
observer();
Expand All @@ -147,7 +148,7 @@ const TruncateBase: React.FunctionComponent<TruncateProps> = ({
if (shouldRenderByMaxChars) {
setIsTruncated(content.length > maxCharsDisplayed);
}
}, [shouldRenderByMaxChars]);
}, [shouldRenderByMaxChars, content.length, maxCharsDisplayed]);

useEffect(() => {
setShouldRenderByMaxChars(maxCharsDisplayed > 0);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import { render, screen, within } from '@testing-library/react';
import { Truncate, TruncatePosition } from '../Truncate';
import { render, screen, fireEvent, waitFor } from '@testing-library/react';
import { Truncate } from '../Truncate';
import styles from '@patternfly/react-styles/css/components/Truncate/truncate';
import '@testing-library/jest-dom';

jest.mock('../../Tooltip', () => ({
Tooltip: ({ content, position, children, triggerRef, ...props }) => (
<div data-testid="Tooltip-mock" {...props}>
<div data-testid="Tooltip-mock-content-container">Test {content}</div>
<div>Test {content}</div>
<p>{`position: ${position}`}</p>
{children}
</div>
Expand Down Expand Up @@ -242,3 +242,21 @@ describe('Truncation with maxCharsDisplayed', () => {
expect(asFragment()).toMatchSnapshot();
});
});

test('Tooltip appears on keyboard focus when external triggerRef is provided (ClipboardCopy regression test)', () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const mockTriggerRef = { current: document.createElement('div') };

render(
<Truncate
content="This is a very long piece of content that should be truncated when the container is too small"
tooltipProps={{ triggerRef: mockTriggerRef }}
/>
);

// Simulate keyboard focus on the external trigger element
fireEvent.focus(mockTriggerRef.current);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we instead import userEvent and utilize that to handle the focusing in this test?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think fireEvent might actually be right here as it's how it would come into the Truncate component, rather than being a full integration test. currently the mockTriggerRef isn't actually rendered as part of the DOM to tab to. could update the .current and check, but now i'm thinking maybe that would be better tested in the clipboardcopy

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah maybe worth trying to see if wew can put this sort of test in ClipboardCopy, though having it live in Truncate could make sense if we were ever to use it in other components in the same way we are in ClipboardCopy.

Not a blocker for me, otherwise things lgtm here


// The tooltip should be present and visible
const tooltip = screen.getByTestId('Tooltip-mock');
expect(tooltip).toBeInTheDocument();
});
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,7 @@ exports[`Truncation with maxCharsDisplayed Matches snapshot with default positio
<div
data-testid="Tooltip-mock"
>
<div
data-testid="Tooltip-mock-content-container"
>
<div>
Test Test truncation content
</div>
<p>
Expand Down Expand Up @@ -43,9 +41,7 @@ exports[`renders default truncation 1`] = `
<div
data-testid="Tooltip-mock"
>
<div
data-testid="Tooltip-mock-content-container"
>
<div>
Test Vestibulum interdum risus et enim faucibus, sit amet molestie est accumsan.
</div>
<p>
Expand All @@ -70,9 +66,7 @@ exports[`renders start truncation with &lrm; at start and end 1`] = `
<div
data-testid="Tooltip-mock"
>
<div
data-testid="Tooltip-mock-content-container"
>
<div>
Test Vestibulum interdum risus et enim faucibus, sit amet molestie est accumsan.
</div>
<p>
Expand Down
Loading