-
Notifications
You must be signed in to change notification settings - Fork 377
fix(Truncate): remove resize observer #12055
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
Changes from all commits
f0bd943
2109481
07ccf59
542162b
2cf0532
e0b9d9d
eb0989d
a07e291
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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 | ||
|
|
||
| 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); | ||
| }); | ||
| 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> | ||
|
|
@@ -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)', () => { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we instead import
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i think
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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(); | ||
| }); | ||

There was a problem hiding this comment.
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.