From 332be84efd6a7f8d3b53e996e07a18783c515fde Mon Sep 17 00:00:00 2001 From: Jesse Hallam Date: Wed, 28 May 2025 18:53:34 -0300 Subject: [PATCH] Revert "MM-63648 - markdown images sometimes do not show the more button (#30716)" (#31224) This reverts commit 20f9f58e4c6ba5e4eceba83a1fab9b28bed5fd71. --- .../__snapshots__/show_more.test.tsx.snap | 234 +++++++++- .../post_view/show_more/show_more.test.tsx | 417 +++--------------- .../post_view/show_more/show_more.tsx | 45 +- 3 files changed, 295 insertions(+), 401 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 27c54c88d8..6ce6b94a37 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,19 +1,231 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP exports[`components/post_view/ShowMore should match snapshot 1`] = ` -
+
-
-
-

- text -

-
+
+

+ 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`] = ` +
+
+
+
+
+ +
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 4db9d75abd..43201691af 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,397 +1,102 @@ // 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 {container} = renderWithContext({children}); - expect(container).toMatchSnapshot(); + const wrapper = shallow({children}); + 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 collapsed view', () => { + const wrapper = shallow(); + wrapper.setState({isOverflow: true, isCollapsed: true}); + 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, PostMessageView on expanded view', () => { + const wrapper = shallow(); + wrapper.setState({isOverflow: true, isCollapsed: false}); + 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 collapsed view', () => { + const wrapper = shallow( + , + ); + wrapper.setState({isOverflow: true, isCollapsed: true}); + 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, PostAttachment on expanded view', () => { + 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 match snapshot, PostMessageView on expanded view with compactDisplay', () => { + const wrapper = shallow( + , + ); + wrapper.setState({isOverflow: true, isCollapsed: false}); + expect(wrapper).toMatchSnapshot(); }); - describe('ResizeObserver functionality', () => { - let originalResizeObserver: any; + test('should call checkTextOverflow', () => { + const wrapper = shallow(); + const instance = wrapper.instance() as ShowMore; + instance.checkTextOverflow = jest.fn(); - beforeEach(() => { - // Store original implementation - originalResizeObserver = window.ResizeObserver; + expect(instance.checkTextOverflow).not.toBeCalled(); - // Setup fake timers for requestAnimationFrame - jest.useFakeTimers(); - }); + wrapper.setProps({isRHSExpanded: true}); + expect(instance.checkTextOverflow).toBeCalledTimes(1); - afterEach(() => { - // Restore original implementation - window.ResizeObserver = originalResizeObserver; + wrapper.setProps({isRHSExpanded: false}); + expect(instance.checkTextOverflow).toBeCalledTimes(2); - // Restore real timers - jest.useRealTimers(); - }); + wrapper.setProps({isRHSOpen: true}); + expect(instance.checkTextOverflow).toBeCalledTimes(3); - test('should set up ResizeObserver on mount', () => { - // Track observer creation - const observeMock = jest.fn(); - const disconnectMock = jest.fn(); + wrapper.setProps({isRHSOpen: false}); + expect(instance.checkTextOverflow).toBeCalledTimes(4); - // Mock ResizeObserver - window.ResizeObserver = jest.fn().mockImplementation(() => ({ - observe: observeMock, - disconnect: disconnectMock, - })) as unknown as typeof ResizeObserver; + wrapper.setProps({text: 'text change'}); + expect(instance.checkTextOverflow).toBeCalledTimes(5); - // Render component - renderWithContext({children}); + wrapper.setProps({text: 'text another change'}); + expect(instance.checkTextOverflow).toBeCalledTimes(6); - // 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; - } - }); + wrapper.setProps({checkOverflow: 1}); + expect(instance.checkTextOverflow).toBeCalledTimes(7); }); }); 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 1585c79395..bf5c5b067d 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,6 +13,8 @@ type Props = { children?: React.ReactNode; checkOverflow?: number; isAttachmentText?: boolean; + isRHSExpanded: boolean; + isRHSOpen: boolean; text?: string; compactDisplay: boolean; overflowType?: AttachmentTextOverflowType; @@ -28,7 +30,6 @@ 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); @@ -41,17 +42,16 @@ export default class ShowMore extends React.PureComponent { } componentDidMount() { - this.setupResizeObserver(); - - // Initial check for overflow this.checkTextOverflow(); + + window.addEventListener('resize', this.handleResize); } 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,39 +59,12 @@ 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(); @@ -121,6 +94,10 @@ export default class ShowMore extends React.PureComponent { }); }; + handleResize = () => { + this.checkTextOverflow(); + }; + render() { const { isCollapsed,