From e82d44772a3fa2673c14ac676f63e2f89ed10bc7 Mon Sep 17 00:00:00 2001 From: Anandaroop Roy Date: Tue, 28 Jul 2026 14:57:58 -0400 Subject: [PATCH 1/8] feat: add color prop to HTML component Needed because for *unlisted* works, the artist bio card is rendered with inverted colors, and this needs to work correctly regardless of theme. --- src/app/Components/HTML.tsx | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/app/Components/HTML.tsx b/src/app/Components/HTML.tsx index 6fabe2920c4..9a2a27d6d51 100644 --- a/src/app/Components/HTML.tsx +++ b/src/app/Components/HTML.tsx @@ -1,4 +1,5 @@ import { + Color, Flex, FlexProps, TextProps, @@ -15,6 +16,7 @@ import RenderHtml, { CustomBlockRenderer, MixedStyleRecord } from "react-native- interface HTMLProps extends FlexProps { html: string + color?: Color onLinkPress?: (href: string) => void tagStyles?: MixedStyleRecord variant?: TextProps["variant"] @@ -29,6 +31,7 @@ export const FONTS = { export const HTML: React.FC = ({ html, + color: textColor = "mono100", onLinkPress, tagStyles = {}, variant = "sm", @@ -71,25 +74,25 @@ export const HTML: React.FC = ({ { a: { textDecorationLine: "underline", - textDecorationColor: color("mono100"), - color: color("mono100"), + textDecorationColor: color(textColor), + color: color(textColor), textDecorationStyle: "solid", }, p: { fontFamily: FONTS.regular, - color: color("mono100"), + color: color(textColor), marginBottom: "1em", ...variantStyles, }, li: { fontFamily: FONTS.regular, - color: color("mono100"), + color: color(textColor), marginBottom: "1em", ...omit(variantStyles, "lineHeight"), // to prevent mis-aligned markers }, em: { fontFamily: FONTS.italic, - color: color("mono100"), + color: color(textColor), }, h1: { fontFamily: FONTS.medium, @@ -98,7 +101,7 @@ export const HTML: React.FC = ({ letterSpacing: theme.textTreatments["xl"].letterSpacing, marginBottom: "1em", fontWeight: "normal", - color: color("mono100"), + color: color(textColor), }, h2: { fontFamily: FONTS.medium, @@ -107,7 +110,7 @@ export const HTML: React.FC = ({ letterSpacing: theme.textTreatments["lg-display"].letterSpacing, marginBottom: "1em", fontWeight: "normal", - color: color("mono100"), + color: color(textColor), }, h3: { fontFamily: FONTS.regular, @@ -116,7 +119,7 @@ export const HTML: React.FC = ({ letterSpacing: theme.textTreatments["md"].letterSpacing, marginBottom: "1em", fontWeight: "normal", - color: color("mono100"), + color: color(textColor), }, }, tagStyles From 9949616b21e2013d341afb50486f0a6022012492 Mon Sep 17 00:00:00 2001 From: Anandaroop Roy Date: Tue, 28 Jul 2026 15:03:40 -0400 Subject: [PATCH 2/8] fix: bring Artwork screen's AboutArtist rendering in line with Artist screen's --- .../Scenes/Artwork/Components/AboutArtist.tsx | 42 ++++++++++++++----- .../__tests__/AboutArtist.tests.tsx | 24 ++++++++++- 2 files changed, 54 insertions(+), 12 deletions(-) diff --git a/src/app/Scenes/Artwork/Components/AboutArtist.tsx b/src/app/Scenes/Artwork/Components/AboutArtist.tsx index 0834508c2c9..bd490f2e619 100644 --- a/src/app/Scenes/Artwork/Components/AboutArtist.tsx +++ b/src/app/Scenes/Artwork/Components/AboutArtist.tsx @@ -1,16 +1,21 @@ -import { Spacer, Flex, Box, Text, Join, Screen } from "@artsy/palette-mobile" +import { Spacer, Flex, Box, Text, LinkText, Join, Screen } from "@artsy/palette-mobile" import { AboutArtist_artwork$data } from "__generated__/AboutArtist_artwork.graphql" import { ArtistListItemContainer as ArtistListItem } from "app/Components/ArtistListItem" -import { ReadMore } from "app/Components/ReadMore" +import { HTML } from "app/Components/HTML" import { truncatedTextLimit } from "app/utils/hardware" import { Schema } from "app/utils/track" +import { useState } from "react" import { createFragmentContainer, graphql } from "react-relay" +import { useTracking } from "react-tracking" interface AboutArtistProps { artwork: AboutArtist_artwork$data } export const AboutArtist: React.FC = ({ artwork }) => { + const tracking = useTracking() + const [expanded, setExpanded] = useState(false) + const artists = artwork.artists || [] const hasSingleArtist = artists && artists.length === 1 @@ -29,6 +34,21 @@ export const AboutArtist: React.FC = ({ artwork }) => { const backgroundColor = artwork.isUnlisted ? "mono100" : "mono0" const textColor = artwork.isUnlisted ? "mono0" : "mono100" + const truncatedText = text?.slice(0, textLimit) + const canExpand = !!text && text.length > textLimit + + const handleExpandPress = () => { + if (!expanded) { + tracking.trackEvent({ + action_name: Schema.ActionNames.ReadMore, + action_type: Schema.ActionTypes.Tap, + context_module: Schema.ContextModules.ArtistBiography, + flow: Schema.Flow.AboutTheArtist, + }) + } + setExpanded((prev) => !prev) + } + return ( @@ -51,16 +71,16 @@ export const AboutArtist: React.FC = ({ artwork }) => { {!!hasSingleArtist && !!text && !!artwork.displayArtistBio && ( - + {!!canExpand && ( + + {expanded ? "Read Less" : "Read More"} + + )} )} @@ -73,7 +93,7 @@ export const AboutArtistFragmentContainer = createFragmentContainer(AboutArtist, displayArtistBio artists(shallow: true) { id - biographyBlurb(partnerBio: false) { + biographyBlurb(format: HTML, partnerBio: false) { text } diff --git a/src/app/Scenes/Artwork/Components/__tests__/AboutArtist.tests.tsx b/src/app/Scenes/Artwork/Components/__tests__/AboutArtist.tests.tsx index af5893f0873..9a170d13369 100644 --- a/src/app/Scenes/Artwork/Components/__tests__/AboutArtist.tests.tsx +++ b/src/app/Scenes/Artwork/Components/__tests__/AboutArtist.tests.tsx @@ -85,7 +85,7 @@ describe("AboutArtist", () => { }), }) - const readMoreLink = screen.getByText("Read more") + const readMoreLink = screen.getByText("Read More") fireEvent.press(readMoreLink) expect(mockTrackEvent).toBeCalledWith({ action_name: "readMore", @@ -94,4 +94,26 @@ describe("AboutArtist", () => { flow: "AboutTheArtist", }) }) + + it("renders HTML-formatted bio content without raw markdown markers", () => { + renderWithRelay({ + Artwork: () => ({ + isUnlisted: false, + displayArtistBio: true, + artists: [ + { + biographyBlurb: { + text: "

Key Exhibitions

  • David Hockney, The Metropolitan Museum of Art, 2017
", + }, + }, + ], + }), + }) + + expect(screen.getByText("Key Exhibitions")).toBeOnTheScreen() + expect( + screen.getByText("David Hockney, The Metropolitan Museum of Art, 2017") + ).toBeOnTheScreen() + expect(screen.queryByText(/###/)).not.toBeOnTheScreen() + }) }) From 021440f5a69613e46bada3f93ae6ef838afea12a Mon Sep 17 00:00:00 2001 From: Anandaroop Roy Date: Tue, 28 Jul 2026 16:15:34 -0400 Subject: [PATCH 3/8] fix: truncate bios on visible text length instead of raw HTML length biographyBlurb(format: HTML) returns markup, and slicing/measuring it by raw character count let tags eat the truncation budget and could cut mid-tag or mid-attribute, breaking the markup. truncateHtml now counts and cuts only on visible (non-tag) characters, applied to both AboutArtist and Biography which share this truncation pattern. Co-Authored-By: Claude Sonnet 5 --- src/app/Components/Artist/Biography.tsx | 9 ++-- .../Scenes/Artwork/Components/AboutArtist.tsx | 5 +- src/app/utils/__tests__/truncateHtml.tests.ts | 46 +++++++++++++++++++ src/app/utils/truncateHtml.ts | 43 +++++++++++++++++ 4 files changed, 96 insertions(+), 7 deletions(-) create mode 100644 src/app/utils/__tests__/truncateHtml.tests.ts create mode 100644 src/app/utils/truncateHtml.ts diff --git a/src/app/Components/Artist/Biography.tsx b/src/app/Components/Artist/Biography.tsx index 033e25f3434..8395c2d580c 100644 --- a/src/app/Components/Artist/Biography.tsx +++ b/src/app/Components/Artist/Biography.tsx @@ -1,6 +1,7 @@ import { Text } from "@artsy/palette-mobile" import { Biography_artist$key } from "__generated__/Biography_artist.graphql" import { HTML } from "app/Components/HTML" +import { truncateHtml, visibleHtmlTextLength } from "app/utils/truncateHtml" import { useState } from "react" import { graphql, useFragment } from "react-relay" @@ -22,15 +23,13 @@ export const Biography: React.FC = ({ artist, variant = "sm" }) const credit = data.biographyBlurb.credit const text = !!credit ? `${data.biographyBlurb.text} ${credit}` : data.biographyBlurb.text - const truncatedText = text.slice(0, MAX_CHARS) - const canExpand = text.length > MAX_CHARS + const canExpand = visibleHtmlTextLength(text) > MAX_CHARS + const truncatedText = truncateHtml(text, MAX_CHARS) return ( <> MAX_CHARS && !expanded ? "... " : " " - }`} + html={`${expanded ? text : truncatedText}${canExpand && !expanded ? "... " : " "}`} variant={variant} /> diff --git a/src/app/Scenes/Artwork/Components/AboutArtist.tsx b/src/app/Scenes/Artwork/Components/AboutArtist.tsx index bd490f2e619..ab94b07ac0d 100644 --- a/src/app/Scenes/Artwork/Components/AboutArtist.tsx +++ b/src/app/Scenes/Artwork/Components/AboutArtist.tsx @@ -4,6 +4,7 @@ import { ArtistListItemContainer as ArtistListItem } from "app/Components/Artist import { HTML } from "app/Components/HTML" import { truncatedTextLimit } from "app/utils/hardware" import { Schema } from "app/utils/track" +import { truncateHtml, visibleHtmlTextLength } from "app/utils/truncateHtml" import { useState } from "react" import { createFragmentContainer, graphql } from "react-relay" import { useTracking } from "react-tracking" @@ -34,8 +35,8 @@ export const AboutArtist: React.FC = ({ artwork }) => { const backgroundColor = artwork.isUnlisted ? "mono100" : "mono0" const textColor = artwork.isUnlisted ? "mono0" : "mono100" - const truncatedText = text?.slice(0, textLimit) - const canExpand = !!text && text.length > textLimit + const canExpand = !!text && visibleHtmlTextLength(text) > textLimit + const truncatedText = text ? truncateHtml(text, textLimit) : undefined const handleExpandPress = () => { if (!expanded) { diff --git a/src/app/utils/__tests__/truncateHtml.tests.ts b/src/app/utils/__tests__/truncateHtml.tests.ts new file mode 100644 index 00000000000..54d3cb06457 --- /dev/null +++ b/src/app/utils/__tests__/truncateHtml.tests.ts @@ -0,0 +1,46 @@ +import { truncateHtml, visibleHtmlTextLength } from "app/utils/truncateHtml" + +describe("visibleHtmlTextLength", () => { + it("returns the string length when there are no tags", () => { + expect(visibleHtmlTextLength("No tags here")).toEqual(12) + }) + + it("excludes tags from the count", () => { + expect(visibleHtmlTextLength("

Hello world

")).toEqual(11) + }) + + it("returns zero for an empty string", () => { + expect(visibleHtmlTextLength("")).toEqual(0) + }) +}) + +describe("truncateHtml", () => { + it("behaves like a plain slice when there is no markup", () => { + expect(truncateHtml("Hello world", 5)).toEqual("Hello") + }) + + it("returns the entire string when the budget exceeds the visible text length", () => { + const html = "

Hi

" + expect(truncateHtml(html, 100)).toEqual(html) + }) + + it("does not count tag markup toward the visible character budget", () => { + const html = "

Hello

" + expect(truncateHtml(html, 5)).toEqual("

Hello

") + }) + + it("stops as soon as the visible character budget is reached, dropping trailing markup", () => { + const html = "

Hello world

" + expect(truncateHtml(html, 5)).toEqual("

Hello") + }) + + it("never cuts in the middle of a tag or attribute", () => { + const html = + 'Hockney retrospective' + + const result = truncateHtml(html, 3) + + expect(result).toEqual('Hoc') + expect(visibleHtmlTextLength(result)).toEqual(3) + }) +}) diff --git a/src/app/utils/truncateHtml.ts b/src/app/utils/truncateHtml.ts new file mode 100644 index 00000000000..c7bd6bb8798 --- /dev/null +++ b/src/app/utils/truncateHtml.ts @@ -0,0 +1,43 @@ +/** + * Strips tags to measure the length of the text a user would actually see. + */ +export function visibleHtmlTextLength(html: string): number { + return html.replace(/<[^>]*>/g, "").length +} + +/** + * Truncates an HTML string to at most `maxVisibleChars` of visible (non-tag) text, + * without ever cutting in the middle of a tag or attribute. + */ +export function truncateHtml(html: string, maxVisibleChars: number): string { + let result = "" + let visibleCount = 0 + let inTag = false + + for (let i = 0; i < html.length; i++) { + const char = html[i] + + if (char === "<") { + inTag = true + result += char + continue + } + + if (inTag) { + result += char + if (char === ">") { + inTag = false + } + continue + } + + if (visibleCount >= maxVisibleChars) { + break + } + + result += char + visibleCount++ + } + + return result +} From 6af6ea201fb9e7b9df3c884a93d019fe5698b7ad Mon Sep 17 00:00:00 2001 From: Anandaroop Roy Date: Tue, 28 Jul 2026 16:24:34 -0400 Subject: [PATCH 4/8] fix(a11y): mark Read More/Less as a button for screen readers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Read More/Less toggle used palette-mobile's LinkText, which defaults accessibilityRole to "link" — misleading for a toggle that doesn't navigate. ReadMore set this explicitly to "button"; restore that here. Co-Authored-By: Claude Sonnet 5 --- src/app/Scenes/Artwork/Components/AboutArtist.tsx | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/app/Scenes/Artwork/Components/AboutArtist.tsx b/src/app/Scenes/Artwork/Components/AboutArtist.tsx index ab94b07ac0d..4fde2aa14fe 100644 --- a/src/app/Scenes/Artwork/Components/AboutArtist.tsx +++ b/src/app/Scenes/Artwork/Components/AboutArtist.tsx @@ -78,7 +78,12 @@ export const AboutArtist: React.FC = ({ artwork }) => { variant="sm" /> {!!canExpand && ( - + {expanded ? "Read Less" : "Read More"} )} From 19c9b9ef8a383572d06d038a3228199ebc7d49e6 Mon Sep 17 00:00:00 2001 From: Anandaroop Roy Date: Tue, 28 Jul 2026 16:24:43 -0400 Subject: [PATCH 5/8] test: fix assertion in AboutArtist HTML-formatting test --- .../Scenes/Artwork/Components/__tests__/AboutArtist.tests.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/Scenes/Artwork/Components/__tests__/AboutArtist.tests.tsx b/src/app/Scenes/Artwork/Components/__tests__/AboutArtist.tests.tsx index 9a170d13369..7a59872488c 100644 --- a/src/app/Scenes/Artwork/Components/__tests__/AboutArtist.tests.tsx +++ b/src/app/Scenes/Artwork/Components/__tests__/AboutArtist.tests.tsx @@ -114,6 +114,6 @@ describe("AboutArtist", () => { expect( screen.getByText("David Hockney, The Metropolitan Museum of Art, 2017") ).toBeOnTheScreen() - expect(screen.queryByText(/###/)).not.toBeOnTheScreen() + expect(screen.queryByText(/

/)).not.toBeOnTheScreen() }) }) From de8f588031659074f09b1478909f80111be1dfd4 Mon Sep 17 00:00:00 2001 From: Anandaroop Roy Date: Tue, 28 Jul 2026 16:42:37 -0400 Subject: [PATCH 6/8] fix: handle HTML entities and avoid dangling tags in truncateHtml MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two issues from PR review: the char-by-char scanner counted entity references (e.g. "&") one character at a time, so a cut could land mid-entity and a valid entity's raw length inflated the visible budget. Separately, once the budget was spent the scanner kept consuming any following tag — including a new opening tag like

— leaving it dangling open when the caller appends "... " right after it. Fix: treat a full "&name;"/"{"/"" run as one visible character in both truncateHtml and visibleHtmlTextLength, and once the budget is spent, only continue consuming closing tags (to finish elements already open) rather than any tag. Co-Authored-By: Claude Sonnet 5 --- src/app/utils/__tests__/truncateHtml.tests.ts | 29 ++++++++++++++ src/app/utils/truncateHtml.ts | 39 +++++++++++++++---- 2 files changed, 61 insertions(+), 7 deletions(-) diff --git a/src/app/utils/__tests__/truncateHtml.tests.ts b/src/app/utils/__tests__/truncateHtml.tests.ts index 54d3cb06457..aae8b7f617b 100644 --- a/src/app/utils/__tests__/truncateHtml.tests.ts +++ b/src/app/utils/__tests__/truncateHtml.tests.ts @@ -12,6 +12,14 @@ describe("visibleHtmlTextLength", () => { it("returns zero for an empty string", () => { expect(visibleHtmlTextLength("")).toEqual(0) }) + + it("counts a named entity reference as a single visible character", () => { + expect(visibleHtmlTextLength("Hauser & Wirth")).toEqual(14) + }) + + it("counts a numeric entity reference as a single visible character", () => { + expect(visibleHtmlTextLength("Rock & Roll")).toEqual(11) + }) }) describe("truncateHtml", () => { @@ -43,4 +51,25 @@ describe("truncateHtml", () => { expect(result).toEqual('Hoc') expect(visibleHtmlTextLength(result)).toEqual(3) }) + + it("does not cut in the middle of an entity reference", () => { + const result = truncateHtml("Arts & Crafts", 7) + + expect(result).toEqual("Arts & ") + expect(visibleHtmlTextLength(result)).toEqual(7) + }) + + it("treats a bare ampersand that is not part of an entity as a normal character", () => { + expect(truncateHtml("Fish & Chips", 6)).toEqual("Fish &") + }) + + it("stops before a new tag opens once the budget is spent, without leaving it dangling open", () => { + const html = "

abc

Key Exhibitions

" + expect(truncateHtml(html, 3)).toEqual("

abc

") + }) + + it("still emits closing tags for elements opened before the budget was spent", () => { + const html = "

abc

" + expect(truncateHtml(html, 3)).toEqual(html) + }) }) diff --git a/src/app/utils/truncateHtml.ts b/src/app/utils/truncateHtml.ts index c7bd6bb8798..ab6dabfad5c 100644 --- a/src/app/utils/truncateHtml.ts +++ b/src/app/utils/truncateHtml.ts @@ -1,13 +1,21 @@ +const ENTITY_PATTERN = "&(#\\d+|#x[0-9a-fA-F]+|[a-zA-Z][a-zA-Z0-9]*);" +const ENTITY_REGEX = new RegExp(`^${ENTITY_PATTERN}`) +const ENTITY_REGEX_GLOBAL = new RegExp(ENTITY_PATTERN, "g") + /** * Strips tags to measure the length of the text a user would actually see. + * Entity references (e.g. `&`) count as a single character, matching how + * they render. */ export function visibleHtmlTextLength(html: string): number { - return html.replace(/<[^>]*>/g, "").length + return html.replace(/<[^>]*>/g, "").replace(ENTITY_REGEX_GLOBAL, "&").length } /** * Truncates an HTML string to at most `maxVisibleChars` of visible (non-tag) text, - * without ever cutting in the middle of a tag or attribute. + * without ever cutting in the middle of a tag, an attribute, or an entity reference + * (e.g. `&`). Once the budget is spent, still emits any tags needed to close + * elements that are already open, but stops before a new tag would open. */ export function truncateHtml(html: string, maxVisibleChars: number): string { let result = "" @@ -17,18 +25,35 @@ export function truncateHtml(html: string, maxVisibleChars: number): string { for (let i = 0; i < html.length; i++) { const char = html[i] + if (inTag) { + result += char + if (char === ">") { + inTag = false + } + continue + } + if (char === "<") { + const isClosingTag = html[i + 1] === "/" + if (visibleCount >= maxVisibleChars && !isClosingTag) { + break + } inTag = true result += char continue } - if (inTag) { - result += char - if (char === ">") { - inTag = false + if (char === "&") { + const entityMatch = ENTITY_REGEX.exec(html.slice(i)) + if (entityMatch) { + if (visibleCount >= maxVisibleChars) { + break + } + result += entityMatch[0] + visibleCount++ + i += entityMatch[0].length - 1 + continue } - continue } if (visibleCount >= maxVisibleChars) { From 239df271f5c31bbff6374363c5ba8a7271d8b7d9 Mon Sep 17 00:00:00 2001 From: Anandaroop Roy Date: Tue, 28 Jul 2026 17:00:19 -0400 Subject: [PATCH 7/8] fix: set baseStyle so HTML's color prop reaches unstyled tags too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The color prop only reached text through tagsStyles entries for a/p/li/em/h1/h2/h3. Anything outside those — bare root text, or tags like strong/blockquote/ol with no explicit entry — kept react-native-render-html's default color, rendering dark-on-dark on the inverted (unlisted) card. This is reachable today: truncateHtml can stop right after a closing block tag, and AboutArtist appends "... " outside any element at that point. baseStyle applies to the whole render tree, closing the gap for every tag at once instead of enumerating more tagsStyles entries. Co-Authored-By: Claude Sonnet 5 --- src/app/Components/HTML.tsx | 1 + src/app/Components/__tests__/HTML.tests.tsx | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+) create mode 100644 src/app/Components/__tests__/HTML.tests.tsx diff --git a/src/app/Components/HTML.tsx b/src/app/Components/HTML.tsx index 9a2a27d6d51..8ad7a6e42a1 100644 --- a/src/app/Components/HTML.tsx +++ b/src/app/Components/HTML.tsx @@ -50,6 +50,7 @@ export const HTML: React.FC = ({ { + it("applies the color prop to bare text with no wrapping tag", () => { + renderWithWrappers() + + const node = screen.getByText("Bare text with no tag") + expect(node).toHaveStyle({ color: "#FFFFFF" }) + }) + + it("applies the color prop to tags without an explicit tagsStyles entry", () => { + renderWithWrappers() + + const node = screen.getByText("Bold text") + expect(node).toHaveStyle({ color: "#FFFFFF" }) + }) +}) From 068cd1334d330c81305ebfdda2a7faf44ff2016c Mon Sep 17 00:00:00 2001 From: Anandaroop Roy Date: Tue, 28 Jul 2026 17:18:12 -0400 Subject: [PATCH 8/8] refactor: unify truncateHtml and visibleHtmlTextLength into one scan MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit They answered the same "what counts as visible" question two different ways — one by regex-stripping tags, the other by walking characters with an inTag flag — and could disagree on malformed input (e.g. an unterminated "<" tag), since only one of the two strips it. Both callers then risked "canExpand: true" alongside a truncatedText that was already the full string, making Read More a no-op. truncateHtml now returns { text, wasTruncated } from a single pass, so the two can no longer drift apart by construction. Simplifies both call sites, which no longer need two separate function calls. Co-Authored-By: Claude Sonnet 5 --- src/app/Components/Artist/Biography.tsx | 5 +- .../Scenes/Artwork/Components/AboutArtist.tsx | 7 +- src/app/utils/__tests__/truncateHtml.tests.ts | 79 ++++++++++--------- src/app/utils/truncateHtml.ts | 22 +++--- 4 files changed, 60 insertions(+), 53 deletions(-) diff --git a/src/app/Components/Artist/Biography.tsx b/src/app/Components/Artist/Biography.tsx index 8395c2d580c..48364160733 100644 --- a/src/app/Components/Artist/Biography.tsx +++ b/src/app/Components/Artist/Biography.tsx @@ -1,7 +1,7 @@ import { Text } from "@artsy/palette-mobile" import { Biography_artist$key } from "__generated__/Biography_artist.graphql" import { HTML } from "app/Components/HTML" -import { truncateHtml, visibleHtmlTextLength } from "app/utils/truncateHtml" +import { truncateHtml } from "app/utils/truncateHtml" import { useState } from "react" import { graphql, useFragment } from "react-relay" @@ -23,8 +23,7 @@ export const Biography: React.FC = ({ artist, variant = "sm" }) const credit = data.biographyBlurb.credit const text = !!credit ? `${data.biographyBlurb.text} ${credit}` : data.biographyBlurb.text - const canExpand = visibleHtmlTextLength(text) > MAX_CHARS - const truncatedText = truncateHtml(text, MAX_CHARS) + const { text: truncatedText, wasTruncated: canExpand } = truncateHtml(text, MAX_CHARS) return ( <> diff --git a/src/app/Scenes/Artwork/Components/AboutArtist.tsx b/src/app/Scenes/Artwork/Components/AboutArtist.tsx index 4fde2aa14fe..66e7621bdf6 100644 --- a/src/app/Scenes/Artwork/Components/AboutArtist.tsx +++ b/src/app/Scenes/Artwork/Components/AboutArtist.tsx @@ -4,7 +4,7 @@ import { ArtistListItemContainer as ArtistListItem } from "app/Components/Artist import { HTML } from "app/Components/HTML" import { truncatedTextLimit } from "app/utils/hardware" import { Schema } from "app/utils/track" -import { truncateHtml, visibleHtmlTextLength } from "app/utils/truncateHtml" +import { truncateHtml } from "app/utils/truncateHtml" import { useState } from "react" import { createFragmentContainer, graphql } from "react-relay" import { useTracking } from "react-tracking" @@ -35,8 +35,9 @@ export const AboutArtist: React.FC = ({ artwork }) => { const backgroundColor = artwork.isUnlisted ? "mono100" : "mono0" const textColor = artwork.isUnlisted ? "mono0" : "mono100" - const canExpand = !!text && visibleHtmlTextLength(text) > textLimit - const truncatedText = text ? truncateHtml(text, textLimit) : undefined + const { text: truncatedText, wasTruncated: canExpand } = text + ? truncateHtml(text, textLimit) + : { text: undefined, wasTruncated: false } const handleExpandPress = () => { if (!expanded) { diff --git a/src/app/utils/__tests__/truncateHtml.tests.ts b/src/app/utils/__tests__/truncateHtml.tests.ts index aae8b7f617b..c4cb10ec906 100644 --- a/src/app/utils/__tests__/truncateHtml.tests.ts +++ b/src/app/utils/__tests__/truncateHtml.tests.ts @@ -1,75 +1,80 @@ -import { truncateHtml, visibleHtmlTextLength } from "app/utils/truncateHtml" - -describe("visibleHtmlTextLength", () => { - it("returns the string length when there are no tags", () => { - expect(visibleHtmlTextLength("No tags here")).toEqual(12) - }) - - it("excludes tags from the count", () => { - expect(visibleHtmlTextLength("

Hello world

")).toEqual(11) - }) - - it("returns zero for an empty string", () => { - expect(visibleHtmlTextLength("")).toEqual(0) - }) - - it("counts a named entity reference as a single visible character", () => { - expect(visibleHtmlTextLength("Hauser & Wirth")).toEqual(14) - }) - - it("counts a numeric entity reference as a single visible character", () => { - expect(visibleHtmlTextLength("Rock & Roll")).toEqual(11) - }) -}) +import { truncateHtml } from "app/utils/truncateHtml" describe("truncateHtml", () => { it("behaves like a plain slice when there is no markup", () => { - expect(truncateHtml("Hello world", 5)).toEqual("Hello") + const { text, wasTruncated } = truncateHtml("Hello world", 5) + + expect(text).toEqual("Hello") + expect(wasTruncated).toBe(true) }) - it("returns the entire string when the budget exceeds the visible text length", () => { + it("returns the entire string, untruncated, when the budget exceeds the visible text length", () => { const html = "

Hi

" - expect(truncateHtml(html, 100)).toEqual(html) + const { text, wasTruncated } = truncateHtml(html, 100) + + expect(text).toEqual(html) + expect(wasTruncated).toBe(false) }) it("does not count tag markup toward the visible character budget", () => { const html = "

Hello

" - expect(truncateHtml(html, 5)).toEqual("

Hello

") + const { text, wasTruncated } = truncateHtml(html, 5) + + expect(text).toEqual("

Hello

") + expect(wasTruncated).toBe(false) }) it("stops as soon as the visible character budget is reached, dropping trailing markup", () => { const html = "

Hello world

" - expect(truncateHtml(html, 5)).toEqual("

Hello") + const { text, wasTruncated } = truncateHtml(html, 5) + + expect(text).toEqual("

Hello") + expect(wasTruncated).toBe(true) }) it("never cuts in the middle of a tag or attribute", () => { const html = 'Hockney retrospective' - const result = truncateHtml(html, 3) + const { text, wasTruncated } = truncateHtml(html, 3) - expect(result).toEqual('Hoc') - expect(visibleHtmlTextLength(result)).toEqual(3) + expect(text).toEqual('Hoc') + expect(wasTruncated).toBe(true) }) it("does not cut in the middle of an entity reference", () => { - const result = truncateHtml("Arts & Crafts", 7) + const { text } = truncateHtml("Arts & Crafts", 7) - expect(result).toEqual("Arts & ") - expect(visibleHtmlTextLength(result)).toEqual(7) + expect(text).toEqual("Arts & ") }) it("treats a bare ampersand that is not part of an entity as a normal character", () => { - expect(truncateHtml("Fish & Chips", 6)).toEqual("Fish &") + const { text } = truncateHtml("Fish & Chips", 6) + + expect(text).toEqual("Fish &") }) it("stops before a new tag opens once the budget is spent, without leaving it dangling open", () => { const html = "

abc

Key Exhibitions

" - expect(truncateHtml(html, 3)).toEqual("

abc

") + const { text } = truncateHtml(html, 3) + + expect(text).toEqual("

abc

") }) it("still emits closing tags for elements opened before the budget was spent", () => { const html = "

abc

" - expect(truncateHtml(html, 3)).toEqual(html) + const { text } = truncateHtml(html, 3) + + expect(text).toEqual(html) + }) + + it("keeps the returned text and wasTruncated flag consistent even on malformed HTML", () => { + // An unterminated "]*>/g, "").replace(ENTITY_REGEX_GLOBAL, "&").length +export interface TruncatedHtml { + text: string + wasTruncated: boolean } /** @@ -16,11 +11,15 @@ export function visibleHtmlTextLength(html: string): number { * without ever cutting in the middle of a tag, an attribute, or an entity reference * (e.g. `&`). Once the budget is spent, still emits any tags needed to close * elements that are already open, but stops before a new tag would open. + * + * `wasTruncated` and the returned `text` come from the same scan, so callers can't + * see them disagree about whether truncation happened. */ -export function truncateHtml(html: string, maxVisibleChars: number): string { +export function truncateHtml(html: string, maxVisibleChars: number): TruncatedHtml { let result = "" let visibleCount = 0 let inTag = false + let wasTruncated = false for (let i = 0; i < html.length; i++) { const char = html[i] @@ -36,6 +35,7 @@ export function truncateHtml(html: string, maxVisibleChars: number): string { if (char === "<") { const isClosingTag = html[i + 1] === "/" if (visibleCount >= maxVisibleChars && !isClosingTag) { + wasTruncated = true break } inTag = true @@ -47,6 +47,7 @@ export function truncateHtml(html: string, maxVisibleChars: number): string { const entityMatch = ENTITY_REGEX.exec(html.slice(i)) if (entityMatch) { if (visibleCount >= maxVisibleChars) { + wasTruncated = true break } result += entityMatch[0] @@ -57,6 +58,7 @@ export function truncateHtml(html: string, maxVisibleChars: number): string { } if (visibleCount >= maxVisibleChars) { + wasTruncated = true break } @@ -64,5 +66,5 @@ export function truncateHtml(html: string, maxVisibleChars: number): string { visibleCount++ } - return result + return { text: result, wasTruncated } }