fix(Truncate): remove resize observer #12055

gitdallas · 2025-10-14T15:50:52Z

What: Closes #10974

patternfly-build · 2025-10-14T15:54:59Z

Preview: https://pf-react-pr-12055.surge.sh

A11y report: https://pf-react-pr-12055-a11y.surge.sh

rebeccaalpert · 2025-10-14T16:39:01Z

I'd want to make sure that #11811 still works - noting this down so I don't forget when I come back to review this.

thatblindgeye

One issue with this update is that the truncate always has a tabindex of 0 now, when it should really only have that when truncation is actually in effect. Otherwise potentially a lot of static text that isn't truncated will just add an equal amount of unneccessary tab stops to the page.

gitdallas · 2025-10-15T14:47:57Z

@thatblindgeye - looks like i can set isTruncated to false originally and then it seems to work initially, however it won't update if something is resized.. maybe that doesn't matter as much as the performance? because i'm not sure if there is a better way to do this without adding the resizeObserver back

thatblindgeye · 2025-10-16T13:34:40Z

looks like i can set isTruncated to false originally and then it seems to work initially, however it won't update if something is resized.. maybe that doesn't matter as much as the performance? because i'm not sure if there is a better way to do this without adding the resizeObserver back

I think both are important, though I don't know which (if either) is more important. That's probably a lengthier discussion.

@christianvogt for your use case, are you taking advantage of the resizability of the Truncate component, i.e. whether the content is actually truncated or not can change depending on the viewport width or the content can be of varying length? Or is it pretty reliable that wherever Truncate is being used things will always be Truncated or always not be Truncated (like in a Table column that is always fairly narrow)?

Mainly asking because if it's the latter - and assuming using the maxCharsDisplayed prop isn't viable - I wonder if adding a prop to dictate whether the resize observer is even used would be useful (or debouncing the resize observer).

christianvogt · 2025-10-16T21:23:33Z

I believe when I created the original issue it was because the component was unmounting and remounting needlessly. Secondary after looking into the code I made the comment about performance and the potential to omit the resize handler. However I didn't consider the tab index case.

For our use case I would still like the text react to container size.

thatblindgeye

After having a quick discussion on Slack, we should update this to keep the resizeObserver, and instead within Truncate import our debounce helper and debounce the handleResize being passed to the getResizeObserver.

kmcfaul

lgtm with the resizeObserver + debounce

packages/react-core/src/components/Truncate/Truncate.tsx

- @patternfly/react-code-editor@6.4.0-prerelease.3 - @patternfly/react-core@6.4.0-prerelease.3 - @patternfly/react-docs@7.4.0-prerelease.4 - @patternfly/react-drag-drop@6.4.0-prerelease.3 - demo-app-ts@6.0.0-prerelease.163 - @patternfly/react-table@6.4.0-prerelease.4 - @patternfly/react-templates@6.4.0-prerelease.3

Signed-off-by: gitdallas <5322142+gitdallas@users.noreply.github.com>

rebeccaalpert · 2025-10-28T19:16:26Z

Looks like #11811 still works, woo!

thatblindgeye · 2025-10-28T19:42:42Z

packages/react-core/src/components/Truncate/__tests__/Truncate.test.tsx

  });
 });
+
+test('Tooltip appears on keyboard focus when external triggerRef is provided (ClipboardCopy regression test)', () => {


thatblindgeye · 2025-10-28T19:43:40Z

packages/react-core/src/components/Truncate/__tests__/Truncate.test.tsx

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


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

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

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

thatblindgeye

Above isn't a blocker to me, depending if we'd want to try quickly moving the new test to ClipboardCopy I'm good with things as-is

gitdallas · 2025-10-29T14:51:15Z

@thatblindgeye added a clipboardcopy/truncate integration test.. separate file as i was having trouble not using the mock in the existing 😆

wise-king-sullyman · 2025-11-03T16:17:40Z

.../react-core/src/components/ClipboardCopy/__tests__/ClipboardCopyTruncateIntegration.test.tsx

+
+// This test file uses the real Truncate component for integration testing, instead of a mock
+
+describe('ClipboardCopy Truncate Integration', () => {


Nit: It feels weird to wrap a single test in a describe block?

wise-king-sullyman · 2025-11-03T16:32:34Z

.../react-core/src/components/ClipboardCopy/__tests__/ClipboardCopyTruncateIntegration.test.tsx

+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


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.

Signed-off-by: gitdallas <5322142+gitdallas@users.noreply.github.com>

gitdallas requested review from a team, dlabaj and rebeccaalpert and removed request for a team October 14, 2025 15:51

gitdallas force-pushed the fix/10974-truncate-remove-resizeObserver branch from 0104b5a to ec5e337 Compare October 14, 2025 15:57

gitdallas force-pushed the fix/10974-truncate-remove-resizeObserver branch from ec5e337 to c047af3 Compare October 14, 2025 16:47

gitdallas changed the title ~~Fix/10974 truncate remove resize observer~~ fix(Truncate): remove resize observer Oct 14, 2025

thatblindgeye reviewed Oct 15, 2025

View reviewed changes

thatblindgeye requested changes Oct 21, 2025

View reviewed changes

gitdallas requested a review from thatblindgeye October 22, 2025 20:21

kmcfaul approved these changes Oct 27, 2025

View reviewed changes

thatblindgeye requested changes Oct 27, 2025

View reviewed changes

packages/react-core/src/components/Truncate/Truncate.tsx Show resolved Hide resolved

patternfly-build and others added 6 commits October 27, 2025 15:22

check truncated before showing tooltip

2109481

Signed-off-by: gitdallas <5322142+gitdallas@users.noreply.github.com>

cypress fix

07ccf59

Signed-off-by: gitdallas <5322142+gitdallas@users.noreply.github.com>

truncate to false initially for better tabindex

542162b

Signed-off-by: gitdallas <5322142+gitdallas@users.noreply.github.com>

bring back resizeObserver with debounce so tabIndex updates

2cf0532

Signed-off-by: gitdallas <5322142+gitdallas@users.noreply.github.com>

start from main and add debounce

e0b9d9d

Signed-off-by: gitdallas <5322142+gitdallas@users.noreply.github.com>

gitdallas force-pushed the fix/10974-truncate-remove-resizeObserver branch from f33716f to e0b9d9d Compare October 28, 2025 15:39

gitdallas requested a review from thatblindgeye October 28, 2025 15:42

cleanup

eb0989d

Signed-off-by: gitdallas <5322142+gitdallas@users.noreply.github.com>

thatblindgeye reviewed Oct 28, 2025

View reviewed changes

thatblindgeye approved these changes Oct 29, 2025

View reviewed changes

wise-king-sullyman reviewed Nov 3, 2025

View reviewed changes

add clipboardcopy truncate integration test

a07e291

Signed-off-by: gitdallas <5322142+gitdallas@users.noreply.github.com>

gitdallas force-pushed the fix/10974-truncate-remove-resizeObserver branch from 9d7e85f to a07e291 Compare November 4, 2025 19:12

thatblindgeye merged commit 20fb229 into patternfly:main Nov 4, 2025
13 checks passed


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

		describe('ClipboardCopy Truncate Integration', () => {

fix(Truncate): remove resize observer #12055

fix(Truncate): remove resize observer #12055

Uh oh!

Conversation

gitdallas commented Oct 14, 2025

Uh oh!

patternfly-build commented Oct 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rebeccaalpert commented Oct 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thatblindgeye left a comment

Choose a reason for hiding this comment

Uh oh!

gitdallas commented Oct 15, 2025

Uh oh!

thatblindgeye commented Oct 16, 2025

Uh oh!

christianvogt commented Oct 16, 2025

Uh oh!

thatblindgeye left a comment

Choose a reason for hiding this comment

Uh oh!

kmcfaul left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rebeccaalpert commented Oct 28, 2025

Uh oh!

thatblindgeye Oct 28, 2025

Choose a reason for hiding this comment

Uh oh!

thatblindgeye Oct 28, 2025

Choose a reason for hiding this comment

Uh oh!

gitdallas Oct 28, 2025

Choose a reason for hiding this comment

Uh oh!

thatblindgeye Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

thatblindgeye left a comment

Choose a reason for hiding this comment

Uh oh!

gitdallas commented Oct 29, 2025

Uh oh!

wise-king-sullyman Nov 3, 2025

Choose a reason for hiding this comment

Uh oh!

wise-king-sullyman Nov 3, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

patternfly-build commented Oct 14, 2025 •

edited

Loading

rebeccaalpert commented Oct 14, 2025 •

edited

Loading