From 4c8aa82f1b2392addea906b7ef999200f9cfe69a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tao=20Bojl=C3=A9n?= Date: Tue, 28 Jul 2026 15:38:27 +0100 Subject: [PATCH] fix(mrt): collapse long text fields with Read more (#870) (#903) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(mrt): add memoized CollapsibleText component (#870) Co-Authored-By: pi * feat(mrt): collapse long STRING fields with Read more (#870) Co-Authored-By: pi * fix(mrt): drop horizontal scroll for text containers (#870) Co-Authored-By: pi * fix(mrt): remove page-wide horizontal scroll from review view (#870) Co-Authored-By: pi * test(mrt): strengthen CollapsibleText grapheme test, fix stale comment (#870) Co-Authored-By: pi * fix(mrt): allow string field flex item to shrink for text wrapping (#870) The FieldComponent wrapper sat inside FieldsComponent's flex flex-wrap container with the default min-width: auto, so a flex item containing a long unbroken string expanded to the string's intrinsic width instead of wrapping. break-words and WebkitLineClamp only take effect when the element has a bounded content width, so the CollapsibleText Read more toggle appeared but did nothing on huge unbroken tokens. Adding min-w-0 lets the flex item shrink below its content width so wrapping and the line clamp take effect. Co-Authored-By: pi * fix(mrt): short-circuit grapheme count, reset expanded on text change (#870) Address review feedback on CollapsibleText: - countGraphemes → exceedsGraphemeThreshold: stop iterating once the count is known to exceed maxGraphemes, so a 1MB string segments at most maxGraphemes+1 graphemes instead of all of them. - Reset the expanded state when the text prop changes. When navigating between review jobs that reuse the same field name, React reuses the CollapsibleText instance; without the reset, a newly loaded long value inherited the previous job's expanded state. - The custom-thresholds test now asserts the maxLines prop lands on the clamped div's WebkitLineClamp style, independently verifying the line-count constraint (previously maxLines was passed but untested). - New test: expanding one long value then rerendering with a different long value restores the collapsed state. Co-Authored-By: pi * test(mrt): strengthen collapsible text thresholds (#870) Co-Authored-By: pi --- .../v2/ManualReviewJobContentView.tsx | 2 +- .../v2/ManualReviewJobFieldsComponent.tsx | 26 ++-- .../v2/components/CollapsibleText.test.tsx | 114 ++++++++++++++++++ .../v2/components/CollapsibleText.tsx | 89 ++++++++++++++ 4 files changed, 223 insertions(+), 8 deletions(-) create mode 100644 client/src/webpages/dashboard/mrt/manual_review_job/v2/components/CollapsibleText.test.tsx create mode 100644 client/src/webpages/dashboard/mrt/manual_review_job/v2/components/CollapsibleText.tsx diff --git a/client/src/webpages/dashboard/mrt/manual_review_job/v2/ManualReviewJobContentView.tsx b/client/src/webpages/dashboard/mrt/manual_review_job/v2/ManualReviewJobContentView.tsx index dc88bd8..1c500b8 100644 --- a/client/src/webpages/dashboard/mrt/manual_review_job/v2/ManualReviewJobContentView.tsx +++ b/client/src/webpages/dashboard/mrt/manual_review_job/v2/ManualReviewJobContentView.tsx @@ -131,7 +131,7 @@ export default function ManualReviewJobContentView(props: { ); return ( -
+
{/* Split the data into two columns: non-media fields and media fields*/}
diff --git a/client/src/webpages/dashboard/mrt/manual_review_job/v2/ManualReviewJobFieldsComponent.tsx b/client/src/webpages/dashboard/mrt/manual_review_job/v2/ManualReviewJobFieldsComponent.tsx index 347011a..ef1bd1f 100644 --- a/client/src/webpages/dashboard/mrt/manual_review_job/v2/ManualReviewJobFieldsComponent.tsx +++ b/client/src/webpages/dashboard/mrt/manual_review_job/v2/ManualReviewJobFieldsComponent.tsx @@ -14,6 +14,7 @@ import ReactAudioPlayer from 'react-audio-player'; import { Link } from 'react-router-dom'; import ComponentLoading from '../../../../../components/common/ComponentLoading'; +import CollapsibleText from '@/webpages/dashboard/mrt/manual_review_job/v2/components/CollapsibleText'; import { GQLContentItem, @@ -190,12 +191,23 @@ function TableRowComponent(props: {
); } + case 'STRING': { + return ( +
+ {label ? ( +
+ {label} +
+ ) : null} + +
+ ); + } case 'BOOLEAN': case 'GEOHASH': case 'ID': case 'NUMBER': case 'POLICY_ID': - case 'STRING': case 'EMAIL_ADDRESS': { // EMAIL_ADDRESS renders as plain text for now; a follow-up could make // it a mailto/pivot link the way IP_ADDRESS pivots on the IP. @@ -528,7 +540,7 @@ function FieldComponent(props: { case 'EMAIL_ADDRESS': case 'DATETIME': return ( -
+
{!hideLabels ? (
@@ -646,7 +658,7 @@ function ContainerComponent(props: { type: data.container!.valueScalarType, }; return ( -
+
{/*Talk to ethan about how to avoid casting here*/} ) : null}
diff --git a/client/src/webpages/dashboard/mrt/manual_review_job/v2/components/CollapsibleText.test.tsx b/client/src/webpages/dashboard/mrt/manual_review_job/v2/components/CollapsibleText.test.tsx new file mode 100644 index 0000000..94a54d2 --- /dev/null +++ b/client/src/webpages/dashboard/mrt/manual_review_job/v2/components/CollapsibleText.test.tsx @@ -0,0 +1,114 @@ +import { fireEvent, render, screen } from '@testing-library/react'; +import React from 'react'; + +import '@testing-library/jest-dom/extend-expect'; + +import CollapsibleText from '@/webpages/dashboard/mrt/manual_review_job/v2/components/CollapsibleText'; + +describe('CollapsibleText', () => { + it('renders short text in full without a Read more button', () => { + render(); + expect(screen.getByText('hello world')).toBeInTheDocument(); + expect(screen.queryByRole('button')).not.toBeInTheDocument(); + }); + + it('collapses text exceeding maxGraphemes and shows Read more', () => { + const longText = 'a'.repeat(2001); + render(); + // The full text is in the DOM (CSS line-clamp hides overflow visually, not in the DOM). + expect(screen.getByText(longText)).toBeInTheDocument(); + expect( + screen.getByRole('button', { name: /read more/i }), + ).toBeInTheDocument(); + }); + + it('expands to full text and toggles to Read less on click', () => { + const longText = 'a'.repeat(2001); + render(); + const moreButton = screen.getByRole('button', { name: /read more/i }); + fireEvent.click(moreButton); + expect( + screen.getByRole('button', { name: /read less/i }), + ).toBeInTheDocument(); + // Full text is now rendered (not clamped). + expect(screen.getByText(longText)).toBeInTheDocument(); + }); + + it('collapses again on Read less click', () => { + const longText = 'a'.repeat(2001); + render(); + fireEvent.click(screen.getByRole('button', { name: /read more/i })); + fireEvent.click(screen.getByRole('button', { name: /read less/i })); + expect( + screen.getByRole('button', { name: /read more/i }), + ).toBeInTheDocument(); + }); + + it('counts graphemes, not UTF-16 code units (emoji with skin tone)', () => { + // 👨🏿 is a single grapheme but 4 UTF-16 code units. A naive `.length` + // check would collapse at 501 such graphemes (length 2004 > 2000), but + // a correct grapheme count (501) stays under the threshold → no collapse. + const grapheme = '👨🏿'; + const underThresholdByGrapheme = grapheme.repeat(501); + expect(underThresholdByGrapheme.length).toBeGreaterThan(2000); + render(); + expect(screen.queryByRole('button')).not.toBeInTheDocument(); + + // 2001 graphemes (8016 UTF-16 code units) does exceed the grapheme + // threshold → collapses. + const overThresholdByGrapheme = grapheme.repeat(2001); + render(); + expect( + screen.getByRole('button', { name: /read more/i }), + ).toBeInTheDocument(); + }); + + it('respects a custom maxGraphemes threshold', () => { + // 11 chars, under default maxGraphemes (2000) but over custom maxGraphemes (10). + render(); + expect( + screen.getByRole('button', { name: /read more/i }), + ).toBeInTheDocument(); + }); + + it('respects a custom maxLines threshold for wrapping text', () => { + const wrappingText = 'wrap '.repeat(20).trim(); + const { container, rerender } = render( +
+ +
, + ); + const clampedDiv = container.querySelector( + 'div[style*="-webkit-box"]', + ) as HTMLElement; + expect(clampedDiv.style.webkitLineClamp).toBe('2'); + + rerender( +
+ +
, + ); + expect(clampedDiv.style.webkitLineClamp).toBe('3'); + }); + + it('does not collapse text at exactly maxGraphemes', () => { + render(); + expect(screen.queryByRole('button')).not.toBeInTheDocument(); + }); + + it('resets to collapsed when the text prop changes', () => { + // Simulates navigating between review jobs that reuse the same field name + // (React reuses the CollapsibleText instance). Expanding one long value, + //then rendering a different long value, must restore the collapsed state. + const { rerender } = render(); + fireEvent.click(screen.getByRole('button', { name: /read more/i })); + expect( + screen.getByRole('button', { name: /read less/i }), + ).toBeInTheDocument(); + + rerender(); + expect( + screen.getByRole('button', { name: /read more/i }), + ).toBeInTheDocument(); + }); +}); diff --git a/client/src/webpages/dashboard/mrt/manual_review_job/v2/components/CollapsibleText.tsx b/client/src/webpages/dashboard/mrt/manual_review_job/v2/components/CollapsibleText.tsx new file mode 100644 index 0000000..4046528 --- /dev/null +++ b/client/src/webpages/dashboard/mrt/manual_review_job/v2/components/CollapsibleText.tsx @@ -0,0 +1,89 @@ +import { memo, useEffect, useMemo, useState } from 'react'; + +type CollapsibleTextProps = { + text: string; + maxLines?: number; + maxGraphemes?: number; +}; + +/** + * Count graphemes using Intl.Segmenter (handles all scripts correctly), + * stopping early once the count is known to exceed `threshold`. This avoids + * segmenting an entire pathological paste (e.g. a 1MB string) when we only + * need to know whether it crosses the collapse threshold. + */ +function exceedsGraphemeThreshold(text: string, threshold: number): boolean { + const segmenter = new Intl.Segmenter(undefined, { granularity: 'grapheme' }); + let count = 0; + for (const _ of segmenter.segment(text)) { + count += 1; + if (count > threshold) { + return true; + } + } + return false; +} + +/** + * Renders text with wrapping. When the text exceeds `maxGraphemes`, it is + * collapsed to `maxLines` visible lines with a "Read more" / "Read less" + * toggle. Short text renders in full with no toggle. + * + * Memoized: props are primitives (string, number), so React.memo's shallow + * comparison prevents re-renders (and re-segmentation) when a parent re-renders + * but the text hasn't changed — important for thread histories with many copies + * of the same message. + */ +function CollapsibleTextImpl({ + text, + maxLines = 12, + maxGraphemes = 2000, +}: CollapsibleTextProps) { + const isCollapsible = useMemo( + () => exceedsGraphemeThreshold(text, maxGraphemes), + [text, maxGraphemes], + ); + const [expanded, setExpanded] = useState(false); + + // When the text prop changes (e.g. navigating between review jobs that reuse + // the same field name → React reuses this component instance), reset to the + // collapsed state so newly loaded long content doesn't inherit the previous + // job's expanded view. + useEffect(() => { + setExpanded(false); + }, [text]); + + if (!isCollapsible) { + return ( +
{text}
+ ); + } + + return ( +
+
+ {text} +
+ +
+ ); +} + +const CollapsibleText = memo(CollapsibleTextImpl); +export default CollapsibleText; -- 2.51.2