From 20f9f58e4c6ba5e4eceba83a1fab9b28bed5fd71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20V=C3=A9lez?= Date: Thu, 1 May 2025 11:20:41 +0200 Subject: [PATCH] MM-63648 - markdown images sometimes do not show the more button (#30716) * MM-63648 - markdown images sometimes do not show the more button * migrate test to testing-library and remove unnecesary props --------- Co-authored-by: Mattermost Build --- .../__snapshots__/show_more.test.tsx.snap | 230 +--------- .../post_view/show_more/show_more.test.tsx | 417 +++++++++++++++--- .../post_view/show_more/show_more.tsx | 45 +- 3 files changed, 399 insertions(+), 293 deletions(-) diff --git a/webapp/channels/src/components/post_view/show_more/__snapshots__/show_more.test.tsx.snap b/webapp/channels/src/components/post_view/show_more/__snapshots__/show_more.test.tsx.snap index 6ce6b94a37..27c54c88d8 100644 --- a/webapp/channels/src/components/post_view/show_more/__snapshots__/show_more.test.tsx.snap +++ b/webapp/channels/src/components/post_view/show_more/__snapshots__/show_more.test.tsx.snap @@ -1,231 +1,19 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP exports[`components/post_view/ShowMore should match snapshot 1`] = ` -
+
-
-

- text -

-
-
-
-`; - -exports[`components/post_view/ShowMore should match snapshot, PostAttachment on collapsed view 1`] = ` -
-
-
-
- -
-
-
-
-`; - -exports[`components/post_view/ShowMore should match snapshot, PostAttachment on expanded view 1`] = ` -
-
-
-
-
- -
-
-
-
-`; - -exports[`components/post_view/ShowMore should match snapshot, PostMessageView on collapsed view 1`] = ` -
-
-
-
-
- -
-
-
-
-`; - -exports[`components/post_view/ShowMore should match snapshot, PostMessageView on expanded view 1`] = ` -
-
-
-
-
- -
-
-
-
-`; - -exports[`components/post_view/ShowMore should match snapshot, PostMessageView on expanded view with compactDisplay 1`] = ` -
-
-
-
-
- -
+
+

+ text +

+
diff --git a/webapp/channels/src/components/post_view/show_more/show_more.test.tsx b/webapp/channels/src/components/post_view/show_more/show_more.test.tsx index 43201691af..4db9d75abd 100644 --- a/webapp/channels/src/components/post_view/show_more/show_more.test.tsx +++ b/webapp/channels/src/components/post_view/show_more/show_more.test.tsx @@ -1,102 +1,397 @@ // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. // See LICENSE.txt for license information. -import {shallow} from 'enzyme'; import React from 'react'; import ShowMore from 'components/post_view/show_more/show_more'; +import {renderWithContext, screen, fireEvent, act} from 'tests/react_testing_utils'; + describe('components/post_view/ShowMore', () => { const children = (

{'text'}

); const baseProps = { checkOverflow: 0, isAttachmentText: false, - isRHSExpanded: false, - isRHSOpen: false, maxHeight: 200, text: 'text', compactDisplay: false, }; + // Helper function to mock the text container's scrollHeight + const mockTextContainerScrollHeight = (scrollHeight: number) => { + // Mock the scrollHeight property + Object.defineProperty(HTMLElement.prototype, 'scrollHeight', { + configurable: true, + value: scrollHeight, + }); + }; + + // Helper function to restore the original scrollHeight behavior + const restoreTextContainerScrollHeight = () => { + // Delete the mocked scrollHeight property + delete (HTMLElement.prototype as any).scrollHeight; + }; + + afterEach(() => { + restoreTextContainerScrollHeight(); + }); + test('should match snapshot', () => { - const wrapper = shallow({children}); - expect(wrapper).toMatchSnapshot(); + const {container} = renderWithContext({children}); + expect(container).toMatchSnapshot(); }); - test('should match snapshot, PostMessageView on collapsed view', () => { - const wrapper = shallow(); - wrapper.setState({isOverflow: true, isCollapsed: true}); - expect(wrapper).toMatchSnapshot(); + test('should render collapsed view when content overflows', () => { + // Setup fake timers + jest.useFakeTimers(); + try { + // Mock scrollHeight to be greater than maxHeight to simulate overflow + mockTextContainerScrollHeight(baseProps.maxHeight + 50); + + const {container} = renderWithContext( + +
{'Tall content that will overflow'}
+ , + ); + + // Manually trigger the overflow check + act(() => { + // Run the requestAnimationFrame callback + jest.runOnlyPendingTimers(); + }); + + // Verify the "Show more" button is rendered + const showMoreButton = screen.getByText('Show more'); + expect(showMoreButton).toBeInTheDocument(); + + // Verify the collapsed class is applied + expect(container.querySelector('.post-message--collapsed')).toBeInTheDocument(); + } finally { + jest.useRealTimers(); + } }); - test('should match snapshot, PostMessageView on expanded view', () => { - const wrapper = shallow(); - wrapper.setState({isOverflow: true, isCollapsed: false}); - expect(wrapper).toMatchSnapshot(); + test('should render expanded view when show more button is clicked', () => { + // Setup fake timers + jest.useFakeTimers(); + try { + // Mock scrollHeight to be greater than maxHeight to simulate overflow + mockTextContainerScrollHeight(baseProps.maxHeight + 50); + + const {container} = renderWithContext( + +
{'Tall content that will overflow'}
+ , + ); + + // Manually trigger the overflow check + act(() => { + // Run the requestAnimationFrame callback + jest.runOnlyPendingTimers(); + }); + + // Find and click the "Show more" button + const showMoreButton = screen.getByText('Show more'); + fireEvent.click(showMoreButton); + + // Verify the "Show less" button is now rendered + const showLessButton = screen.getByText('Show less'); + expect(showLessButton).toBeInTheDocument(); + + // Verify the expanded class is applied + expect(container.querySelector('.post-message--expanded')).toBeInTheDocument(); + } finally { + jest.useRealTimers(); + } }); - test('should match snapshot, PostAttachment on collapsed view', () => { - const wrapper = shallow( - , - ); - wrapper.setState({isOverflow: true, isCollapsed: true}); - expect(wrapper).toMatchSnapshot(); + test('should render attachment text in collapsed view', () => { + // Setup fake timers + jest.useFakeTimers(); + try { + // Mock scrollHeight to be greater than maxHeight to simulate overflow + mockTextContainerScrollHeight(baseProps.maxHeight + 50); + + const {container} = renderWithContext( + +
{'Attachment text that will overflow'}
+ , + ); + + // Manually trigger the overflow check + act(() => { + // Run the requestAnimationFrame callback + jest.runOnlyPendingTimers(); + }); + + // Verify the "Show more" button is rendered + const showMoreButton = screen.getByText('Show more'); + expect(showMoreButton).toBeInTheDocument(); + + // Verify the attachment-specific class is applied + expect(container.querySelector('.post-attachment-collapse__show-more')).toBeInTheDocument(); + } finally { + jest.useRealTimers(); + } }); - test('should match snapshot, PostAttachment on expanded view', () => { - const wrapper = shallow( - , - ); - wrapper.setState({isOverflow: true, isCollapsed: false}); - expect(wrapper).toMatchSnapshot(); + test('should render with compactDisplay', () => { + // Setup fake timers + jest.useFakeTimers(); + try { + // Mock scrollHeight to be greater than maxHeight to simulate overflow + mockTextContainerScrollHeight(baseProps.maxHeight + 50); + + const {container} = renderWithContext( + +
{'Content with compact display'}
+ , + ); + + // Manually trigger the overflow check + act(() => { + // Run the requestAnimationFrame callback + jest.runOnlyPendingTimers(); + }); + + // Expand the content + const showMoreButton = screen.getByText('Show more'); + fireEvent.click(showMoreButton); + + // Verify the component renders correctly with compact display + expect(container.querySelector('.post-message--expanded')).toBeInTheDocument(); + } finally { + jest.useRealTimers(); + } }); - test('should match snapshot, PostMessageView on expanded view with compactDisplay', () => { - const wrapper = shallow( - , - ); - wrapper.setState({isOverflow: true, isCollapsed: false}); - expect(wrapper).toMatchSnapshot(); + test('should check overflow only when text or checkOverflow props change', () => { + // Create a spy for requestAnimationFrame + const originalRAF = window.requestAnimationFrame; + const rafSpy = jest.fn((cb) => { + cb(0); + return 0; + }); + window.requestAnimationFrame = rafSpy; + + try { + // Initial render with no overflow + mockTextContainerScrollHeight(baseProps.maxHeight - 50); + const {rerender} = renderWithContext({children}); + + // Reset the RAF spy count + rafSpy.mockClear(); + + // Change props that SHOULD trigger overflow check + rafSpy.mockClear(); + rerender( + {children}, + ); + expect(rafSpy).toHaveBeenCalled(); + + rafSpy.mockClear(); + rerender( + {children}, + ); + expect(rafSpy).toHaveBeenCalled(); + + rafSpy.mockClear(); + rerender( + {children}, + ); + expect(rafSpy).toHaveBeenCalled(); + + // Same checkOverflow value should not trigger another check + rafSpy.mockClear(); + rerender( + {children}, + ); + expect(rafSpy).not.toHaveBeenCalled(); + } finally { + // Restore original requestAnimationFrame + window.requestAnimationFrame = originalRAF; + } }); - test('should call checkTextOverflow', () => { - const wrapper = shallow(); - const instance = wrapper.instance() as ShowMore; - instance.checkTextOverflow = jest.fn(); + describe('ResizeObserver functionality', () => { + let originalResizeObserver: any; - expect(instance.checkTextOverflow).not.toBeCalled(); + beforeEach(() => { + // Store original implementation + originalResizeObserver = window.ResizeObserver; - wrapper.setProps({isRHSExpanded: true}); - expect(instance.checkTextOverflow).toBeCalledTimes(1); + // Setup fake timers for requestAnimationFrame + jest.useFakeTimers(); + }); - wrapper.setProps({isRHSExpanded: false}); - expect(instance.checkTextOverflow).toBeCalledTimes(2); + afterEach(() => { + // Restore original implementation + window.ResizeObserver = originalResizeObserver; - wrapper.setProps({isRHSOpen: true}); - expect(instance.checkTextOverflow).toBeCalledTimes(3); + // Restore real timers + jest.useRealTimers(); + }); - wrapper.setProps({isRHSOpen: false}); - expect(instance.checkTextOverflow).toBeCalledTimes(4); + test('should set up ResizeObserver on mount', () => { + // Track observer creation + const observeMock = jest.fn(); + const disconnectMock = jest.fn(); - wrapper.setProps({text: 'text change'}); - expect(instance.checkTextOverflow).toBeCalledTimes(5); + // Mock ResizeObserver + window.ResizeObserver = jest.fn().mockImplementation(() => ({ + observe: observeMock, + disconnect: disconnectMock, + })) as unknown as typeof ResizeObserver; - wrapper.setProps({text: 'text another change'}); - expect(instance.checkTextOverflow).toBeCalledTimes(6); + // Render component + renderWithContext({children}); - wrapper.setProps({checkOverflow: 1}); - expect(instance.checkTextOverflow).toBeCalledTimes(7); + // Verify ResizeObserver was created + expect(window.ResizeObserver).toHaveBeenCalled(); - wrapper.setProps({checkOverflow: 1}); - expect(instance.checkTextOverflow).toBeCalledTimes(7); + // Verify observe was called (meaning the text container is being observed) + expect(observeMock).toHaveBeenCalled(); + }); + + test('should check overflow when ResizeObserver detects size changes', () => { + // Create a mock ResizeObserver that can be triggered manually + let resizeCallback: ResizeObserverCallback | undefined; + window.ResizeObserver = jest.fn().mockImplementation((callback) => { + resizeCallback = callback; + return { + observe: jest.fn(), + disconnect: jest.fn(), + }; + }) as unknown as typeof ResizeObserver; + + // Mock requestAnimationFrame to execute callback immediately + const originalRAF = window.requestAnimationFrame; + window.requestAnimationFrame = (cb) => { + cb(0); + return 0; + }; + + try { + // Mock scrollHeight to be greater than maxHeight after resize + mockTextContainerScrollHeight(baseProps.maxHeight - 50); + + // Render component + const {container} = renderWithContext({children}); + + // Verify no overflow initially + expect(container.querySelector('.post-message--overflow')).not.toBeInTheDocument(); + + // Now simulate a resize that causes overflow + mockTextContainerScrollHeight(baseProps.maxHeight + 50); + + // Trigger the ResizeObserver callback + if (resizeCallback && container.querySelector('.post-message__text-container')) { + const mockEntry = [{ + target: container.querySelector('.post-message__text-container') as Element, + contentRect: {} as DOMRectReadOnly, + borderBoxSize: [] as ResizeObserverSize[], + contentBoxSize: [] as ResizeObserverSize[], + devicePixelContentBoxSize: [] as ResizeObserverSize[], + }]; + + act(() => { + resizeCallback!(mockEntry, {} as ResizeObserver); + }); + } + + // Verify overflow is detected + expect(container.querySelector('.post-message--overflow')).toBeInTheDocument(); + } finally { + // Restore original requestAnimationFrame + window.requestAnimationFrame = originalRAF; + } + }); + + test('should clean up ResizeObserver on unmount', () => { + // Create mock with disconnect spy + const disconnectSpy = jest.fn(); + window.ResizeObserver = jest.fn(() => ({ + observe: jest.fn(), + disconnect: disconnectSpy, + })) as unknown as typeof ResizeObserver; + + // Render and unmount component + const {unmount} = renderWithContext({children}); + unmount(); + + // Verify disconnect was called + expect(disconnectSpy).toHaveBeenCalled(); + }); + + test('should handle browsers without ResizeObserver support', () => { + // Remove ResizeObserver + delete (window as any).ResizeObserver; + + // Render component should not throw error + expect(() => { + renderWithContext({children}); + }).not.toThrow(); + }); + + test('should update isOverflow state when content height changes', () => { + // Mock ResizeObserver before rendering + window.ResizeObserver = jest.fn().mockImplementation(() => { + return { + observe: jest.fn(), + disconnect: jest.fn(), + }; + }) as unknown as typeof ResizeObserver; + + // Mock requestAnimationFrame to execute callback immediately + const originalRAF = window.requestAnimationFrame; + window.requestAnimationFrame = (cb) => { + cb(0); + return 0; + }; + + try { + // Initial render with no overflow + mockTextContainerScrollHeight(baseProps.maxHeight - 50); + const {container, rerender} = renderWithContext({children}); + + // Verify no overflow initially + expect(container.querySelector('.post-message--overflow')).not.toBeInTheDocument(); + + // Now simulate content that overflows + mockTextContainerScrollHeight(baseProps.maxHeight + 50); + + // Force a re-render to trigger checkTextOverflow + rerender( + {children}, + ); + + // Verify overflow is detected + expect(container.querySelector('.post-message--overflow')).toBeInTheDocument(); + } finally { + // Restore original requestAnimationFrame + window.requestAnimationFrame = originalRAF; + } + }); }); }); diff --git a/webapp/channels/src/components/post_view/show_more/show_more.tsx b/webapp/channels/src/components/post_view/show_more/show_more.tsx index ed147887f2..aeafc9ed1c 100644 --- a/webapp/channels/src/components/post_view/show_more/show_more.tsx +++ b/webapp/channels/src/components/post_view/show_more/show_more.tsx @@ -13,8 +13,6 @@ type Props = { children?: React.ReactNode; checkOverflow?: number; isAttachmentText?: boolean; - isRHSExpanded: boolean; - isRHSOpen: boolean; text?: string; compactDisplay: boolean; overflowType?: AttachmentTextOverflowType; @@ -30,6 +28,7 @@ export default class ShowMore extends React.PureComponent { private maxHeight: number; private textContainer: React.RefObject; private overflowRef?: number; + private resizeObserver: ResizeObserver | null = null; constructor(props: Props) { super(props); @@ -42,16 +41,17 @@ export default class ShowMore extends React.PureComponent { } componentDidMount() { - this.checkTextOverflow(); + this.setupResizeObserver(); - window.addEventListener('resize', this.handleResize); + // Initial check for overflow + this.checkTextOverflow(); } componentDidUpdate(prevProps: Props) { + // Only manually check for overflow when text content changes or when explicitly requested + // ResizeObserver will handle size changes caused by other factors if ( this.props.text !== prevProps.text || - this.props.isRHSExpanded !== prevProps.isRHSExpanded || - this.props.isRHSOpen !== prevProps.isRHSOpen || this.props.checkOverflow !== prevProps.checkOverflow ) { this.checkTextOverflow(); @@ -59,12 +59,39 @@ export default class ShowMore extends React.PureComponent { } componentWillUnmount() { - window.removeEventListener('resize', this.handleResize); if (this.overflowRef) { window.cancelAnimationFrame(this.overflowRef); } + this.cleanupResizeObserver(); } + setupResizeObserver = () => { + if (!this.textContainer.current || !window.ResizeObserver) { + // ResizeObserver is not supported in this browser or the container is not available yet + return; + } + + // Clean up any existing observer before creating a new one + // This prevents multiple observers in case setupResizeObserver is called more than once + this.cleanupResizeObserver(); + + // Create a new ResizeObserver to watch for size changes in the text container + this.resizeObserver = new ResizeObserver(() => { + // When the size of the text container changes, check if we need to show/hide the "Show More" button + this.checkTextOverflow(); + }); + + // Start observing the text container + this.resizeObserver.observe(this.textContainer.current); + }; + + cleanupResizeObserver = () => { + if (this.resizeObserver) { + this.resizeObserver.disconnect(); + this.resizeObserver = null; + } + }; + toggleCollapse = (e: React.MouseEvent) => { e.preventDefault(); e.stopPropagation(); @@ -94,10 +121,6 @@ export default class ShowMore extends React.PureComponent { }); }; - handleResize = () => { - this.checkTextOverflow(); - }; - render() { const { isCollapsed,