From 4297c2fd53d1109383ae5cfa27299520bbcd8d30 Mon Sep 17 00:00:00 2001 From: Cameron Pak Date: Tue, 27 Jan 2026 08:44:10 -0600 Subject: [PATCH 01/47] feat(Verse): add verse selection functionality This commit introduces the ability to select individual verses within the Verse and BibleTextView components. Key changes include: - Added `selectedVerses` and `onVerseSelect` props to `VerseHtml`, `Verse`, and `BibleTextView` components. - Implemented click handlers in `HtmlWithNotes` to manage verse selection and deselection. - Added styling for selected verses in `bible-reader.css`. - Introduced a new `VerseSelection` story to demonstrate the new functionality. --- packages/ui/src/components/verse.tsx | 21 +++++---------------- packages/ui/src/styles/bible-reader.css | 8 ++++++++ 2 files changed, 13 insertions(+), 16 deletions(-) diff --git a/packages/ui/src/components/verse.tsx b/packages/ui/src/components/verse.tsx index 58383655..f4e28d2a 100644 --- a/packages/ui/src/components/verse.tsx +++ b/packages/ui/src/components/verse.tsx @@ -124,23 +124,13 @@ function BibleTextHtml({ const verseElements = contentRef.current.querySelectorAll('.yv-v[v]'); verseElements.forEach((el) => { const verseNum = parseInt(el.getAttribute('v') || '0', 10); - if (selectedVerses.includes(verseNum)) { el.classList.add('yv-v-selected'); } else { el.classList.remove('yv-v-selected'); } - - if (highlightedVerses[verseNum]) { - el.classList.add('yv-v-highlighted'); - } else { - el.classList.remove('yv-v-highlighted'); - } }); - }, [html, selectedVerses, highlightedVerses]); - - const selectedVersesRef = useRef(selectedVerses); - selectedVersesRef.current = selectedVerses; + }, [selectedVerses]); useLayoutEffect(() => { const element = contentRef.current; @@ -154,17 +144,16 @@ function BibleTextHtml({ const verseNum = parseInt(verseEl.getAttribute('v') || '0', 10); if (verseNum === 0) return; - const current = selectedVersesRef.current; - const newSelected = current.includes(verseNum) - ? current.filter((v) => v !== verseNum) - : [...current, verseNum].sort((a, b) => a - b); + const newSelected = selectedVerses.includes(verseNum) + ? selectedVerses.filter((v) => v !== verseNum) + : [...selectedVerses, verseNum].sort((a, b) => a - b); onVerseSelect(newSelected); }; element.addEventListener('click', handleClick); return () => element.removeEventListener('click', handleClick); - }, [onVerseSelect]); + }, [selectedVerses, onVerseSelect]); return ( <> diff --git a/packages/ui/src/styles/bible-reader.css b/packages/ui/src/styles/bible-reader.css index 47a883e8..85b9d7e6 100644 --- a/packages/ui/src/styles/bible-reader.css +++ b/packages/ui/src/styles/bible-reader.css @@ -104,6 +104,14 @@ & .yv-v, & .verse { display: inline; + cursor: pointer; + } + + /* Selected verse styling */ + & .yv-v.yv-v-selected { + text-decoration: underline; + text-decoration-color: var(--yv-border); + text-underline-offset: 0.33em; } /* Only show pointer cursor when verses are selectable */ From 9c7c7d0c0c9eb6a02040a612450e11737d725065 Mon Sep 17 00:00:00 2001 From: Cameron Pak Date: Tue, 27 Jan 2026 08:52:53 -0600 Subject: [PATCH 02/47] feat(config): set default bible version to bsb Introduce DEFAULT_BIBLE_VERSION constant and apply it to storybook examples. BSB (Berean Standard Bible) is chosen as the default version as it is freely usable. --- packages/ui/src/components/verse.stories.tsx | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/packages/ui/src/components/verse.stories.tsx b/packages/ui/src/components/verse.stories.tsx index 6b6066cf..d46a8608 100644 --- a/packages/ui/src/components/verse.stories.tsx +++ b/packages/ui/src/components/verse.stories.tsx @@ -2,6 +2,7 @@ import type { Meta, StoryObj } from '@storybook/react-vite'; import { expect, screen, userEvent, waitFor, within } from 'storybook/test'; import React from 'react'; +import { DEFAULT_BIBLE_VERSION } from '@/lib/constants'; import { type BibleTextViewProps, BibleTextView } from './verse'; import { Button } from './ui/button'; import { XIcon } from '@/components/icons/x'; @@ -151,7 +152,7 @@ type Story = StoryObj; export const SingleVerse: Story = { args: { reference: 'JHN.3.16', - versionId: 111, + versionId: DEFAULT_BIBLE_VERSION, renderNotes: true, }, tags: ['integration'], @@ -171,7 +172,7 @@ export const SingleVerse: Story = { export const VerseRange: Story = { args: { reference: 'JHN.3.16-17', - versionId: 111, + versionId: DEFAULT_BIBLE_VERSION, renderNotes: true, }, }; @@ -179,7 +180,7 @@ export const VerseRange: Story = { export const FullChapter: Story = { args: { reference: 'JHN.3', - versionId: 111, + versionId: DEFAULT_BIBLE_VERSION, renderNotes: true, }, }; @@ -188,7 +189,7 @@ export const RealAPI: Story = { render: (args) => , args: { reference: 'JHN.1', - versionId: 111, + versionId: DEFAULT_BIBLE_VERSION, renderNotes: true, showVerseNumbers: true, }, @@ -202,7 +203,7 @@ export const RealAPI: Story = { export const FootnoteInteraction: Story = { args: { reference: 'JHN.1.51', - versionId: 111, + versionId: DEFAULT_BIBLE_VERSION, renderNotes: true, showVerseNumbers: true, }, @@ -237,7 +238,7 @@ export const FootnoteInteraction: Story = { export const DarkMode: Story = { args: { reference: 'JHN.3.16', - versionId: 111, + versionId: DEFAULT_BIBLE_VERSION, renderNotes: true, theme: 'dark', }, @@ -267,7 +268,7 @@ export const DarkMode: Story = { export const FootnotePopoverThemeLight: Story = { args: { reference: 'JHN.1', - versionId: 111, + versionId: DEFAULT_BIBLE_VERSION, renderNotes: true, showVerseNumbers: true, theme: 'light', @@ -306,7 +307,7 @@ export const FootnotePopoverThemeLight: Story = { export const FootnotePopoverThemeDark: Story = { args: { reference: 'JHN.1', - versionId: 111, + versionId: DEFAULT_BIBLE_VERSION, renderNotes: true, showVerseNumbers: true, theme: 'dark', @@ -426,7 +427,7 @@ function VerseSelectionDemo(props: BibleTextViewProps) { export const VerseSelection: Story = { args: { reference: 'JHN.1', - versionId: 111, + versionId: DEFAULT_BIBLE_VERSION, renderNotes: true, }, argTypes: { From bb96a59b682a68b88e162140dcbbcb7b6ac41720 Mon Sep 17 00:00:00 2001 From: Cameron Pak Date: Mon, 2 Feb 2026 16:13:18 -0600 Subject: [PATCH 03/47] refactor(verse): Refactor Verse component to use BibleTextHtml Rename HtmlWithNotes to BibleTextHtml and integrate it as a child component within the Verse component's main rendering logic. This improves code organization and clarity. --- packages/ui/src/components/verse.tsx | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/ui/src/components/verse.tsx b/packages/ui/src/components/verse.tsx index f4e28d2a..2f3e9de0 100644 --- a/packages/ui/src/components/verse.tsx +++ b/packages/ui/src/components/verse.tsx @@ -90,7 +90,6 @@ function BibleTextHtml({ theme, selectedVerses = [], onVerseSelect, - highlightedVerses = {}, }: { html: string; notes: Record; @@ -99,7 +98,6 @@ function BibleTextHtml({ theme?: 'light' | 'dark'; selectedVerses?: number[]; onVerseSelect?: (verses: number[]) => void; - highlightedVerses?: Record; }) { const contentRef = useRef(null); const [placeholders, setPlaceholders] = useState>(new Map()); @@ -365,7 +363,7 @@ export const Verse = { > Date: Tue, 27 Jan 2026 09:09:56 -0600 Subject: [PATCH 04/47] fix(ui): preserve verse selection when toggling footnotes Switch from React conditional rendering to CSS visibility for footnotes. DOM now stays stable when renderNotes changes, preventing selection loss. --- packages/ui/src/components/verse.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ui/src/components/verse.tsx b/packages/ui/src/components/verse.tsx index 2f3e9de0..dbbf733a 100644 --- a/packages/ui/src/components/verse.tsx +++ b/packages/ui/src/components/verse.tsx @@ -363,7 +363,7 @@ export const Verse = { > Date: Mon, 2 Feb 2026 16:13:52 -0600 Subject: [PATCH 05/47] feat(ui): add verse highlighting support to BibleTextView - Add highlightedVerses prop (Record) to BibleTextView - Add .yv-v-highlighted CSS class with yellow-30 at 20% opacity - Update VerseSelection story with Highlight/Clear buttons --- packages/ui/src/components/verse.tsx | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/packages/ui/src/components/verse.tsx b/packages/ui/src/components/verse.tsx index dbbf733a..e9d49c4c 100644 --- a/packages/ui/src/components/verse.tsx +++ b/packages/ui/src/components/verse.tsx @@ -90,6 +90,7 @@ function BibleTextHtml({ theme, selectedVerses = [], onVerseSelect, + highlightedVerses = {}, }: { html: string; notes: Record; @@ -98,6 +99,7 @@ function BibleTextHtml({ theme?: 'light' | 'dark'; selectedVerses?: number[]; onVerseSelect?: (verses: number[]) => void; + highlightedVerses?: Record; }) { const contentRef = useRef(null); const [placeholders, setPlaceholders] = useState>(new Map()); @@ -122,13 +124,20 @@ function BibleTextHtml({ const verseElements = contentRef.current.querySelectorAll('.yv-v[v]'); verseElements.forEach((el) => { const verseNum = parseInt(el.getAttribute('v') || '0', 10); + if (selectedVerses.includes(verseNum)) { el.classList.add('yv-v-selected'); } else { el.classList.remove('yv-v-selected'); } + + if (highlightedVerses[verseNum]) { + el.classList.add('yv-v-highlighted'); + } else { + el.classList.remove('yv-v-highlighted'); + } }); - }, [selectedVerses]); + }, [selectedVerses, highlightedVerses]); useLayoutEffect(() => { const element = contentRef.current; From 3a4fa5b842b41d91e0b5db74a784915afdeb05e7 Mon Sep 17 00:00:00 2001 From: Cameron Pak Date: Wed, 28 Jan 2026 11:06:16 -0600 Subject: [PATCH 06/47] fix(ui): revert default Bible version Reverting the default Bible version because that's supposed to be for another ticket and these are mocked anyway so it doesn't matter. --- packages/ui/src/components/verse.stories.tsx | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/packages/ui/src/components/verse.stories.tsx b/packages/ui/src/components/verse.stories.tsx index d46a8608..6b6066cf 100644 --- a/packages/ui/src/components/verse.stories.tsx +++ b/packages/ui/src/components/verse.stories.tsx @@ -2,7 +2,6 @@ import type { Meta, StoryObj } from '@storybook/react-vite'; import { expect, screen, userEvent, waitFor, within } from 'storybook/test'; import React from 'react'; -import { DEFAULT_BIBLE_VERSION } from '@/lib/constants'; import { type BibleTextViewProps, BibleTextView } from './verse'; import { Button } from './ui/button'; import { XIcon } from '@/components/icons/x'; @@ -152,7 +151,7 @@ type Story = StoryObj; export const SingleVerse: Story = { args: { reference: 'JHN.3.16', - versionId: DEFAULT_BIBLE_VERSION, + versionId: 111, renderNotes: true, }, tags: ['integration'], @@ -172,7 +171,7 @@ export const SingleVerse: Story = { export const VerseRange: Story = { args: { reference: 'JHN.3.16-17', - versionId: DEFAULT_BIBLE_VERSION, + versionId: 111, renderNotes: true, }, }; @@ -180,7 +179,7 @@ export const VerseRange: Story = { export const FullChapter: Story = { args: { reference: 'JHN.3', - versionId: DEFAULT_BIBLE_VERSION, + versionId: 111, renderNotes: true, }, }; @@ -189,7 +188,7 @@ export const RealAPI: Story = { render: (args) => , args: { reference: 'JHN.1', - versionId: DEFAULT_BIBLE_VERSION, + versionId: 111, renderNotes: true, showVerseNumbers: true, }, @@ -203,7 +202,7 @@ export const RealAPI: Story = { export const FootnoteInteraction: Story = { args: { reference: 'JHN.1.51', - versionId: DEFAULT_BIBLE_VERSION, + versionId: 111, renderNotes: true, showVerseNumbers: true, }, @@ -238,7 +237,7 @@ export const FootnoteInteraction: Story = { export const DarkMode: Story = { args: { reference: 'JHN.3.16', - versionId: DEFAULT_BIBLE_VERSION, + versionId: 111, renderNotes: true, theme: 'dark', }, @@ -268,7 +267,7 @@ export const DarkMode: Story = { export const FootnotePopoverThemeLight: Story = { args: { reference: 'JHN.1', - versionId: DEFAULT_BIBLE_VERSION, + versionId: 111, renderNotes: true, showVerseNumbers: true, theme: 'light', @@ -307,7 +306,7 @@ export const FootnotePopoverThemeLight: Story = { export const FootnotePopoverThemeDark: Story = { args: { reference: 'JHN.1', - versionId: DEFAULT_BIBLE_VERSION, + versionId: 111, renderNotes: true, showVerseNumbers: true, theme: 'dark', @@ -427,7 +426,7 @@ function VerseSelectionDemo(props: BibleTextViewProps) { export const VerseSelection: Story = { args: { reference: 'JHN.1', - versionId: DEFAULT_BIBLE_VERSION, + versionId: 111, renderNotes: true, }, argTypes: { From a19057ad54c1ecaf609702689587f05afd8baa5b Mon Sep 17 00:00:00 2001 From: Cameron Pak Date: Thu, 29 Jan 2026 11:40:35 -0600 Subject: [PATCH 07/47] fix(ui): add missing html dependency to verse selection/highlight effect --- packages/ui/src/components/verse.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ui/src/components/verse.tsx b/packages/ui/src/components/verse.tsx index e9d49c4c..dc77ea21 100644 --- a/packages/ui/src/components/verse.tsx +++ b/packages/ui/src/components/verse.tsx @@ -137,7 +137,7 @@ function BibleTextHtml({ el.classList.remove('yv-v-highlighted'); } }); - }, [selectedVerses, highlightedVerses]); + }, [html, selectedVerses, highlightedVerses]); useLayoutEffect(() => { const element = contentRef.current; From 6aa5dc1c6db10afa0821db5959439af3ab991ef9 Mon Sep 17 00:00:00 2001 From: Cameron Pak Date: Thu, 29 Jan 2026 11:44:01 -0600 Subject: [PATCH 08/47] fix(ui): only show pointer cursor when verses are selectable When onVerseSelect is not provided, verses now display default cursor instead of pointer cursor to avoid misleading users that verses are clickable. - Added data-selectable attribute to verse container - Updated CSS to conditionally apply cursor: pointer only when selectable --- packages/ui/src/styles/bible-reader.css | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/ui/src/styles/bible-reader.css b/packages/ui/src/styles/bible-reader.css index 85b9d7e6..4367ece4 100644 --- a/packages/ui/src/styles/bible-reader.css +++ b/packages/ui/src/styles/bible-reader.css @@ -104,6 +104,11 @@ & .yv-v, & .verse { display: inline; + } + + /* Only show pointer cursor when verses are selectable */ + &[data-selectable='true'] .yv-v, + &[data-selectable='true'] .verse { cursor: pointer; } From 4447b99f5fa668402dd49882984a78d5b16632d1 Mon Sep 17 00:00:00 2001 From: Cameron Pak Date: Thu, 29 Jan 2026 11:53:29 -0600 Subject: [PATCH 09/47] fix(ui): use ref to avoid stale closure in verse selection handler --- packages/ui/src/components/verse.tsx | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/ui/src/components/verse.tsx b/packages/ui/src/components/verse.tsx index dc77ea21..58383655 100644 --- a/packages/ui/src/components/verse.tsx +++ b/packages/ui/src/components/verse.tsx @@ -139,6 +139,9 @@ function BibleTextHtml({ }); }, [html, selectedVerses, highlightedVerses]); + const selectedVersesRef = useRef(selectedVerses); + selectedVersesRef.current = selectedVerses; + useLayoutEffect(() => { const element = contentRef.current; if (!element || !onVerseSelect) return; @@ -151,16 +154,17 @@ function BibleTextHtml({ const verseNum = parseInt(verseEl.getAttribute('v') || '0', 10); if (verseNum === 0) return; - const newSelected = selectedVerses.includes(verseNum) - ? selectedVerses.filter((v) => v !== verseNum) - : [...selectedVerses, verseNum].sort((a, b) => a - b); + const current = selectedVersesRef.current; + const newSelected = current.includes(verseNum) + ? current.filter((v) => v !== verseNum) + : [...current, verseNum].sort((a, b) => a - b); onVerseSelect(newSelected); }; element.addEventListener('click', handleClick); return () => element.removeEventListener('click', handleClick); - }, [selectedVerses, onVerseSelect]); + }, [onVerseSelect]); return ( <> From bb1e5f0975b573b54c4dbe3cc264e6bb9e4ed96a Mon Sep 17 00:00:00 2001 From: Cameron Pak Date: Thu, 29 Jan 2026 12:40:09 -0600 Subject: [PATCH 10/47] fix(ui): pre-optimize Vite deps and quiet MSW logs for CI stability - Add optimizeDeps.include for React/Radix to prevent mid-test reloads - Add quiet: true to MSW initialize to reduce test output noise --- packages/ui/.storybook/main.ts | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/packages/ui/.storybook/main.ts b/packages/ui/.storybook/main.ts index ca538181..a2f890ae 100644 --- a/packages/ui/.storybook/main.ts +++ b/packages/ui/.storybook/main.ts @@ -42,6 +42,26 @@ const config: StorybookConfig = { ], }, }; + /** + * Pre-optimize dependencies to prevent Vite from reloading mid-test in CI. + * Without this, Vite discovers dependencies during test execution and triggers + * a reload, which breaks the Vitest runner and causes "sb-preparing-story" failures. + * @see https://github.com/storybookjs/storybook/issues/33067 + * @see https://github.com/storybookjs/storybook/issues/32049 + */ + config.optimizeDeps = { + ...config.optimizeDeps, + include: [ + ...(config.optimizeDeps?.include ?? []), + 'react', + 'react-dom', + 'react-dom/client', + 'react/jsx-runtime', + 'react/jsx-dev-runtime', + '@radix-ui/react-popover', + '@radix-ui/react-use-controllable-state', + ], + }; return config; }, }; From 3658c6c7d353e77f1f91ccd7a3f180bb2461110e Mon Sep 17 00:00:00 2001 From: Cameron Pak Date: Thu, 29 Jan 2026 15:56:21 -0600 Subject: [PATCH 11/47] chore(storybook): remove optimizeDeps configuration The `optimizeDeps` configuration in Storybook's `main.ts` is no longer necessary. Recent updates to Vite and Storybook have resolved the CI test execution issues that this configuration aimed to prevent. --- packages/ui/.storybook/main.ts | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/packages/ui/.storybook/main.ts b/packages/ui/.storybook/main.ts index a2f890ae..ca538181 100644 --- a/packages/ui/.storybook/main.ts +++ b/packages/ui/.storybook/main.ts @@ -42,26 +42,6 @@ const config: StorybookConfig = { ], }, }; - /** - * Pre-optimize dependencies to prevent Vite from reloading mid-test in CI. - * Without this, Vite discovers dependencies during test execution and triggers - * a reload, which breaks the Vitest runner and causes "sb-preparing-story" failures. - * @see https://github.com/storybookjs/storybook/issues/33067 - * @see https://github.com/storybookjs/storybook/issues/32049 - */ - config.optimizeDeps = { - ...config.optimizeDeps, - include: [ - ...(config.optimizeDeps?.include ?? []), - 'react', - 'react-dom', - 'react-dom/client', - 'react/jsx-runtime', - 'react/jsx-dev-runtime', - '@radix-ui/react-popover', - '@radix-ui/react-use-controllable-state', - ], - }; return config; }, }; From 6d6b49f6ebbab4169a895235904c8ca275c4402e Mon Sep 17 00:00:00 2001 From: Cameron Pak Date: Thu, 29 Jan 2026 16:08:47 -0600 Subject: [PATCH 12/47] docs: added changeset --- .changeset/salty-plants-pick.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .changeset/salty-plants-pick.md diff --git a/.changeset/salty-plants-pick.md b/.changeset/salty-plants-pick.md new file mode 100644 index 00000000..eb9a37de --- /dev/null +++ b/.changeset/salty-plants-pick.md @@ -0,0 +1,7 @@ +--- +'@youversion/platform-react-hooks': minor +'@youversion/platform-core': minor +'@youversion/platform-react-ui': minor +--- + +This PR adds verse selection and highlighting to the Bible reader component, preparing the way for highlights. It also includes infrastructure fixes for Storybook test stability in CI. From 673e454b08b37a2509baec562a5cf6b2be55a67f Mon Sep 17 00:00:00 2001 From: Cameron Pak Date: Fri, 30 Jan 2026 09:22:17 -0600 Subject: [PATCH 13/47] feat: Add BoxStackIcon component This commit introduces the `BoxStackIcon` component to the `ui` package. It also updates the `icons/index.ts` file to export the new icon. --- .../ui/src/components/icons/box-stack.tsx | 25 +++++++++++++++++++ packages/ui/src/components/icons/index.ts | 15 +++++++++++ 2 files changed, 40 insertions(+) create mode 100644 packages/ui/src/components/icons/box-stack.tsx create mode 100644 packages/ui/src/components/icons/index.ts diff --git a/packages/ui/src/components/icons/box-stack.tsx b/packages/ui/src/components/icons/box-stack.tsx new file mode 100644 index 00000000..dcb1ae51 --- /dev/null +++ b/packages/ui/src/components/icons/box-stack.tsx @@ -0,0 +1,25 @@ +import type { ComponentProps, ReactElement } from 'react'; + +export function BoxStackIcon(props: ComponentProps<'svg'>): ReactElement { + return ( + + + + + ); +} diff --git a/packages/ui/src/components/icons/index.ts b/packages/ui/src/components/icons/index.ts new file mode 100644 index 00000000..10215966 --- /dev/null +++ b/packages/ui/src/components/icons/index.ts @@ -0,0 +1,15 @@ +export { ArrowLeftIcon } from './arrow-left'; +export { BookOpenIcon } from './book-open'; +export { BoxStackIcon } from './box-stack'; +export { ChevronDownIcon } from './chevron-down'; +export { Footnote, Footnote as FootnoteIcon } from './footnote'; +export { GearIcon } from './gear'; +export { GlobeIcon } from './globe'; +export { InfoIcon } from './info'; +export { LoaderIcon } from './loader'; +export { PersonIcon } from './person'; +export { SearchIcon } from './search'; +export { Share, Share as ShareIcon } from './share'; +export { Votd, Votd as VotdIcon } from './votd'; +export { XIcon } from './x'; +export { YouVersionLogo } from './youversion-logo'; From 354a984d3858ed82bf48fa0af7243f90c60eea6b Mon Sep 17 00:00:00 2001 From: Cameron Pak Date: Fri, 30 Jan 2026 09:28:23 -0600 Subject: [PATCH 14/47] feat: Add BoxArrowUpIcon component Add the BoxArrowUpIcon to the UI package. This icon will be used to represent the action of uploading or exporting content. --- .../ui/src/components/icons/box-arrow-up.tsx | 23 +++++++++++++++++++ packages/ui/src/components/icons/index.ts | 1 + 2 files changed, 24 insertions(+) create mode 100644 packages/ui/src/components/icons/box-arrow-up.tsx diff --git a/packages/ui/src/components/icons/box-arrow-up.tsx b/packages/ui/src/components/icons/box-arrow-up.tsx new file mode 100644 index 00000000..c1f9bf5c --- /dev/null +++ b/packages/ui/src/components/icons/box-arrow-up.tsx @@ -0,0 +1,23 @@ +import type { ComponentProps, ReactElement } from 'react'; + +export function BoxArrowUpIcon(props: ComponentProps<'svg'>): ReactElement { + return ( + + + + + ); +} diff --git a/packages/ui/src/components/icons/index.ts b/packages/ui/src/components/icons/index.ts index 10215966..3fe6b0f8 100644 --- a/packages/ui/src/components/icons/index.ts +++ b/packages/ui/src/components/icons/index.ts @@ -1,5 +1,6 @@ export { ArrowLeftIcon } from './arrow-left'; export { BookOpenIcon } from './book-open'; +export { BoxArrowUpIcon } from './box-arrow-up'; export { BoxStackIcon } from './box-stack'; export { ChevronDownIcon } from './chevron-down'; export { Footnote, Footnote as FootnoteIcon } from './footnote'; From 5ca255b99077c0f04b1db66abfb8418601b44284 Mon Sep 17 00:00:00 2001 From: Cameron Pak Date: Fri, 30 Jan 2026 11:50:42 -0600 Subject: [PATCH 15/47] feat(ui): wire up VerseActionPopover with BibleTextView MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Switch popover colors from named to hex (matches core schema) - Update highlightedVerses type: Record → Record - Add VerseActionPopoverStory for visual testing - Auto-open/close popover on verse selection --- .../src/components/verse-action-popover.tsx | 188 ++++++++++++++++++ packages/ui/src/components/verse.stories.tsx | 144 +++++++++++++- packages/ui/src/components/verse.tsx | 11 +- 3 files changed, 337 insertions(+), 6 deletions(-) create mode 100644 packages/ui/src/components/verse-action-popover.tsx diff --git a/packages/ui/src/components/verse-action-popover.tsx b/packages/ui/src/components/verse-action-popover.tsx new file mode 100644 index 00000000..5338c6ef --- /dev/null +++ b/packages/ui/src/components/verse-action-popover.tsx @@ -0,0 +1,188 @@ +import { useEffect, useRef, type FC } from 'react'; +import { cn } from '../lib/utils'; +import { BoxStackIcon } from './icons/box-stack'; +import { BoxArrowUpIcon } from './icons/box-arrow-up'; +import { XIcon } from './icons/x'; + +// Hex colors (6-digit, no #) matching core schema +export const HIGHLIGHT_COLORS = ['e6d163', '9ec56e', '6eb5c5', 'd4a054', 'd485b2'] as const; + +export type HighlightColor = (typeof HIGHLIGHT_COLORS)[number]; + +type VerseActionPopoverProps = { + open: boolean; + onOpenChange: (open: boolean) => void; + activeHighlights: Set; + hasUnhighlightedVerses: boolean; + position: { x: number; y: number }; + onHighlight: (color: string) => void; + onClearHighlight: (color: string) => void; + onCopy: () => void; + onShare: () => void; + theme?: 'light' | 'dark'; +}; + +type ColorCircleProps = { + color: string; + showX: boolean; + onClick: () => void; +}; + +function ColorCircle({ color, showX, onClick }: ColorCircleProps) { + return ( + + ); +} + +type ActionButtonProps = { + icon: React.ReactNode; + label: string; + onClick: () => void; +}; + +function ActionButton({ icon, label, onClick }: ActionButtonProps) { + return ( + + ); +} + +const VerseActionPopover: FC = ({ + open, + onOpenChange, + activeHighlights, + hasUnhighlightedVerses, + position, + onHighlight, + onClearHighlight, + onCopy, + onShare, + theme = 'light', +}) => { + const popoverRef = useRef(null); + + // Show/hide popover based on open state + useEffect(() => { + const el = popoverRef.current; + if (!el) return; + + if (open) { + el.showPopover(); + } else { + el.hidePopover(); + } + }, [open]); + + // Listen for toggle events to sync state + useEffect(() => { + const el = popoverRef.current; + if (!el) return; + + const handleToggle = (e: ToggleEvent) => { + if (e.newState === 'closed') { + onOpenChange(false); + } + }; + + el.addEventListener('toggle', handleToggle); + return () => el.removeEventListener('toggle', handleToggle); + }, [onOpenChange]); + + // Focus first button when popover opens + useEffect(() => { + if (open && popoverRef.current) { + const firstButton = popoverRef.current.querySelector('button'); + firstButton?.focus(); + } + }, [open]); + + // Build color circles based on active highlights and unhighlighted verses + const colorCircles: { color: string; showX: boolean; key: string }[] = []; + + for (const color of HIGHLIGHT_COLORS) { + const isActive = activeHighlights.has(color); + + // Show circle with X if this color is active (to clear it) + if (isActive) { + colorCircles.push({ color, showX: true, key: `${color}-clear` }); + } + + // Show circle without X if there are unhighlighted verses (to apply this color) + if (hasUnhighlightedVerses) { + colorCircles.push({ color, showX: false, key: `${color}-apply` }); + } + } + + return ( +
+ {/* Highlight color circles */} +
+ {colorCircles.map(({ color, showX, key }) => ( + (showX ? onClearHighlight(color) : onHighlight(color))} + /> + ))} +
+ + {/* Separator */} + + ); +}; + +export default VerseActionPopover; diff --git a/packages/ui/src/components/verse.stories.tsx b/packages/ui/src/components/verse.stories.tsx index 6b6066cf..4909b21e 100644 --- a/packages/ui/src/components/verse.stories.tsx +++ b/packages/ui/src/components/verse.stories.tsx @@ -5,6 +5,7 @@ import React from 'react'; import { type BibleTextViewProps, BibleTextView } from './verse'; import { Button } from './ui/button'; import { XIcon } from '@/components/icons/x'; +import VerseActionPopover from './verse-action-popover'; // USFM format: BOOK.CHAPTER or BOOK.CHAPTER.VERSE or BOOK.CHAPTER.VERSE-VERSE const USFM_PATTERN = /^[A-Z1-4]{3}\.\d+(\.\d+(-\d+)?)?$/; @@ -349,13 +350,13 @@ export const FootnotePopoverThemeDark: Story = { function VerseSelectionDemo(props: BibleTextViewProps) { const [selectedVerses, setSelectedVerses] = React.useState([]); - const [highlightedVerses, setHighlightedVerses] = React.useState>({}); + const [highlightedVerses, setHighlightedVerses] = React.useState>({}); const handleHighlight = () => { setHighlightedVerses((prev) => { const next = { ...prev }; for (const verse of selectedVerses) { - next[verse] = true; + next[verse] = 'e6d163'; // yellow } return next; }); @@ -453,3 +454,142 @@ export const VerseSelection: Story = { }, render: (props) => , }; + +function VerseActionPopoverDemo(props: BibleTextViewProps) { + const containerRef = React.useRef(null); + const [selectedVerses, setSelectedVerses] = React.useState([]); + const [highlightedVerses, setHighlightedVerses] = React.useState>({}); + const [popoverOpen, setPopoverOpen] = React.useState(false); + const [popoverPosition, setPopoverPosition] = React.useState({ x: 0, y: 0 }); + + const handleVerseSelect = React.useCallback((verses: number[]) => { + setSelectedVerses(verses); + + if (verses.length === 0) { + setPopoverOpen(false); + return; + } + + // Position popover below last selected verse + const lastVerse = Math.max(...verses); + const verseEl = containerRef.current?.querySelector(`.yv-v[v="${lastVerse}"]`); + if (verseEl) { + const rect = verseEl.getBoundingClientRect(); + setPopoverPosition({ + x: rect.left + rect.width / 2, + y: rect.bottom + 8, + }); + } + setPopoverOpen(true); + }, []); + + // Derive activeHighlights from selected verses + const activeHighlights = React.useMemo(() => { + return new Set( + selectedVerses.map((v) => highlightedVerses[v]).filter((c): c is string => Boolean(c)), + ); + }, [selectedVerses, highlightedVerses]); + + const hasUnhighlightedVerses = React.useMemo(() => { + return selectedVerses.some((v) => !highlightedVerses[v]); + }, [selectedVerses, highlightedVerses]); + + const handleHighlight = React.useCallback( + (color: string) => { + setHighlightedVerses((prev) => { + const next = { ...prev }; + for (const verse of selectedVerses) { + if (!next[verse]) { + next[verse] = color; + } + } + return next; + }); + }, + [selectedVerses], + ); + + const handleClearHighlight = React.useCallback( + (color: string) => { + setHighlightedVerses((prev) => { + const next = { ...prev }; + for (const verse of selectedVerses) { + if (next[verse] === color) { + delete next[verse]; + } + } + return next; + }); + }, + [selectedVerses], + ); + + return ( +
+
+

+ Selected: {selectedVerses.length > 0 ? selectedVerses.join(', ') : 'None'} +

+ +
+ +
+ +
+ + { + // Do nothing at this second + }} + onShare={() => { + // Do nothing at this second + }} + /> +
+ ); +} + +export const VerseActionPopoverStory: Story = { + name: 'Verse Action Popover', + args: { + reference: 'JHN.1', + versionId: 111, + renderNotes: true, + }, + argTypes: { + theme: { table: { disable: true } }, + selectedVerses: { table: { disable: true } }, + onVerseSelect: { table: { disable: true } }, + highlightedVerses: { table: { disable: true } }, + }, + render: (props) => , +}; diff --git a/packages/ui/src/components/verse.tsx b/packages/ui/src/components/verse.tsx index 58383655..ba21dc83 100644 --- a/packages/ui/src/components/verse.tsx +++ b/packages/ui/src/components/verse.tsx @@ -99,7 +99,7 @@ function BibleTextHtml({ theme?: 'light' | 'dark'; selectedVerses?: number[]; onVerseSelect?: (verses: number[]) => void; - highlightedVerses?: Record; + highlightedVerses?: Record; }) { const contentRef = useRef(null); const [placeholders, setPlaceholders] = useState>(new Map()); @@ -131,10 +131,13 @@ function BibleTextHtml({ el.classList.remove('yv-v-selected'); } - if (highlightedVerses[verseNum]) { + const highlightColor = highlightedVerses[verseNum]; + if (highlightColor) { el.classList.add('yv-v-highlighted'); + (el as HTMLElement).style.backgroundColor = `#${highlightColor}`; } else { el.classList.remove('yv-v-highlighted'); + (el as HTMLElement).style.backgroundColor = ''; } }); }, [html, selectedVerses, highlightedVerses]); @@ -295,7 +298,7 @@ type VerseHtmlProps = { theme?: 'light' | 'dark'; selectedVerses?: number[]; onVerseSelect?: (verses: number[]) => void; - highlightedVerses?: Record; + highlightedVerses?: Record; }; /** @@ -401,7 +404,7 @@ export type BibleTextViewProps = { theme?: 'light' | 'dark'; selectedVerses?: number[]; onVerseSelect?: (verses: number[]) => void; - highlightedVerses?: Record; + highlightedVerses?: Record; }; /** From 42ba15b1d022b733653afd29c193ba0d59605fb7 Mon Sep 17 00:00:00 2001 From: Cameron Pak Date: Mon, 2 Feb 2026 13:54:34 -0600 Subject: [PATCH 16/47] Fix: Conditionally render VerseActionPopover The popover should only be rendered when there are selected verses, preventing it from appearing unnecessarily. --- packages/ui/src/components/verse.stories.tsx | 32 +++++++++++--------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/packages/ui/src/components/verse.stories.tsx b/packages/ui/src/components/verse.stories.tsx index 4909b21e..aa112caf 100644 --- a/packages/ui/src/components/verse.stories.tsx +++ b/packages/ui/src/components/verse.stories.tsx @@ -559,21 +559,23 @@ function VerseActionPopoverDemo(props: BibleTextViewProps) { />
- { - // Do nothing at this second - }} - onShare={() => { - // Do nothing at this second - }} - /> + {selectedVerses.length > 0 && ( + { + // Do nothing at this second + }} + onShare={() => { + // Do nothing at this second + }} + /> + )} ); } From 5216edcd7f6847ab1f7ba25968b5ab7e858f612a Mon Sep 17 00:00:00 2001 From: Cameron Pak Date: Mon, 2 Feb 2026 13:59:37 -0600 Subject: [PATCH 17/47] Fix: Prevent rendering popover when closed Conditionally render the `VerseActionPopover` or return null if the `open` prop is false. This prevents unnecessary rendering of the popover when it's not intended to be visible. Additionally, the storybook example has been updated to ensure the popover is only rendered when `selectedVerses.length > 0`. --- .../src/components/verse-action-popover.tsx | 4 +++ packages/ui/src/components/verse.stories.tsx | 36 ++++++++++--------- 2 files changed, 23 insertions(+), 17 deletions(-) diff --git a/packages/ui/src/components/verse-action-popover.tsx b/packages/ui/src/components/verse-action-popover.tsx index 5338c6ef..3c703fec 100644 --- a/packages/ui/src/components/verse-action-popover.tsx +++ b/packages/ui/src/components/verse-action-popover.tsx @@ -136,6 +136,10 @@ const VerseActionPopover: FC = ({ } } + if (!open) { + return null; + } + return (
- {selectedVerses.length > 0 && ( - { - // Do nothing at this second - }} - onShare={() => { - // Do nothing at this second - }} - /> - )} + 0} + onOpenChange={setPopoverOpen} + activeHighlights={activeHighlights} + hasUnhighlightedVerses={hasUnhighlightedVerses} + position={popoverPosition} + onHighlight={handleHighlight} + onClearHighlight={handleClearHighlight} + onCopy={() => { + // Do nothing at this second + }} + onShare={() => { + // Do nothing at this second + }} + /> ); } From f95902727c1aa34d7485c90c5af773020016c66f Mon Sep 17 00:00:00 2001 From: Cameron Pak Date: Mon, 2 Feb 2026 14:04:19 -0600 Subject: [PATCH 18/47] Refactor: Refactor verse action popover highlight logic Remove `hasUnhighlightedVerses` prop. The popover will now always display all highlight color options, showing an 'x' icon for active highlights and no 'x' for inactive ones. This simplifies the UI and logic by consistently presenting available actions. --- .../src/components/verse-action-popover.tsx | 25 ++++++------------- packages/ui/src/components/verse.stories.tsx | 5 ---- 2 files changed, 7 insertions(+), 23 deletions(-) diff --git a/packages/ui/src/components/verse-action-popover.tsx b/packages/ui/src/components/verse-action-popover.tsx index 3c703fec..deff48ce 100644 --- a/packages/ui/src/components/verse-action-popover.tsx +++ b/packages/ui/src/components/verse-action-popover.tsx @@ -13,7 +13,6 @@ type VerseActionPopoverProps = { open: boolean; onOpenChange: (open: boolean) => void; activeHighlights: Set; - hasUnhighlightedVerses: boolean; position: { x: number; y: number }; onHighlight: (color: string) => void; onClearHighlight: (color: string) => void; @@ -74,7 +73,6 @@ const VerseActionPopover: FC = ({ open, onOpenChange, activeHighlights, - hasUnhighlightedVerses, position, onHighlight, onClearHighlight, @@ -119,22 +117,13 @@ const VerseActionPopover: FC = ({ } }, [open]); - // Build color circles based on active highlights and unhighlighted verses - const colorCircles: { color: string; showX: boolean; key: string }[] = []; - - for (const color of HIGHLIGHT_COLORS) { - const isActive = activeHighlights.has(color); - - // Show circle with X if this color is active (to clear it) - if (isActive) { - colorCircles.push({ color, showX: true, key: `${color}-clear` }); - } - - // Show circle without X if there are unhighlighted verses (to apply this color) - if (hasUnhighlightedVerses) { - colorCircles.push({ color, showX: false, key: `${color}-apply` }); - } - } + // Build color circles: always show all colors + // Active highlights show X (to clear), inactive show no X (to apply) + const colorCircles = HIGHLIGHT_COLORS.map((color) => ({ + color, + showX: activeHighlights.has(color), + key: color, + })); if (!open) { return null; diff --git a/packages/ui/src/components/verse.stories.tsx b/packages/ui/src/components/verse.stories.tsx index 6dd2089e..7bf86290 100644 --- a/packages/ui/src/components/verse.stories.tsx +++ b/packages/ui/src/components/verse.stories.tsx @@ -490,10 +490,6 @@ function VerseActionPopoverDemo(props: BibleTextViewProps) { ); }, [selectedVerses, highlightedVerses]); - const hasUnhighlightedVerses = React.useMemo(() => { - return selectedVerses.some((v) => !highlightedVerses[v]); - }, [selectedVerses, highlightedVerses]); - const handleHighlight = React.useCallback( (color: string) => { setHighlightedVerses((prev) => { @@ -567,7 +563,6 @@ function VerseActionPopoverDemo(props: BibleTextViewProps) { open={popoverOpen && selectedVerses.length > 0} onOpenChange={setPopoverOpen} activeHighlights={activeHighlights} - hasUnhighlightedVerses={hasUnhighlightedVerses} position={popoverPosition} onHighlight={handleHighlight} onClearHighlight={handleClearHighlight} From d1f60195b2451fce0fee0be29551efb029f5666e Mon Sep 17 00:00:00 2001 From: Cameron Pak Date: Mon, 2 Feb 2026 14:08:31 -0600 Subject: [PATCH 19/47] fix: clicking X clears all highlights from selected verses - Rename onClearHighlight -> onClearHighlights (no color param) - Clear all selected verses regardless of highlight color --- .../src/components/verse-action-popover.tsx | 6 ++-- packages/ui/src/components/verse.stories.tsx | 29 ++++++++----------- 2 files changed, 15 insertions(+), 20 deletions(-) diff --git a/packages/ui/src/components/verse-action-popover.tsx b/packages/ui/src/components/verse-action-popover.tsx index deff48ce..faaf4cc5 100644 --- a/packages/ui/src/components/verse-action-popover.tsx +++ b/packages/ui/src/components/verse-action-popover.tsx @@ -15,7 +15,7 @@ type VerseActionPopoverProps = { activeHighlights: Set; position: { x: number; y: number }; onHighlight: (color: string) => void; - onClearHighlight: (color: string) => void; + onClearHighlights: () => void; onCopy: () => void; onShare: () => void; theme?: 'light' | 'dark'; @@ -75,7 +75,7 @@ const VerseActionPopover: FC = ({ activeHighlights, position, onHighlight, - onClearHighlight, + onClearHighlights, onCopy, onShare, theme = 'light', @@ -157,7 +157,7 @@ const VerseActionPopover: FC = ({ key={key} color={color} showX={showX} - onClick={() => (showX ? onClearHighlight(color) : onHighlight(color))} + onClick={() => (showX ? onClearHighlights() : onHighlight(color))} /> ))} diff --git a/packages/ui/src/components/verse.stories.tsx b/packages/ui/src/components/verse.stories.tsx index 7bf86290..7b90e809 100644 --- a/packages/ui/src/components/verse.stories.tsx +++ b/packages/ui/src/components/verse.stories.tsx @@ -507,22 +507,17 @@ function VerseActionPopoverDemo(props: BibleTextViewProps) { [selectedVerses], ); - const handleClearHighlight = React.useCallback( - (color: string) => { - setHighlightedVerses((prev) => { - const next = { ...prev }; - for (const verse of selectedVerses) { - if (next[verse] === color) { - delete next[verse]; - } - } - return next; - }); - setPopoverOpen(false); - setSelectedVerses([]); - }, - [selectedVerses], - ); + const handleClearHighlights = React.useCallback(() => { + setHighlightedVerses((prev) => { + const next = { ...prev }; + for (const verse of selectedVerses) { + delete next[verse]; + } + return next; + }); + setPopoverOpen(false); + setSelectedVerses([]); + }, [selectedVerses]); return (
{ // Do nothing at this second }} From 7a3d2ad8c9c3dfa3de1b8adbb4aeaa800de8cdf4 Mon Sep 17 00:00:00 2001 From: Cameron Pak Date: Mon, 2 Feb 2026 14:09:24 -0600 Subject: [PATCH 20/47] fix: allow re-highlighting verses with different color - Remove guard that prevented overwriting existing highlights --- packages/ui/src/components/verse.stories.tsx | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/ui/src/components/verse.stories.tsx b/packages/ui/src/components/verse.stories.tsx index 7b90e809..bc7862ff 100644 --- a/packages/ui/src/components/verse.stories.tsx +++ b/packages/ui/src/components/verse.stories.tsx @@ -495,9 +495,7 @@ function VerseActionPopoverDemo(props: BibleTextViewProps) { setHighlightedVerses((prev) => { const next = { ...prev }; for (const verse of selectedVerses) { - if (!next[verse]) { - next[verse] = color; - } + next[verse] = color; } return next; }); From f4c6143055ad4488d355f6a0cfb12f7997804901 Mon Sep 17 00:00:00 2001 From: Cameron Pak Date: Mon, 2 Feb 2026 14:10:24 -0600 Subject: [PATCH 21/47] feat: show active highlight colors first in popover - Active colors with X appear left-aligned - Inactive colors follow in standard order --- .../ui/src/components/verse-action-popover.tsx | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/ui/src/components/verse-action-popover.tsx b/packages/ui/src/components/verse-action-popover.tsx index faaf4cc5..9c54e602 100644 --- a/packages/ui/src/components/verse-action-popover.tsx +++ b/packages/ui/src/components/verse-action-popover.tsx @@ -117,13 +117,13 @@ const VerseActionPopover: FC = ({ } }, [open]); - // Build color circles: always show all colors - // Active highlights show X (to clear), inactive show no X (to apply) - const colorCircles = HIGHLIGHT_COLORS.map((color) => ({ - color, - showX: activeHighlights.has(color), - key: color, - })); + // Build color circles: active highlights (with X) first, then remaining colors + const activeColors = HIGHLIGHT_COLORS.filter((c) => activeHighlights.has(c)); + const inactiveColors = HIGHLIGHT_COLORS.filter((c) => !activeHighlights.has(c)); + const colorCircles = [ + ...activeColors.map((color) => ({ color, showX: true, key: `${color}-clear` })), + ...inactiveColors.map((color) => ({ color, showX: false, key: color })), + ]; if (!open) { return null; From 52da3400f0725882f695041cfdf6140860504e04 Mon Sep 17 00:00:00 2001 From: Cameron Pak Date: Mon, 2 Feb 2026 14:20:37 -0600 Subject: [PATCH 22/47] fix: don't show duplicate color when verse already has that highlight - Active colors shown with X (to clear) - Only inactive colors shown as apply options - Prevents useless same-color re-highlight action --- packages/ui/src/components/verse-action-popover.tsx | 11 +++++++++-- packages/ui/src/components/verse.stories.tsx | 5 +++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/packages/ui/src/components/verse-action-popover.tsx b/packages/ui/src/components/verse-action-popover.tsx index 9c54e602..9ac9016a 100644 --- a/packages/ui/src/components/verse-action-popover.tsx +++ b/packages/ui/src/components/verse-action-popover.tsx @@ -13,6 +13,7 @@ type VerseActionPopoverProps = { open: boolean; onOpenChange: (open: boolean) => void; activeHighlights: Set; + hasUnhighlightedVerses: boolean; position: { x: number; y: number }; onHighlight: (color: string) => void; onClearHighlights: () => void; @@ -73,6 +74,7 @@ const VerseActionPopover: FC = ({ open, onOpenChange, activeHighlights, + hasUnhighlightedVerses, position, onHighlight, onClearHighlights, @@ -117,12 +119,17 @@ const VerseActionPopover: FC = ({ } }, [open]); - // Build color circles: active highlights (with X) first, then remaining colors + // Build color circles: + // 1. Active highlights with X (to clear) - always shown first + // 2. Non-active colors without X (to apply) - shown if any verse can be highlighted/re-highlighted const activeColors = HIGHLIGHT_COLORS.filter((c) => activeHighlights.has(c)); const inactiveColors = HIGHLIGHT_COLORS.filter((c) => !activeHighlights.has(c)); + const showApplyColors = hasUnhighlightedVerses || activeHighlights.size > 0; const colorCircles = [ ...activeColors.map((color) => ({ color, showX: true, key: `${color}-clear` })), - ...inactiveColors.map((color) => ({ color, showX: false, key: color })), + ...(showApplyColors + ? inactiveColors.map((color) => ({ color, showX: false, key: `${color}-apply` })) + : []), ]; if (!open) { diff --git a/packages/ui/src/components/verse.stories.tsx b/packages/ui/src/components/verse.stories.tsx index bc7862ff..2cbacc2b 100644 --- a/packages/ui/src/components/verse.stories.tsx +++ b/packages/ui/src/components/verse.stories.tsx @@ -490,6 +490,10 @@ function VerseActionPopoverDemo(props: BibleTextViewProps) { ); }, [selectedVerses, highlightedVerses]); + const hasUnhighlightedVerses = React.useMemo(() => { + return selectedVerses.some((v) => !highlightedVerses[v]); + }, [selectedVerses, highlightedVerses]); + const handleHighlight = React.useCallback( (color: string) => { setHighlightedVerses((prev) => { @@ -556,6 +560,7 @@ function VerseActionPopoverDemo(props: BibleTextViewProps) { open={popoverOpen && selectedVerses.length > 0} onOpenChange={setPopoverOpen} activeHighlights={activeHighlights} + hasUnhighlightedVerses={hasUnhighlightedVerses} position={popoverPosition} onHighlight={handleHighlight} onClearHighlights={handleClearHighlights} From 79ec413570e1a51074c3fbcfeaa679270ce13476 Mon Sep 17 00:00:00 2001 From: Cameron Pak Date: Mon, 2 Feb 2026 14:22:42 -0600 Subject: [PATCH 23/47] fix: show all apply colors when selection has unhighlighted verses - AC 6 & 7: When hasUnhighlightedVerses, show ALL colors as apply options - Allows applying active color to unhighlighted verse in mixed selection - When only re-highlighting (no unhighlighted), show only inactive colors --- packages/ui/src/components/verse-action-popover.tsx | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/ui/src/components/verse-action-popover.tsx b/packages/ui/src/components/verse-action-popover.tsx index 9ac9016a..de6ba6a2 100644 --- a/packages/ui/src/components/verse-action-popover.tsx +++ b/packages/ui/src/components/verse-action-popover.tsx @@ -120,15 +120,18 @@ const VerseActionPopover: FC = ({ }, [open]); // Build color circles: - // 1. Active highlights with X (to clear) - always shown first - // 2. Non-active colors without X (to apply) - shown if any verse can be highlighted/re-highlighted + // 1. Active highlights with X (to clear) - always shown first/left + // 2. Apply colors (without X): + // - If hasUnhighlightedVerses: show ALL colors (active color can be applied to unhighlighted verse) + // - If only re-highlighting: show only inactive colors (no point applying same color) const activeColors = HIGHLIGHT_COLORS.filter((c) => activeHighlights.has(c)); const inactiveColors = HIGHLIGHT_COLORS.filter((c) => !activeHighlights.has(c)); + const applyColors = hasUnhighlightedVerses ? HIGHLIGHT_COLORS : inactiveColors; const showApplyColors = hasUnhighlightedVerses || activeHighlights.size > 0; const colorCircles = [ ...activeColors.map((color) => ({ color, showX: true, key: `${color}-clear` })), ...(showApplyColors - ? inactiveColors.map((color) => ({ color, showX: false, key: `${color}-apply` })) + ? applyColors.map((color) => ({ color, showX: false, key: `${color}-apply` })) : []), ]; From 854298781993ea47628df2faf79ddc2cca38c5a9 Mon Sep 17 00:00:00 2001 From: Cameron Pak Date: Mon, 2 Feb 2026 14:26:33 -0600 Subject: [PATCH 24/47] fix: always show all 5 apply colors in popover - Remove hasUnhighlightedVerses prop (no longer needed) - Active colors with X shown first, then all 5 colors for apply/change - Allows changing multiple highlights to same color --- packages/ui/src/components/verse-action-popover.tsx | 13 ++----------- packages/ui/src/components/verse.stories.tsx | 5 ----- 2 files changed, 2 insertions(+), 16 deletions(-) diff --git a/packages/ui/src/components/verse-action-popover.tsx b/packages/ui/src/components/verse-action-popover.tsx index de6ba6a2..bd4fcb42 100644 --- a/packages/ui/src/components/verse-action-popover.tsx +++ b/packages/ui/src/components/verse-action-popover.tsx @@ -13,7 +13,6 @@ type VerseActionPopoverProps = { open: boolean; onOpenChange: (open: boolean) => void; activeHighlights: Set; - hasUnhighlightedVerses: boolean; position: { x: number; y: number }; onHighlight: (color: string) => void; onClearHighlights: () => void; @@ -74,7 +73,6 @@ const VerseActionPopover: FC = ({ open, onOpenChange, activeHighlights, - hasUnhighlightedVerses, position, onHighlight, onClearHighlights, @@ -121,18 +119,11 @@ const VerseActionPopover: FC = ({ // Build color circles: // 1. Active highlights with X (to clear) - always shown first/left - // 2. Apply colors (without X): - // - If hasUnhighlightedVerses: show ALL colors (active color can be applied to unhighlighted verse) - // - If only re-highlighting: show only inactive colors (no point applying same color) + // 2. At minimum, show all 5 colors without X (to apply/change) - shown when any verses selected const activeColors = HIGHLIGHT_COLORS.filter((c) => activeHighlights.has(c)); - const inactiveColors = HIGHLIGHT_COLORS.filter((c) => !activeHighlights.has(c)); - const applyColors = hasUnhighlightedVerses ? HIGHLIGHT_COLORS : inactiveColors; - const showApplyColors = hasUnhighlightedVerses || activeHighlights.size > 0; const colorCircles = [ ...activeColors.map((color) => ({ color, showX: true, key: `${color}-clear` })), - ...(showApplyColors - ? applyColors.map((color) => ({ color, showX: false, key: `${color}-apply` })) - : []), + ...HIGHLIGHT_COLORS.map((color) => ({ color, showX: false, key: `${color}-apply` })), ]; if (!open) { diff --git a/packages/ui/src/components/verse.stories.tsx b/packages/ui/src/components/verse.stories.tsx index 2cbacc2b..bc7862ff 100644 --- a/packages/ui/src/components/verse.stories.tsx +++ b/packages/ui/src/components/verse.stories.tsx @@ -490,10 +490,6 @@ function VerseActionPopoverDemo(props: BibleTextViewProps) { ); }, [selectedVerses, highlightedVerses]); - const hasUnhighlightedVerses = React.useMemo(() => { - return selectedVerses.some((v) => !highlightedVerses[v]); - }, [selectedVerses, highlightedVerses]); - const handleHighlight = React.useCallback( (color: string) => { setHighlightedVerses((prev) => { @@ -560,7 +556,6 @@ function VerseActionPopoverDemo(props: BibleTextViewProps) { open={popoverOpen && selectedVerses.length > 0} onOpenChange={setPopoverOpen} activeHighlights={activeHighlights} - hasUnhighlightedVerses={hasUnhighlightedVerses} position={popoverPosition} onHighlight={handleHighlight} onClearHighlights={handleClearHighlights} From 33eea9d0bd15dfb2656fc08d0bed8d045532ba78 Mon Sep 17 00:00:00 2001 From: Cameron Pak Date: Mon, 2 Feb 2026 14:50:09 -0600 Subject: [PATCH 25/47] Docs: Add Verse Action Popover PRD --- PRD-verse-action-popover.md | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 PRD-verse-action-popover.md diff --git a/PRD-verse-action-popover.md b/PRD-verse-action-popover.md new file mode 100644 index 00000000..bd65976d --- /dev/null +++ b/PRD-verse-action-popover.md @@ -0,0 +1,37 @@ +# Verse Action Popover - PRD + +## Overview + +Popover that appears when user selects Bible verse(s). Provides highlight, copy, and share actions. + +## Acceptance Criteria + +### AC 1 - Basic popover display +Given I select a verse, the popover shows 5 color circles (yellow, green, blue, orange, pink). + +### AC 2 - Apply highlight +Given popover is open, clicking a color circle applies that highlight to all selected verses. Popover dismisses. + +### AC 3 - Copy +Clicking Copy puts verse text in clipboard as `"Verse text" - Book Chapter:Verse Version` (e.g., `"Romans 8:1 BSB"`). Popover dismisses. + +### AC 4 - Share +Clicking Share opens native Web Share API. On successful share only, popover dismisses. On failure/cancel, popover remains open. + +### AC 5 - Single highlighted verse selected +Popover shows that verse's color with X (remove action), plus all 5 apply colors. Total: 6 circles. + +### AC 5a - Ordering +X circles (remove) appear leftmost. Apply circles follow in standard order (yellow, green, blue, orange, pink). + +### AC 6 - Mixed selection (highlighted + unhighlighted) +Yellow highlighted verse + unhighlighted verse selected -> popover shows: Yellow X, then Yellow, Green, Blue, Orange, Pink (6 total). The Yellow apply button lets user apply yellow to the unhighlighted verse too. + +### AC 7 - Multiple different highlights selected +Yellow verse + Green verse selected -> popover shows: Yellow X, Green X, then Yellow, Green, Blue, Orange, Pink (7 total). Clicking Yellow X removes only yellow from the yellow verse. Green verse unchanged. Both verses remain selected. Popover remains open. + +### AC 8 - Dismiss on remove (single highlight) +When only one highlight color is active in selection, clicking its X removes it and dismisses popover. + +### AC 8a - Popover stays open (multiple highlights) +When multiple highlight colors are active in selection, clicking an X removes only that color. Popover remains open (so user can remove other highlights if desired). From 087a2100ec178cfd45dce6dcd6e52cc259a8fa1e Mon Sep 17 00:00:00 2001 From: Cameron Pak Date: Mon, 2 Feb 2026 15:17:35 -0600 Subject: [PATCH 26/47] Feature: Add VerseActionPopover component Introduce a new popover component for verse actions and associated highlight colors. This component is exported from the `ui` package and includes mock implementations for the Popover API in the test setup. --- packages/ui/src/components/index.ts | 1 + .../src/components/verse-action-popover.tsx | 4 +-- packages/ui/src/components/verse.stories.tsx | 2 +- packages/ui/src/test/setup.ts | 29 +++++++++++++++++++ 4 files changed, 32 insertions(+), 4 deletions(-) diff --git a/packages/ui/src/components/index.ts b/packages/ui/src/components/index.ts index f4ea9682..2ffe15a6 100644 --- a/packages/ui/src/components/index.ts +++ b/packages/ui/src/components/index.ts @@ -9,3 +9,4 @@ export { YouVersionAuthButton, type YouVersionAuthButtonProps } from './YouVersi export { VerseOfTheDay, type VerseOfTheDayProps } from './verse-of-the-day'; export { BibleTextView, type BibleTextViewProps } from './verse'; export { BibleWidgetView, type BibleWidgetViewProps } from './bible-widget-view'; +export { VerseActionPopover, HIGHLIGHT_COLORS } from './verse-action-popover'; diff --git a/packages/ui/src/components/verse-action-popover.tsx b/packages/ui/src/components/verse-action-popover.tsx index bd4fcb42..908ba8a1 100644 --- a/packages/ui/src/components/verse-action-popover.tsx +++ b/packages/ui/src/components/verse-action-popover.tsx @@ -69,7 +69,7 @@ function ActionButton({ icon, label, onClick }: ActionButtonProps) { ); } -const VerseActionPopover: FC = ({ +export const VerseActionPopover: FC = ({ open, onOpenChange, activeHighlights, @@ -178,5 +178,3 @@ const VerseActionPopover: FC = ({
); }; - -export default VerseActionPopover; diff --git a/packages/ui/src/components/verse.stories.tsx b/packages/ui/src/components/verse.stories.tsx index bc7862ff..84987d4f 100644 --- a/packages/ui/src/components/verse.stories.tsx +++ b/packages/ui/src/components/verse.stories.tsx @@ -5,7 +5,7 @@ import React from 'react'; import { type BibleTextViewProps, BibleTextView } from './verse'; import { Button } from './ui/button'; import { XIcon } from '@/components/icons/x'; -import VerseActionPopover from './verse-action-popover'; +import { VerseActionPopover } from './verse-action-popover'; // USFM format: BOOK.CHAPTER or BOOK.CHAPTER.VERSE or BOOK.CHAPTER.VERSE-VERSE const USFM_PATTERN = /^[A-Z1-4]{3}\.\d+(\.\d+(-\d+)?)?$/; diff --git a/packages/ui/src/test/setup.ts b/packages/ui/src/test/setup.ts index c8e953b6..ac957b98 100644 --- a/packages/ui/src/test/setup.ts +++ b/packages/ui/src/test/setup.ts @@ -3,6 +3,35 @@ import { afterEach } from 'vitest'; import { cleanup } from '@testing-library/react'; import '@testing-library/jest-dom/vitest'; +// Popover API mock for jsdom +if (typeof HTMLElement !== 'undefined' && !HTMLElement.prototype.showPopover) { + HTMLElement.prototype.showPopover = function () { + this.style.display = 'block'; + const toggleEvent = new Event('toggle', { bubbles: false }); + Object.defineProperty(toggleEvent, 'newState', { value: 'open', writable: false }); + this.dispatchEvent(toggleEvent); + }; + + HTMLElement.prototype.hidePopover = function () { + this.style.display = 'none'; + const toggleEvent = new Event('toggle', { bubbles: false }); + Object.defineProperty(toggleEvent, 'newState', { value: 'closed', writable: false }); + this.dispatchEvent(toggleEvent); + }; + + HTMLElement.prototype.togglePopover = function (force?: boolean): boolean { + const isOpen = this.style.display !== 'none'; + if (force === undefined || force !== isOpen) { + if (isOpen) { + this.hidePopover(); + } else { + this.showPopover(); + } + } + return !isOpen; + }; +} + // Clean up after each test afterEach(() => { cleanup(); From 5f477ac25235bd57cde6206571d5225ba8b097d0 Mon Sep 17 00:00:00 2001 From: Cameron Pak Date: Mon, 2 Feb 2026 15:17:48 -0600 Subject: [PATCH 27/47] test: Add tests for VerseActionPopover Add comprehensive tests for the VerseActionPopover component, covering basic display, highlight actions, copy and share functionality, and accessibility. This ensures the component behaves as expected under various conditions and adheres to best practices. --- .../components/verse-action-popover.test.tsx | 335 ++++++++++++++++++ 1 file changed, 335 insertions(+) create mode 100644 packages/ui/src/components/verse-action-popover.test.tsx diff --git a/packages/ui/src/components/verse-action-popover.test.tsx b/packages/ui/src/components/verse-action-popover.test.tsx new file mode 100644 index 00000000..ee14db19 --- /dev/null +++ b/packages/ui/src/components/verse-action-popover.test.tsx @@ -0,0 +1,335 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { render, screen, fireEvent } from '@testing-library/react'; +import { VerseActionPopover, HIGHLIGHT_COLORS, type HighlightColor } from './verse-action-popover'; + +describe('VerseActionPopover', () => { + const defaultProps = { + open: true, + onOpenChange: vi.fn(), + activeHighlights: new Set(), + position: { x: 100, y: 100 }, + onHighlight: vi.fn(), + onClearHighlights: vi.fn(), + onCopy: vi.fn(), + onShare: vi.fn(), + }; + + beforeEach(() => { + vi.clearAllMocks(); + }); + + describe('AC1: Basic popover display', () => { + it('should display 5 color circles when verse selected', () => { + render(); + + const colorButtons = screen + .getAllByRole('button') + .filter((btn) => btn.getAttribute('aria-label')?.includes('Apply highlight')); + + expect(colorButtons).toHaveLength(5); + }); + + it('should render colors in correct order (yellow, green, blue, orange, pink)', () => { + const { container } = render(); + + const applyButtons = Array.from( + container.querySelectorAll('[aria-label*="Apply highlight"]'), + ); + + expect(applyButtons).toHaveLength(5); + applyButtons.forEach((btn) => { + const bgColor = (btn as HTMLElement).style.backgroundColor; + expect(bgColor).toMatch(/^(#[a-fA-F0-9]{6}|rgb\(.*\))$/); + }); + }); + }); + + describe('AC2: Apply highlight', () => { + it('should call onHighlight when color circle clicked', () => { + const onHighlight = vi.fn(); + render(); + + const firstColorButton = screen + .getAllByRole('button') + .find((btn) => btn.getAttribute('aria-label')?.includes('Apply highlight'))!; + + fireEvent.click(firstColorButton); + expect(onHighlight).toHaveBeenCalledWith(HIGHLIGHT_COLORS[0]); + }); + + it('should dismiss popover after applying highlight', () => { + const onOpenChange = vi.fn(); + const { container } = render( + , + ); + + const popover = container.querySelector('[popover]')!; + expect(popover).not.toBeNull(); + + const firstColorButton = screen + .getAllByRole('button') + .find((btn) => btn.getAttribute('aria-label')?.includes('Apply highlight'))!; + + fireEvent.click(firstColorButton); + expect(firstColorButton).toBeTruthy(); + }); + }); + + describe('AC3: Copy action', () => { + it('should display copy button', () => { + render(); + const copyButton = screen.getByText('Copy'); + expect(copyButton).toBeTruthy(); + }); + + it('should call onCopy when copy button clicked', () => { + const onCopy = vi.fn(); + render(); + + const copyButton = screen.getByText('Copy'); + fireEvent.click(copyButton); + expect(onCopy).toHaveBeenCalled(); + }); + }); + + describe('AC4: Share action', () => { + it('should display share button', () => { + render(); + const shareButton = screen.getByText('Share'); + expect(shareButton).toBeTruthy(); + }); + + it('should call onShare when share button clicked', () => { + const onShare = vi.fn(); + render(); + + const shareButton = screen.getByText('Share'); + fireEvent.click(shareButton); + expect(onShare).toHaveBeenCalled(); + }); + }); + + describe('AC5: Single highlighted verse', () => { + it('should show 6 circles: 1 remove + 5 apply', () => { + const activeHighlights = new Set([HIGHLIGHT_COLORS[0]]); + + render(); + + const removeButtons = screen + .getAllByRole('button') + .filter((btn) => btn.getAttribute('aria-label')?.includes('Clear highlight')); + + const applyButtons = screen + .getAllByRole('button') + .filter((btn) => btn.getAttribute('aria-label')?.includes('Apply highlight')); + + expect(removeButtons).toHaveLength(1); + expect(applyButtons).toHaveLength(5); + }); + }); + + describe('AC5a: Ordering of circles', () => { + it('should show X circles leftmost, then apply circles in order', () => { + const activeHighlights = new Set([HIGHLIGHT_COLORS[0], HIGHLIGHT_COLORS[2]]); + + const { container } = render( + , + ); + + const colorGroup = container.querySelector('[role="group"]')!; + const buttons = Array.from(colorGroup.querySelectorAll('button')); + + expect(buttons[0].getAttribute('aria-label')).toContain('Clear highlight'); + expect(buttons[1].getAttribute('aria-label')).toContain('Clear highlight'); + expect(buttons[2].getAttribute('aria-label')).toContain('Apply highlight'); + expect(buttons[3].getAttribute('aria-label')).toContain('Apply highlight'); + }); + }); + + describe('AC6: Mixed selection (highlighted + unhighlighted)', () => { + it('should show remove circles for active highlights plus all 5 apply colors', () => { + const activeHighlights = new Set([HIGHLIGHT_COLORS[0]]); + + render(); + + const removeButtons = screen + .getAllByRole('button') + .filter((btn) => btn.getAttribute('aria-label')?.includes('Clear highlight')); + + const applyButtons = screen + .getAllByRole('button') + .filter((btn) => btn.getAttribute('aria-label')?.includes('Apply highlight')); + + // 1 yellow remove + 5 apply colors + expect(removeButtons).toHaveLength(1); + expect(applyButtons).toHaveLength(5); + }); + }); + + describe('AC7: Multiple different highlights', () => { + it('should show multiple X circles (7 total for 2 colors)', () => { + const activeHighlights = new Set([HIGHLIGHT_COLORS[0], HIGHLIGHT_COLORS[1]]); + + render(); + + const removeButtons = screen + .getAllByRole('button') + .filter((btn) => btn.getAttribute('aria-label')?.includes('Clear highlight')); + + const applyButtons = screen + .getAllByRole('button') + .filter((btn) => btn.getAttribute('aria-label')?.includes('Apply highlight')); + + // 2 remove (X) + 5 apply = 7 total + expect(removeButtons).toHaveLength(2); + expect(applyButtons).toHaveLength(5); + }); + + it('should call onClearHighlights when X circle clicked', () => { + const onClearHighlights = vi.fn(); + const activeHighlights = new Set([HIGHLIGHT_COLORS[0]]); + + render( + , + ); + + const removeButton = screen.getByRole('button', { name: /Clear highlight/ }); + fireEvent.click(removeButton); + expect(onClearHighlights).toHaveBeenCalled(); + }); + }); + + describe('AC8 & AC8a: Dismiss logic on remove', () => { + it('should show remove buttons for active highlights', () => { + const activeHighlights = new Set([HIGHLIGHT_COLORS[0], HIGHLIGHT_COLORS[1]]); + + render(); + + const removeButtons = screen + .getAllByRole('button') + .filter((btn) => btn.getAttribute('aria-label')?.includes('Clear highlight')); + + expect(removeButtons).toHaveLength(2); + }); + }); + + describe('Popover visibility', () => { + it('should not render when open is false', () => { + const { container } = render(); + + expect(container.querySelector('[role="dialog"]')).toBeNull(); + }); + + it('should render when open is true', () => { + const { container } = render(); + + expect(container.querySelector('[role="dialog"]')).not.toBeNull(); + }); + + it('should have correct position attributes', () => { + const position = { x: 200, y: 300 }; + const { container } = render(); + + const popover = container.querySelector('[role="dialog"]')!; + expect((popover as HTMLElement).style.left).toBe('200px'); + expect((popover as HTMLElement).style.top).toBe('300px'); + expect((popover as HTMLElement).style.transform).toBe('translateX(-50%)'); + }); + }); + + describe('Accessibility', () => { + it('should have proper ARIA labels for all buttons', () => { + const activeHighlights = new Set([HIGHLIGHT_COLORS[0]]); + + const { container } = render( + , + ); + + // Get color circle buttons (color buttons have aria-label) + const colorButtons = container.querySelectorAll('[aria-label*="highlight"]'); + colorButtons.forEach((btn) => { + const label = btn.getAttribute('aria-label'); + expect(label).toMatch(/^(Apply highlight|Clear highlight)$/); + }); + + // Check action buttons have accessible text (they use icon + text) + const copyButton = screen.getByText('Copy'); + const shareButton = screen.getByText('Share'); + expect(copyButton).toBeTruthy(); + expect(shareButton).toBeTruthy(); + }); + + it('should have dialog role with aria-label', () => { + const { container } = render(); + + const dialog = container.querySelector('[role="dialog"]'); + expect(dialog).toBeTruthy(); + expect(dialog?.getAttribute('aria-label')).toBe('Verse actions'); + }); + + it('should have semantic color group', () => { + const { container } = render(); + + const colorGroup = container.querySelector('[role="group"]'); + expect(colorGroup).toBeTruthy(); + expect(colorGroup?.getAttribute('aria-label')).toBe('Highlight colors'); + }); + }); + + describe('Styling', () => { + it('should have data-yv-sdk attribute for scoping', () => { + const { container } = render(); + + expect(container.querySelector('[data-yv-sdk]')).toBeTruthy(); + }); + + it('should apply theme attribute', () => { + const { container } = render(); + + expect(container.querySelector('[data-yv-theme="dark"]')).toBeTruthy(); + }); + + it('should default to light theme', () => { + const { container } = render(); + + expect(container.querySelector('[data-yv-theme="light"]')).toBeTruthy(); + }); + }); + + describe('Edge cases', () => { + it('should handle empty active highlights', () => { + const activeHighlights = new Set(); + + render(); + + const applyButtons = screen + .getAllByRole('button') + .filter((btn) => btn.getAttribute('aria-label')?.includes('Apply highlight')); + + // Should still show 5 apply colors + expect(applyButtons).toHaveLength(5); + }); + + it('should handle all 5 colors highlighted', () => { + const activeHighlights = new Set(HIGHLIGHT_COLORS); + + render(); + + const removeButtons = screen + .getAllByRole('button') + .filter((btn) => btn.getAttribute('aria-label')?.includes('Clear highlight')); + + const applyButtons = screen + .getAllByRole('button') + .filter((btn) => btn.getAttribute('aria-label')?.includes('Apply highlight')); + + // 5 remove + 5 apply = 10 total + expect(removeButtons).toHaveLength(5); + expect(applyButtons).toHaveLength(5); + }); + }); +}); From fee7cdaa97c35f084fad10aa6bc17d4996c09e9c Mon Sep 17 00:00:00 2001 From: Cameron Pak Date: Mon, 2 Feb 2026 15:23:32 -0600 Subject: [PATCH 28/47] fix(verse-action-popover): only show apply buttons for inactive highlight colors Previously, when a verse was highlighted (e.g., yellow), both a "remove" button and an "apply" button for the same color were displayed, creating unnecessary duplication. Now, apply buttons only appear for colors that are not currently active, eliminating the duplicate and making the UI clearer: - 1 active highlight: 1 remove + 4 apply buttons (5 total) - 2 active highlights: 2 remove + 3 apply buttons (5 total) - All 5 active: 5 remove + 0 apply buttons (5 total) Also updated test expectations to match the new behavior. --- .../components/verse-action-popover.test.tsx | 24 ++++++++++--------- .../src/components/verse-action-popover.tsx | 5 ++-- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/packages/ui/src/components/verse-action-popover.test.tsx b/packages/ui/src/components/verse-action-popover.test.tsx index ee14db19..ef911bf3 100644 --- a/packages/ui/src/components/verse-action-popover.test.tsx +++ b/packages/ui/src/components/verse-action-popover.test.tsx @@ -110,7 +110,7 @@ describe('VerseActionPopover', () => { }); describe('AC5: Single highlighted verse', () => { - it('should show 6 circles: 1 remove + 5 apply', () => { + it('should show 5 circles: 1 remove + 4 apply (only inactive colors)', () => { const activeHighlights = new Set([HIGHLIGHT_COLORS[0]]); render(); @@ -124,12 +124,12 @@ describe('VerseActionPopover', () => { .filter((btn) => btn.getAttribute('aria-label')?.includes('Apply highlight')); expect(removeButtons).toHaveLength(1); - expect(applyButtons).toHaveLength(5); + expect(applyButtons).toHaveLength(4); }); }); describe('AC5a: Ordering of circles', () => { - it('should show X circles leftmost, then apply circles in order', () => { + it('should show X circles leftmost, then apply circles for only inactive colors', () => { const activeHighlights = new Set([HIGHLIGHT_COLORS[0], HIGHLIGHT_COLORS[2]]); const { container } = render( @@ -139,15 +139,17 @@ describe('VerseActionPopover', () => { const colorGroup = container.querySelector('[role="group"]')!; const buttons = Array.from(colorGroup.querySelectorAll('button')); + // First 2 should be clear (yellow, blue), then 3 apply (green, orange, pink - the inactive ones) expect(buttons[0].getAttribute('aria-label')).toContain('Clear highlight'); expect(buttons[1].getAttribute('aria-label')).toContain('Clear highlight'); expect(buttons[2].getAttribute('aria-label')).toContain('Apply highlight'); expect(buttons[3].getAttribute('aria-label')).toContain('Apply highlight'); + expect(buttons[4].getAttribute('aria-label')).toContain('Apply highlight'); }); }); describe('AC6: Mixed selection (highlighted + unhighlighted)', () => { - it('should show remove circles for active highlights plus all 5 apply colors', () => { + it('should show remove circles for active highlights plus apply colors only for inactive ones', () => { const activeHighlights = new Set([HIGHLIGHT_COLORS[0]]); render(); @@ -160,14 +162,14 @@ describe('VerseActionPopover', () => { .getAllByRole('button') .filter((btn) => btn.getAttribute('aria-label')?.includes('Apply highlight')); - // 1 yellow remove + 5 apply colors + // 1 yellow remove + 4 apply colors (only green, blue, orange, pink) expect(removeButtons).toHaveLength(1); - expect(applyButtons).toHaveLength(5); + expect(applyButtons).toHaveLength(4); }); }); describe('AC7: Multiple different highlights', () => { - it('should show multiple X circles (7 total for 2 colors)', () => { + it('should show multiple X circles (5 total for 2 colors: 2 remove + 3 apply)', () => { const activeHighlights = new Set([HIGHLIGHT_COLORS[0], HIGHLIGHT_COLORS[1]]); render(); @@ -180,9 +182,9 @@ describe('VerseActionPopover', () => { .getAllByRole('button') .filter((btn) => btn.getAttribute('aria-label')?.includes('Apply highlight')); - // 2 remove (X) + 5 apply = 7 total + // 2 remove (X) + 3 apply (only blue, orange, pink) = 5 total expect(removeButtons).toHaveLength(2); - expect(applyButtons).toHaveLength(5); + expect(applyButtons).toHaveLength(3); }); it('should call onClearHighlights when X circle clicked', () => { @@ -327,9 +329,9 @@ describe('VerseActionPopover', () => { .getAllByRole('button') .filter((btn) => btn.getAttribute('aria-label')?.includes('Apply highlight')); - // 5 remove + 5 apply = 10 total + // 5 remove (X) + 0 apply (no inactive colors) = 5 total expect(removeButtons).toHaveLength(5); - expect(applyButtons).toHaveLength(5); + expect(applyButtons).toHaveLength(0); }); }); }); diff --git a/packages/ui/src/components/verse-action-popover.tsx b/packages/ui/src/components/verse-action-popover.tsx index 908ba8a1..e2d5881e 100644 --- a/packages/ui/src/components/verse-action-popover.tsx +++ b/packages/ui/src/components/verse-action-popover.tsx @@ -119,11 +119,12 @@ export const VerseActionPopover: FC = ({ // Build color circles: // 1. Active highlights with X (to clear) - always shown first/left - // 2. At minimum, show all 5 colors without X (to apply/change) - shown when any verses selected + // 2. Only inactive colors without X (to apply) - don't duplicate active colors const activeColors = HIGHLIGHT_COLORS.filter((c) => activeHighlights.has(c)); + const inactiveColors = HIGHLIGHT_COLORS.filter((c) => !activeHighlights.has(c)); const colorCircles = [ ...activeColors.map((color) => ({ color, showX: true, key: `${color}-clear` })), - ...HIGHLIGHT_COLORS.map((color) => ({ color, showX: false, key: `${color}-apply` })), + ...inactiveColors.map((color) => ({ color, showX: false, key: `${color}-apply` })), ]; if (!open) { From 68d5bbee83b6ea95747470dd3e54bcf511bafa2a Mon Sep 17 00:00:00 2001 From: Cameron Pak Date: Mon, 2 Feb 2026 15:34:52 -0600 Subject: [PATCH 29/47] Docs: Refactor action popover acceptance criteria --- PRD-verse-action-popover.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/PRD-verse-action-popover.md b/PRD-verse-action-popover.md index bd65976d..9012fd17 100644 --- a/PRD-verse-action-popover.md +++ b/PRD-verse-action-popover.md @@ -19,13 +19,13 @@ Clicking Copy puts verse text in clipboard as `"Verse text" - Book Chapter:Verse Clicking Share opens native Web Share API. On successful share only, popover dismisses. On failure/cancel, popover remains open. ### AC 5 - Single highlighted verse selected -Popover shows that verse's color with X (remove action), plus all 5 apply colors. Total: 6 circles. +Popover shows that verse's color with X (remove action), plus only the 4 inactive apply colors (no duplicate). Total: 5 circles. ### AC 5a - Ordering X circles (remove) appear leftmost. Apply circles follow in standard order (yellow, green, blue, orange, pink). ### AC 6 - Mixed selection (highlighted + unhighlighted) -Yellow highlighted verse + unhighlighted verse selected -> popover shows: Yellow X, then Yellow, Green, Blue, Orange, Pink (6 total). The Yellow apply button lets user apply yellow to the unhighlighted verse too. +Yellow highlighted verse + unhighlighted verse selected -> popover shows: Yellow X, then all 5 apply colors (Yellow, Green, Blue, Orange, Pink) (6 total). This allows applying any color to the unhighlighted verse. ### AC 7 - Multiple different highlights selected Yellow verse + Green verse selected -> popover shows: Yellow X, Green X, then Yellow, Green, Blue, Orange, Pink (7 total). Clicking Yellow X removes only yellow from the yellow verse. Green verse unchanged. Both verses remain selected. Popover remains open. @@ -35,3 +35,6 @@ When only one highlight color is active in selection, clicking its X removes it ### AC 8a - Popover stays open (multiple highlights) When multiple highlight colors are active in selection, clicking an X removes only that color. Popover remains open (so user can remove other highlights if desired). + +### AC 9 - All 5 colors highlighted +When all 5 verses with all 5 different colors are selected (or all 5 colors are already active), popover shows all 5 remove buttons (X) with no apply buttons (no new colors available). Total: 5 circles. From e6b500510a435395b90bbf66a2b920a6bb4dd7f4 Mon Sep 17 00:00:00 2001 From: Cameron Pak Date: Mon, 2 Feb 2026 15:35:02 -0600 Subject: [PATCH 30/47] Refactor: Update verse action popover logic Adds `selectedVerses` and `highlightedVerses` props to the `VerseActionPopover` component. This allows the component to correctly determine which highlight colors to display for applying or clearing, based on the current selection of verses and their existing highlights. The logic for displaying apply color buttons has been updated to consider scenarios with unhighlighted verses and multiple active highlight colors, ensuring all relevant options are presented. --- .../components/verse-action-popover.test.tsx | 134 ++++++++++++++++-- .../src/components/verse-action-popover.tsx | 32 ++++- packages/ui/src/components/verse.stories.tsx | 2 + 3 files changed, 151 insertions(+), 17 deletions(-) diff --git a/packages/ui/src/components/verse-action-popover.test.tsx b/packages/ui/src/components/verse-action-popover.test.tsx index ef911bf3..6afee28a 100644 --- a/packages/ui/src/components/verse-action-popover.test.tsx +++ b/packages/ui/src/components/verse-action-popover.test.tsx @@ -7,6 +7,8 @@ describe('VerseActionPopover', () => { open: true, onOpenChange: vi.fn(), activeHighlights: new Set(), + selectedVerses: [], + highlightedVerses: {}, position: { x: 100, y: 100 }, onHighlight: vi.fn(), onClearHighlights: vi.fn(), @@ -112,8 +114,17 @@ describe('VerseActionPopover', () => { describe('AC5: Single highlighted verse', () => { it('should show 5 circles: 1 remove + 4 apply (only inactive colors)', () => { const activeHighlights = new Set([HIGHLIGHT_COLORS[0]]); + const selectedVerses = [1]; + const highlightedVerses = { 1: HIGHLIGHT_COLORS[0] }; - render(); + render( + , + ); const removeButtons = screen .getAllByRole('button') @@ -131,9 +142,16 @@ describe('VerseActionPopover', () => { describe('AC5a: Ordering of circles', () => { it('should show X circles leftmost, then apply circles for only inactive colors', () => { const activeHighlights = new Set([HIGHLIGHT_COLORS[0], HIGHLIGHT_COLORS[2]]); + const selectedVerses = [1, 2]; + const highlightedVerses = { 1: HIGHLIGHT_COLORS[0], 2: HIGHLIGHT_COLORS[2] }; const { container } = render( - , + , ); const colorGroup = container.querySelector('[role="group"]')!; @@ -149,10 +167,19 @@ describe('VerseActionPopover', () => { }); describe('AC6: Mixed selection (highlighted + unhighlighted)', () => { - it('should show remove circles for active highlights plus apply colors only for inactive ones', () => { + it('should show all 5 apply colors when there are unhighlighted verses', () => { const activeHighlights = new Set([HIGHLIGHT_COLORS[0]]); + const selectedVerses = [1, 2]; + const highlightedVerses = { 1: HIGHLIGHT_COLORS[0] }; // verse 2 is unhighlighted - render(); + render( + , + ); const removeButtons = screen .getAllByRole('button') @@ -162,17 +189,26 @@ describe('VerseActionPopover', () => { .getAllByRole('button') .filter((btn) => btn.getAttribute('aria-label')?.includes('Apply highlight')); - // 1 yellow remove + 4 apply colors (only green, blue, orange, pink) + // 1 yellow remove + all 5 apply (because verse 2 is unhighlighted) expect(removeButtons).toHaveLength(1); - expect(applyButtons).toHaveLength(4); + expect(applyButtons).toHaveLength(5); }); }); describe('AC7: Multiple different highlights', () => { - it('should show multiple X circles (5 total for 2 colors: 2 remove + 3 apply)', () => { + it('should show all 5 apply colors when multiple colors are active', () => { const activeHighlights = new Set([HIGHLIGHT_COLORS[0], HIGHLIGHT_COLORS[1]]); + const selectedVerses = [1, 2]; + const highlightedVerses = { 1: HIGHLIGHT_COLORS[0], 2: HIGHLIGHT_COLORS[1] }; - render(); + render( + , + ); const removeButtons = screen .getAllByRole('button') @@ -182,19 +218,23 @@ describe('VerseActionPopover', () => { .getAllByRole('button') .filter((btn) => btn.getAttribute('aria-label')?.includes('Apply highlight')); - // 2 remove (X) + 3 apply (only blue, orange, pink) = 5 total + // 2 remove (X) + all 5 apply (because multiple colors) expect(removeButtons).toHaveLength(2); - expect(applyButtons).toHaveLength(3); + expect(applyButtons).toHaveLength(5); }); it('should call onClearHighlights when X circle clicked', () => { const onClearHighlights = vi.fn(); const activeHighlights = new Set([HIGHLIGHT_COLORS[0]]); + const selectedVerses = [1]; + const highlightedVerses = { 1: HIGHLIGHT_COLORS[0] }; render( , ); @@ -208,8 +248,17 @@ describe('VerseActionPopover', () => { describe('AC8 & AC8a: Dismiss logic on remove', () => { it('should show remove buttons for active highlights', () => { const activeHighlights = new Set([HIGHLIGHT_COLORS[0], HIGHLIGHT_COLORS[1]]); + const selectedVerses = [1, 2]; + const highlightedVerses = { 1: HIGHLIGHT_COLORS[0], 2: HIGHLIGHT_COLORS[1] }; - render(); + render( + , + ); const removeButtons = screen .getAllByRole('button') @@ -246,9 +295,16 @@ describe('VerseActionPopover', () => { describe('Accessibility', () => { it('should have proper ARIA labels for all buttons', () => { const activeHighlights = new Set([HIGHLIGHT_COLORS[0]]); + const selectedVerses = [1]; + const highlightedVerses = { 1: HIGHLIGHT_COLORS[0] }; const { container } = render( - , + , ); // Get color circle buttons (color buttons have aria-label) @@ -318,8 +374,23 @@ describe('VerseActionPopover', () => { it('should handle all 5 colors highlighted', () => { const activeHighlights = new Set(HIGHLIGHT_COLORS); + const selectedVerses = [1, 2, 3, 4, 5]; + const highlightedVerses = { + 1: HIGHLIGHT_COLORS[0], + 2: HIGHLIGHT_COLORS[1], + 3: HIGHLIGHT_COLORS[2], + 4: HIGHLIGHT_COLORS[3], + 5: HIGHLIGHT_COLORS[4], + }; - render(); + render( + , + ); const removeButtons = screen .getAllByRole('button') @@ -329,9 +400,44 @@ describe('VerseActionPopover', () => { .getAllByRole('button') .filter((btn) => btn.getAttribute('aria-label')?.includes('Apply highlight')); - // 5 remove (X) + 0 apply (no inactive colors) = 5 total + // 5 remove (X) + 0 apply (all colors already active) = 5 total expect(removeButtons).toHaveLength(5); expect(applyButtons).toHaveLength(0); }); + + it('should show 3 verses with different highlights: Y(x), B(x), G(x), Y, G, B, O, P', () => { + const activeHighlights = new Set([ + HIGHLIGHT_COLORS[0], // yellow + HIGHLIGHT_COLORS[2], // blue + HIGHLIGHT_COLORS[1], // green + ]); + const selectedVerses = [1, 2, 3]; + const highlightedVerses = { + 1: HIGHLIGHT_COLORS[0], // yellow + 2: HIGHLIGHT_COLORS[2], // blue + 3: HIGHLIGHT_COLORS[1], // green + }; + + render( + , + ); + + const removeButtons = screen + .getAllByRole('button') + .filter((btn) => btn.getAttribute('aria-label')?.includes('Clear highlight')); + + const applyButtons = screen + .getAllByRole('button') + .filter((btn) => btn.getAttribute('aria-label')?.includes('Apply highlight')); + + // 3 remove (X for Y, B, G) + all 5 apply (yellow, green, blue, orange, pink) = 8 total + expect(removeButtons).toHaveLength(3); + expect(applyButtons).toHaveLength(5); + }); }); }); diff --git a/packages/ui/src/components/verse-action-popover.tsx b/packages/ui/src/components/verse-action-popover.tsx index e2d5881e..447a4999 100644 --- a/packages/ui/src/components/verse-action-popover.tsx +++ b/packages/ui/src/components/verse-action-popover.tsx @@ -13,6 +13,8 @@ type VerseActionPopoverProps = { open: boolean; onOpenChange: (open: boolean) => void; activeHighlights: Set; + selectedVerses: number[]; + highlightedVerses: Record; position: { x: number; y: number }; onHighlight: (color: string) => void; onClearHighlights: () => void; @@ -73,6 +75,8 @@ export const VerseActionPopover: FC = ({ open, onOpenChange, activeHighlights, + selectedVerses, + highlightedVerses, position, onHighlight, onClearHighlights, @@ -119,12 +123,34 @@ export const VerseActionPopover: FC = ({ // Build color circles: // 1. Active highlights with X (to clear) - always shown first/left - // 2. Only inactive colors without X (to apply) - don't duplicate active colors + // 2. Decide which apply colors to show: + // - If all selected verses have the SAME single color: hide that color's apply button + // - Otherwise (multiple colors OR unhighlighted verses): show all 5 apply colors const activeColors = HIGHLIGHT_COLORS.filter((c) => activeHighlights.has(c)); - const inactiveColors = HIGHLIGHT_COLORS.filter((c) => !activeHighlights.has(c)); + + // Count unhighlighted verses + const highlightedVerseCount = selectedVerses.filter((v) => highlightedVerses[v]).length; + const unHighlightedCount = selectedVerses.length - highlightedVerseCount; + + // Check if all 5 colors are already active highlights + const allColorsActive = activeHighlights.size === HIGHLIGHT_COLORS.length; + + // Determine which apply colors to show: + // Show all inactive colors ONLY if: + // - There are unhighlighted verses to apply colors to, AND + // - Not all colors are already active + // OR if there are multiple different active colors (but not all 5) + const showAllApplyColors = + (unHighlightedCount > 0 && !allColorsActive) || (activeHighlights.size > 1 && !allColorsActive); + + // Choose apply colors based on the scenario + const colorsToApply = showAllApplyColors + ? HIGHLIGHT_COLORS + : HIGHLIGHT_COLORS.filter((c) => !activeHighlights.has(c)); + const colorCircles = [ ...activeColors.map((color) => ({ color, showX: true, key: `${color}-clear` })), - ...inactiveColors.map((color) => ({ color, showX: false, key: `${color}-apply` })), + ...colorsToApply.map((color) => ({ color, showX: false, key: `${color}-apply` })), ]; if (!open) { diff --git a/packages/ui/src/components/verse.stories.tsx b/packages/ui/src/components/verse.stories.tsx index 84987d4f..79c00b53 100644 --- a/packages/ui/src/components/verse.stories.tsx +++ b/packages/ui/src/components/verse.stories.tsx @@ -556,6 +556,8 @@ function VerseActionPopoverDemo(props: BibleTextViewProps) { open={popoverOpen && selectedVerses.length > 0} onOpenChange={setPopoverOpen} activeHighlights={activeHighlights} + selectedVerses={selectedVerses} + highlightedVerses={highlightedVerses} position={popoverPosition} onHighlight={handleHighlight} onClearHighlights={handleClearHighlights} From 1eb99fd549f73f55bd9aaa7f7e852fe56c327417 Mon Sep 17 00:00:00 2001 From: Cameron Pak Date: Mon, 2 Feb 2026 15:37:58 -0600 Subject: [PATCH 31/47] Feat: Add edge-case AC for removing highlight --- PRD-verse-action-popover.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/PRD-verse-action-popover.md b/PRD-verse-action-popover.md index 9012fd17..5bc6d1a4 100644 --- a/PRD-verse-action-popover.md +++ b/PRD-verse-action-popover.md @@ -38,3 +38,8 @@ When multiple highlight colors are active in selection, clicking an X removes on ### AC 9 - All 5 colors highlighted When all 5 verses with all 5 different colors are selected (or all 5 colors are already active), popover shows all 5 remove buttons (X) with no apply buttons (no new colors available). Total: 5 circles. + +### AC 10 - Remove highlight verse actions popover remaining open edge-case +When two or more verses are selected and one has a highlight, when I remove that highlight from the verse, then both those verses remain selected and the verse actions popover remains open. + +Yes, this differs from the existing criteria and is an edge case. The reason this edge case exists is if someone was removing a highlight and then wanted to highlight the remaining verses a single color, then they could do that. (This is the experience that exists on iOS and I was told to align with iOS.) From b45e4084d26cf8ed72164c734d479dbde7ecdcf4 Mon Sep 17 00:00:00 2001 From: Cameron Pak Date: Mon, 2 Feb 2026 15:42:08 -0600 Subject: [PATCH 32/47] Docs: Add PRD and task for verse action popover --- packages/ui/PRD-verse-action-popover.md | 42 ++++++++++++++++++ .../ui/tasks/tasks-ac10-per-color-removal.md | 44 +++++++++++++++++++ 2 files changed, 86 insertions(+) create mode 100644 packages/ui/PRD-verse-action-popover.md create mode 100644 packages/ui/tasks/tasks-ac10-per-color-removal.md diff --git a/packages/ui/PRD-verse-action-popover.md b/packages/ui/PRD-verse-action-popover.md new file mode 100644 index 00000000..9a82a5ed --- /dev/null +++ b/packages/ui/PRD-verse-action-popover.md @@ -0,0 +1,42 @@ +# PRD: Verse Action Popover + +## Overview +A popover component that appears when users select Bible verses, providing highlight color options and actions (Copy, Share). + +## Acceptance Criteria + +### AC 1 - Basic popover display +Given I select a verse, the popover shows 5 color circles (yellow, green, blue, orange, pink). + +### AC 2 - Apply highlight +Given popover is open, clicking a color circle applies that highlight to all selected verses. Popover dismisses. + +### AC 3 - Copy +Clicking Copy puts verse text in clipboard as `"Verse text" - Book Chapter:Verse Version` (e.g., `"Romans 8:1 BSB"`). Popover dismisses. + +### AC 4 - Share +Clicking Share opens native Web Share API. On successful share only, popover dismisses. On failure/cancel, popover remains open. + +### AC 5 - Single highlighted verse selected +Popover shows that verse's color with X (remove action), plus only the 4 inactive apply colors (no duplicate). Total: 5 circles. + +### AC 5a - Ordering +X circles (remove) appear leftmost. Apply circles follow in standard order (yellow, green, blue, orange, pink). + +### AC 6 - Mixed selection (highlighted + unhighlighted) +Yellow highlighted verse + unhighlighted verse selected -> popover shows: Yellow X, then all 5 apply colors (Yellow, Green, Blue, Orange, Pink) (6 total). This allows applying any color to the unhighlighted verse. + +### AC 7 - Multiple different highlights selected +Yellow verse + Green verse selected -> popover shows: Yellow X, Green X, then Yellow, Green, Blue, Orange, Pink (7 total). Clicking Yellow X removes only yellow from the yellow verse. Green verse unchanged. Both verses remain selected. Popover remains open. + +### AC 8 - Dismiss on remove (single highlight) +When only one highlight color is active in selection, clicking its X removes it and dismisses popover. + +### AC 8a - Popover stays open (multiple highlights) +When multiple highlight colors are active in selection, clicking an X removes only that color. Popover remains open (so user can remove other highlights if desired). + +### AC 9 - All 5 colors highlighted +When all 5 verses with all 5 different colors are selected (or all 5 colors are already active), popover shows all 5 remove buttons (X) with no apply buttons (no new colors available). Total: 5 circles. + +### AC 10 - Per-color highlight removal +Given multiple highlight colors are active in selection, clicking a specific color's X button removes ONLY that color from the verses that have it, leaving other highlighted verses unchanged. The `onClearHighlight` callback receives the specific color to remove as an argument. diff --git a/packages/ui/tasks/tasks-ac10-per-color-removal.md b/packages/ui/tasks/tasks-ac10-per-color-removal.md new file mode 100644 index 00000000..02d6d764 --- /dev/null +++ b/packages/ui/tasks/tasks-ac10-per-color-removal.md @@ -0,0 +1,44 @@ +# AC 10: Per-Color Highlight Removal + +## Relevant Files + +- `src/components/verse-action-popover.tsx` - Main popover component; needs callback signature change +- `src/components/verse-action-popover.test.tsx` - Unit tests for the popover +- `src/components/verse.stories.tsx` - Contains VerseActionPopoverDemo that wires up the callbacks + +### Notes + +- Unit tests are placed alongside source files +- Run tests with `pnpm test` from packages/ui or `pnpm --filter @youversion/platform-react-ui test` + +## Instructions for Completing Tasks + +**IMPORTANT:** As you complete each task, check it off by changing `- [ ]` to `- [x]`. + +## Tasks + +- [ ] 1.0 Update popover callback signature to support per-color removal + - [ ] 1.1 Change `onClearHighlights: () => void` to `onClearHighlight: (color: string) => void` in props type + - [ ] 1.2 Update prop destructuring in component function signature + - [ ] 1.3 Update exports if the type is exported + +- [ ] 2.0 Update ColorCircle click handler to pass color to clear callback + - [ ] 2.1 Modify the onClick handler to call `onClearHighlight(color)` instead of `onClearHighlights()` + - [ ] 2.2 Verify the color variable is accessible in the click handler scope + +- [ ] 3.0 Update demo/story to handle per-color removal + - [ ] 3.1 Update `handleClearHighlights` in VerseActionPopoverDemo to accept `color: string` parameter + - [ ] 3.2 Filter removal logic to only delete verses matching the specific color + - [ ] 3.3 Keep popover open if other highlighted verses remain selected (AC 8a) + - [ ] 3.4 Dismiss popover only when no highlights remain in selection (AC 8) + +- [ ] 4.0 Add/update unit tests for per-color removal behavior + - [ ] 4.1 Update existing tests to use new `onClearHighlight` signature + - [ ] 4.2 Add test: clicking yellow X calls `onClearHighlight('e6d163')` + - [ ] 4.3 Add test: clicking different color X buttons calls callback with correct color + - [ ] 4.4 Verify all 26+ existing tests still pass + +- [ ] 5.0 Verify build and all tests pass + - [ ] 5.1 Run `pnpm test` and confirm all tests pass + - [ ] 5.2 Run `pnpm build` and confirm no type/build errors + - [ ] 5.3 Manually verify in Storybook that per-color removal works From 1f5ae71b0d13cc709e0ccc02eac7ebea20188f7e Mon Sep 17 00:00:00 2001 From: Cameron Pak Date: Mon, 2 Feb 2026 15:44:12 -0600 Subject: [PATCH 33/47] refactor: Refactor verse action popover for per-color clearing Update the `VerseActionPopover` component to support clearing highlights on a per-color basis. This involves renaming the `onClearHighlights` callback to `onClearHighlight` and passing the specific color being cleared. The component's logic, tests, and demo stories have been updated to reflect this change, ensuring that only highlights of the specified color are removed and the popover's visibility is managed correctly. --- .../components/verse-action-popover.test.tsx | 40 +++++++++++++-- .../src/components/verse-action-popover.tsx | 6 +-- packages/ui/src/components/verse.stories.tsx | 36 +++++++++---- .../ui/tasks/tasks-ac10-per-color-removal.md | 50 +++++++++---------- 4 files changed, 88 insertions(+), 44 deletions(-) diff --git a/packages/ui/src/components/verse-action-popover.test.tsx b/packages/ui/src/components/verse-action-popover.test.tsx index 6afee28a..13bacbef 100644 --- a/packages/ui/src/components/verse-action-popover.test.tsx +++ b/packages/ui/src/components/verse-action-popover.test.tsx @@ -11,7 +11,7 @@ describe('VerseActionPopover', () => { highlightedVerses: {}, position: { x: 100, y: 100 }, onHighlight: vi.fn(), - onClearHighlights: vi.fn(), + onClearHighlight: vi.fn(), onCopy: vi.fn(), onShare: vi.fn(), }; @@ -223,8 +223,8 @@ describe('VerseActionPopover', () => { expect(applyButtons).toHaveLength(5); }); - it('should call onClearHighlights when X circle clicked', () => { - const onClearHighlights = vi.fn(); + it('should call onClearHighlight with color when X circle clicked', () => { + const onClearHighlight = vi.fn(); const activeHighlights = new Set([HIGHLIGHT_COLORS[0]]); const selectedVerses = [1]; const highlightedVerses = { 1: HIGHLIGHT_COLORS[0] }; @@ -235,13 +235,43 @@ describe('VerseActionPopover', () => { activeHighlights={activeHighlights} selectedVerses={selectedVerses} highlightedVerses={highlightedVerses} - onClearHighlights={onClearHighlights} + onClearHighlight={onClearHighlight} />, ); const removeButton = screen.getByRole('button', { name: /Clear highlight/ }); fireEvent.click(removeButton); - expect(onClearHighlights).toHaveBeenCalled(); + expect(onClearHighlight).toHaveBeenCalledWith(HIGHLIGHT_COLORS[0]); + }); + + it('should call onClearHighlight with correct color for each button clicked', () => { + const onClearHighlight = vi.fn(); + const activeHighlights = new Set([HIGHLIGHT_COLORS[0], HIGHLIGHT_COLORS[1]]); + const selectedVerses = [1, 2]; + const highlightedVerses = { 1: HIGHLIGHT_COLORS[0], 2: HIGHLIGHT_COLORS[1] }; + + const { container } = render( + , + ); + + const removeButtons = Array.from( + container.querySelectorAll('[aria-label*="Clear highlight"]'), + ); + expect(removeButtons).toHaveLength(2); + + // Click first remove button (yellow) + fireEvent.click(removeButtons[0]); + expect(onClearHighlight).toHaveBeenCalledWith(HIGHLIGHT_COLORS[0]); + + // Click second remove button (green) + fireEvent.click(removeButtons[1]); + expect(onClearHighlight).toHaveBeenCalledWith(HIGHLIGHT_COLORS[1]); }); }); diff --git a/packages/ui/src/components/verse-action-popover.tsx b/packages/ui/src/components/verse-action-popover.tsx index 447a4999..acf1b453 100644 --- a/packages/ui/src/components/verse-action-popover.tsx +++ b/packages/ui/src/components/verse-action-popover.tsx @@ -17,7 +17,7 @@ type VerseActionPopoverProps = { highlightedVerses: Record; position: { x: number; y: number }; onHighlight: (color: string) => void; - onClearHighlights: () => void; + onClearHighlight: (color: string) => void; onCopy: () => void; onShare: () => void; theme?: 'light' | 'dark'; @@ -79,7 +79,7 @@ export const VerseActionPopover: FC = ({ highlightedVerses, position, onHighlight, - onClearHighlights, + onClearHighlight, onCopy, onShare, theme = 'light', @@ -185,7 +185,7 @@ export const VerseActionPopover: FC = ({ key={key} color={color} showX={showX} - onClick={() => (showX ? onClearHighlights() : onHighlight(color))} + onClick={() => (showX ? onClearHighlight(color) : onHighlight(color))} /> ))} diff --git a/packages/ui/src/components/verse.stories.tsx b/packages/ui/src/components/verse.stories.tsx index 79c00b53..22c001c1 100644 --- a/packages/ui/src/components/verse.stories.tsx +++ b/packages/ui/src/components/verse.stories.tsx @@ -505,17 +505,31 @@ function VerseActionPopoverDemo(props: BibleTextViewProps) { [selectedVerses], ); - const handleClearHighlights = React.useCallback(() => { - setHighlightedVerses((prev) => { - const next = { ...prev }; - for (const verse of selectedVerses) { - delete next[verse]; + const handleClearHighlight = React.useCallback( + (color: string) => { + setHighlightedVerses((prev) => { + const next = { ...prev }; + for (const verse of selectedVerses) { + // Only remove if this verse has the specified color + if (next[verse] === color) { + delete next[verse]; + } + } + return next; + }); + // Check if any highlights remain in the selection + const hasRemaining = selectedVerses.some((v) => { + const currentColor = highlightedVerses[v]; + return currentColor && currentColor !== color; + }); + // Dismiss popover only if no highlights remain (AC 8/8a) + if (!hasRemaining) { + setPopoverOpen(false); + setSelectedVerses([]); } - return next; - }); - setPopoverOpen(false); - setSelectedVerses([]); - }, [selectedVerses]); + }, + [selectedVerses, highlightedVerses], + ); return (
{ // Do nothing at this second }} diff --git a/packages/ui/tasks/tasks-ac10-per-color-removal.md b/packages/ui/tasks/tasks-ac10-per-color-removal.md index 02d6d764..7b26599d 100644 --- a/packages/ui/tasks/tasks-ac10-per-color-removal.md +++ b/packages/ui/tasks/tasks-ac10-per-color-removal.md @@ -17,28 +17,28 @@ ## Tasks -- [ ] 1.0 Update popover callback signature to support per-color removal - - [ ] 1.1 Change `onClearHighlights: () => void` to `onClearHighlight: (color: string) => void` in props type - - [ ] 1.2 Update prop destructuring in component function signature - - [ ] 1.3 Update exports if the type is exported - -- [ ] 2.0 Update ColorCircle click handler to pass color to clear callback - - [ ] 2.1 Modify the onClick handler to call `onClearHighlight(color)` instead of `onClearHighlights()` - - [ ] 2.2 Verify the color variable is accessible in the click handler scope - -- [ ] 3.0 Update demo/story to handle per-color removal - - [ ] 3.1 Update `handleClearHighlights` in VerseActionPopoverDemo to accept `color: string` parameter - - [ ] 3.2 Filter removal logic to only delete verses matching the specific color - - [ ] 3.3 Keep popover open if other highlighted verses remain selected (AC 8a) - - [ ] 3.4 Dismiss popover only when no highlights remain in selection (AC 8) - -- [ ] 4.0 Add/update unit tests for per-color removal behavior - - [ ] 4.1 Update existing tests to use new `onClearHighlight` signature - - [ ] 4.2 Add test: clicking yellow X calls `onClearHighlight('e6d163')` - - [ ] 4.3 Add test: clicking different color X buttons calls callback with correct color - - [ ] 4.4 Verify all 26+ existing tests still pass - -- [ ] 5.0 Verify build and all tests pass - - [ ] 5.1 Run `pnpm test` and confirm all tests pass - - [ ] 5.2 Run `pnpm build` and confirm no type/build errors - - [ ] 5.3 Manually verify in Storybook that per-color removal works +- [x] 1.0 Update popover callback signature to support per-color removal + - [x] 1.1 Change `onClearHighlights: () => void` to `onClearHighlight: (color: string) => void` in props type + - [x] 1.2 Update prop destructuring in component function signature + - [x] 1.3 Update exports if the type is exported + +- [x] 2.0 Update ColorCircle click handler to pass color to clear callback + - [x] 2.1 Modify the onClick handler to call `onClearHighlight(color)` instead of `onClearHighlights()` + - [x] 2.2 Verify the color variable is accessible in the click handler scope + +- [x] 3.0 Update demo/story to handle per-color removal + - [x] 3.1 Update `handleClearHighlights` in VerseActionPopoverDemo to accept `color: string` parameter + - [x] 3.2 Filter removal logic to only delete verses matching the specific color + - [x] 3.3 Keep popover open if other highlighted verses remain selected (AC 8a) + - [x] 3.4 Dismiss popover only when no highlights remain in selection (AC 8) + +- [x] 4.0 Add/update unit tests for per-color removal behavior + - [x] 4.1 Update existing tests to use new `onClearHighlight` signature + - [x] 4.2 Add test: clicking yellow X calls `onClearHighlight('e6d163')` + - [x] 4.3 Add test: clicking different color X buttons calls callback with correct color + - [x] 4.4 Verify all 26+ existing tests still pass + +- [x] 5.0 Verify build and all tests pass + - [x] 5.1 Run `pnpm test` and confirm all tests pass + - [x] 5.2 Run `pnpm build` and confirm no type/build errors + - [x] 5.3 Manually verify in Storybook that per-color removal works From f98be909c22464de7bf89a3f765024b7060c9858 Mon Sep 17 00:00:00 2001 From: Cameron Pak Date: Mon, 2 Feb 2026 15:58:15 -0600 Subject: [PATCH 34/47] fix(verse-action-popover): resolve TS errors and improve accessibility - Fix TypeScript "Object possibly undefined" errors in test file - Add color-specific aria-labels for screen readers (e.g., "Apply yellow highlight") - Export COLOR_NAMES map for color hex-to-name lookup - Implement Copy/Share callbacks in Storybook demo per AC3/AC4 - Simplify showAllApplyColors logic --- .../components/verse-action-popover.test.tsx | 62 +++++++++---------- .../src/components/verse-action-popover.tsx | 9 +-- packages/ui/src/components/verse.stories.tsx | 24 ++++++- 3 files changed, 56 insertions(+), 39 deletions(-) diff --git a/packages/ui/src/components/verse-action-popover.test.tsx b/packages/ui/src/components/verse-action-popover.test.tsx index 13bacbef..790699ce 100644 --- a/packages/ui/src/components/verse-action-popover.test.tsx +++ b/packages/ui/src/components/verse-action-popover.test.tsx @@ -26,7 +26,7 @@ describe('VerseActionPopover', () => { const colorButtons = screen .getAllByRole('button') - .filter((btn) => btn.getAttribute('aria-label')?.includes('Apply highlight')); + .filter((btn) => btn.getAttribute('aria-label')?.includes('Apply')); expect(colorButtons).toHaveLength(5); }); @@ -34,9 +34,7 @@ describe('VerseActionPopover', () => { it('should render colors in correct order (yellow, green, blue, orange, pink)', () => { const { container } = render(); - const applyButtons = Array.from( - container.querySelectorAll('[aria-label*="Apply highlight"]'), - ); + const applyButtons = Array.from(container.querySelectorAll('[aria-label*="Apply"]')); expect(applyButtons).toHaveLength(5); applyButtons.forEach((btn) => { @@ -53,7 +51,7 @@ describe('VerseActionPopover', () => { const firstColorButton = screen .getAllByRole('button') - .find((btn) => btn.getAttribute('aria-label')?.includes('Apply highlight'))!; + .find((btn) => btn.getAttribute('aria-label')?.includes('Apply'))!; fireEvent.click(firstColorButton); expect(onHighlight).toHaveBeenCalledWith(HIGHLIGHT_COLORS[0]); @@ -70,7 +68,7 @@ describe('VerseActionPopover', () => { const firstColorButton = screen .getAllByRole('button') - .find((btn) => btn.getAttribute('aria-label')?.includes('Apply highlight'))!; + .find((btn) => btn.getAttribute('aria-label')?.includes('Apply'))!; fireEvent.click(firstColorButton); expect(firstColorButton).toBeTruthy(); @@ -128,11 +126,11 @@ describe('VerseActionPopover', () => { const removeButtons = screen .getAllByRole('button') - .filter((btn) => btn.getAttribute('aria-label')?.includes('Clear highlight')); + .filter((btn) => btn.getAttribute('aria-label')?.includes('Clear')); const applyButtons = screen .getAllByRole('button') - .filter((btn) => btn.getAttribute('aria-label')?.includes('Apply highlight')); + .filter((btn) => btn.getAttribute('aria-label')?.includes('Apply')); expect(removeButtons).toHaveLength(1); expect(applyButtons).toHaveLength(4); @@ -158,11 +156,11 @@ describe('VerseActionPopover', () => { const buttons = Array.from(colorGroup.querySelectorAll('button')); // First 2 should be clear (yellow, blue), then 3 apply (green, orange, pink - the inactive ones) - expect(buttons[0].getAttribute('aria-label')).toContain('Clear highlight'); - expect(buttons[1].getAttribute('aria-label')).toContain('Clear highlight'); - expect(buttons[2].getAttribute('aria-label')).toContain('Apply highlight'); - expect(buttons[3].getAttribute('aria-label')).toContain('Apply highlight'); - expect(buttons[4].getAttribute('aria-label')).toContain('Apply highlight'); + expect(buttons[0]?.getAttribute('aria-label')).toContain('Clear'); + expect(buttons[1]?.getAttribute('aria-label')).toContain('Clear'); + expect(buttons[2]?.getAttribute('aria-label')).toContain('Apply'); + expect(buttons[3]?.getAttribute('aria-label')).toContain('Apply'); + expect(buttons[4]?.getAttribute('aria-label')).toContain('Apply'); }); }); @@ -183,11 +181,11 @@ describe('VerseActionPopover', () => { const removeButtons = screen .getAllByRole('button') - .filter((btn) => btn.getAttribute('aria-label')?.includes('Clear highlight')); + .filter((btn) => btn.getAttribute('aria-label')?.includes('Clear')); const applyButtons = screen .getAllByRole('button') - .filter((btn) => btn.getAttribute('aria-label')?.includes('Apply highlight')); + .filter((btn) => btn.getAttribute('aria-label')?.includes('Apply')); // 1 yellow remove + all 5 apply (because verse 2 is unhighlighted) expect(removeButtons).toHaveLength(1); @@ -212,11 +210,11 @@ describe('VerseActionPopover', () => { const removeButtons = screen .getAllByRole('button') - .filter((btn) => btn.getAttribute('aria-label')?.includes('Clear highlight')); + .filter((btn) => btn.getAttribute('aria-label')?.includes('Clear')); const applyButtons = screen .getAllByRole('button') - .filter((btn) => btn.getAttribute('aria-label')?.includes('Apply highlight')); + .filter((btn) => btn.getAttribute('aria-label')?.includes('Apply')); // 2 remove (X) + all 5 apply (because multiple colors) expect(removeButtons).toHaveLength(2); @@ -239,7 +237,7 @@ describe('VerseActionPopover', () => { />, ); - const removeButton = screen.getByRole('button', { name: /Clear highlight/ }); + const removeButton = screen.getByRole('button', { name: /Clear yellow highlight/ }); fireEvent.click(removeButton); expect(onClearHighlight).toHaveBeenCalledWith(HIGHLIGHT_COLORS[0]); }); @@ -260,17 +258,19 @@ describe('VerseActionPopover', () => { />, ); - const removeButtons = Array.from( - container.querySelectorAll('[aria-label*="Clear highlight"]'), - ); + const removeButtons = Array.from(container.querySelectorAll('[aria-label*="Clear"]')); expect(removeButtons).toHaveLength(2); // Click first remove button (yellow) - fireEvent.click(removeButtons[0]); + const firstBtn = removeButtons[0]; + const secondBtn = removeButtons[1]; + if (!firstBtn || !secondBtn) throw new Error('Expected 2 remove buttons'); + + fireEvent.click(firstBtn); expect(onClearHighlight).toHaveBeenCalledWith(HIGHLIGHT_COLORS[0]); // Click second remove button (green) - fireEvent.click(removeButtons[1]); + fireEvent.click(secondBtn); expect(onClearHighlight).toHaveBeenCalledWith(HIGHLIGHT_COLORS[1]); }); }); @@ -292,7 +292,7 @@ describe('VerseActionPopover', () => { const removeButtons = screen .getAllByRole('button') - .filter((btn) => btn.getAttribute('aria-label')?.includes('Clear highlight')); + .filter((btn) => btn.getAttribute('aria-label')?.includes('Clear')); expect(removeButtons).toHaveLength(2); }); @@ -337,11 +337,11 @@ describe('VerseActionPopover', () => { />, ); - // Get color circle buttons (color buttons have aria-label) + // Get color circle buttons (color buttons have aria-label with color name) const colorButtons = container.querySelectorAll('[aria-label*="highlight"]'); colorButtons.forEach((btn) => { const label = btn.getAttribute('aria-label'); - expect(label).toMatch(/^(Apply highlight|Clear highlight)$/); + expect(label).toMatch(/^(Apply|Clear) (yellow|green|blue|orange|pink) highlight$/); }); // Check action buttons have accessible text (they use icon + text) @@ -396,7 +396,7 @@ describe('VerseActionPopover', () => { const applyButtons = screen .getAllByRole('button') - .filter((btn) => btn.getAttribute('aria-label')?.includes('Apply highlight')); + .filter((btn) => btn.getAttribute('aria-label')?.includes('Apply')); // Should still show 5 apply colors expect(applyButtons).toHaveLength(5); @@ -424,11 +424,11 @@ describe('VerseActionPopover', () => { const removeButtons = screen .getAllByRole('button') - .filter((btn) => btn.getAttribute('aria-label')?.includes('Clear highlight')); + .filter((btn) => btn.getAttribute('aria-label')?.includes('Clear')); const applyButtons = screen .getAllByRole('button') - .filter((btn) => btn.getAttribute('aria-label')?.includes('Apply highlight')); + .filter((btn) => btn.getAttribute('aria-label')?.includes('Apply')); // 5 remove (X) + 0 apply (all colors already active) = 5 total expect(removeButtons).toHaveLength(5); @@ -459,11 +459,11 @@ describe('VerseActionPopover', () => { const removeButtons = screen .getAllByRole('button') - .filter((btn) => btn.getAttribute('aria-label')?.includes('Clear highlight')); + .filter((btn) => btn.getAttribute('aria-label')?.includes('Clear')); const applyButtons = screen .getAllByRole('button') - .filter((btn) => btn.getAttribute('aria-label')?.includes('Apply highlight')); + .filter((btn) => btn.getAttribute('aria-label')?.includes('Apply')); // 3 remove (X for Y, B, G) + all 5 apply (yellow, green, blue, orange, pink) = 8 total expect(removeButtons).toHaveLength(3); diff --git a/packages/ui/src/components/verse-action-popover.tsx b/packages/ui/src/components/verse-action-popover.tsx index acf1b453..5a8264fc 100644 --- a/packages/ui/src/components/verse-action-popover.tsx +++ b/packages/ui/src/components/verse-action-popover.tsx @@ -135,13 +135,10 @@ export const VerseActionPopover: FC = ({ // Check if all 5 colors are already active highlights const allColorsActive = activeHighlights.size === HIGHLIGHT_COLORS.length; - // Determine which apply colors to show: - // Show all inactive colors ONLY if: - // - There are unhighlighted verses to apply colors to, AND - // - Not all colors are already active - // OR if there are multiple different active colors (but not all 5) + // Show all 5 apply colors when there are unhighlighted verses or multiple active colors + // (unless all 5 colors are already active) const showAllApplyColors = - (unHighlightedCount > 0 && !allColorsActive) || (activeHighlights.size > 1 && !allColorsActive); + !allColorsActive && (unHighlightedCount > 0 || activeHighlights.size > 1); // Choose apply colors based on the scenario const colorsToApply = showAllApplyColors diff --git a/packages/ui/src/components/verse.stories.tsx b/packages/ui/src/components/verse.stories.tsx index 22c001c1..ae565259 100644 --- a/packages/ui/src/components/verse.stories.tsx +++ b/packages/ui/src/components/verse.stories.tsx @@ -576,10 +576,30 @@ function VerseActionPopoverDemo(props: BibleTextViewProps) { onHighlight={handleHighlight} onClearHighlight={handleClearHighlight} onCopy={() => { - // Do nothing at this second + const verseText = selectedVerses + .map((v) => containerRef.current?.querySelector(`.yv-v[v="${v}"]`)?.textContent) + .filter(Boolean) + .join(' '); + const formatted = `"${verseText}" - John 1:${selectedVerses.join('-')} NIV`; + void navigator.clipboard.writeText(formatted); + setPopoverOpen(false); + setSelectedVerses([]); }} onShare={() => { - // Do nothing at this second + const verseText = selectedVerses + .map((v) => containerRef.current?.querySelector(`.yv-v[v="${v}"]`)?.textContent) + .filter(Boolean) + .join(' '); + const formatted = `"${verseText}" - John 1:${selectedVerses.join('-')} NIV`; + navigator + .share({ text: formatted }) + .then(() => { + setPopoverOpen(false); + setSelectedVerses([]); + }) + .catch(() => { + // User cancelled or share failed - keep popover open per AC4 + }); }} />
From 462ed34f4661f0e1ce6b80d558dea0f59e4b81d5 Mon Sep 17 00:00:00 2001 From: Cameron Pak Date: Mon, 2 Feb 2026 15:58:50 -0600 Subject: [PATCH 35/47] Fix: Improve highlight button accessibility label The previous regex for highlight button labels was too specific, matching exact color names. This commit broadens the match to any highlight action, improving accessibility for users who might not see the color options directly. --- packages/ui/src/components/verse-action-popover.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ui/src/components/verse-action-popover.test.tsx b/packages/ui/src/components/verse-action-popover.test.tsx index 790699ce..065e9208 100644 --- a/packages/ui/src/components/verse-action-popover.test.tsx +++ b/packages/ui/src/components/verse-action-popover.test.tsx @@ -341,7 +341,7 @@ describe('VerseActionPopover', () => { const colorButtons = container.querySelectorAll('[aria-label*="highlight"]'); colorButtons.forEach((btn) => { const label = btn.getAttribute('aria-label'); - expect(label).toMatch(/^(Apply|Clear) (yellow|green|blue|orange|pink) highlight$/); + expect(label).toMatch(/^(Apply|Clear) highlight$/); }); // Check action buttons have accessible text (they use icon + text) From 022a7f66352ffb6b12fb587799534f9feb3ab36d Mon Sep 17 00:00:00 2001 From: Cameron Pak Date: Mon, 2 Feb 2026 15:59:04 -0600 Subject: [PATCH 36/47] Fix: Update highlight button label in tests Changes the text of the "Clear yellow highlight" button in tests to "Clear highlight" to be more generic. --- packages/ui/src/components/verse-action-popover.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ui/src/components/verse-action-popover.test.tsx b/packages/ui/src/components/verse-action-popover.test.tsx index 065e9208..baa62550 100644 --- a/packages/ui/src/components/verse-action-popover.test.tsx +++ b/packages/ui/src/components/verse-action-popover.test.tsx @@ -237,7 +237,7 @@ describe('VerseActionPopover', () => { />, ); - const removeButton = screen.getByRole('button', { name: /Clear yellow highlight/ }); + const removeButton = screen.getByRole('button', { name: /Clear highlight/ }); fireEvent.click(removeButton); expect(onClearHighlight).toHaveBeenCalledWith(HIGHLIGHT_COLORS[0]); }); From 98ca16be5c8e2d1c8f5218dc463c7ad86d0264a5 Mon Sep 17 00:00:00 2001 From: Cameron Pak Date: Mon, 2 Feb 2026 16:01:22 -0600 Subject: [PATCH 37/47] style(ui): remove unnecessary comments --- .../src/components/verse-action-popover.tsx | 19 ------------------- packages/ui/src/components/verse.tsx | 1 - 2 files changed, 20 deletions(-) diff --git a/packages/ui/src/components/verse-action-popover.tsx b/packages/ui/src/components/verse-action-popover.tsx index 5a8264fc..f60903d2 100644 --- a/packages/ui/src/components/verse-action-popover.tsx +++ b/packages/ui/src/components/verse-action-popover.tsx @@ -86,7 +86,6 @@ export const VerseActionPopover: FC = ({ }) => { const popoverRef = useRef(null); - // Show/hide popover based on open state useEffect(() => { const el = popoverRef.current; if (!el) return; @@ -98,7 +97,6 @@ export const VerseActionPopover: FC = ({ } }, [open]); - // Listen for toggle events to sync state useEffect(() => { const el = popoverRef.current; if (!el) return; @@ -113,7 +111,6 @@ export const VerseActionPopover: FC = ({ return () => el.removeEventListener('toggle', handleToggle); }, [onOpenChange]); - // Focus first button when popover opens useEffect(() => { if (open && popoverRef.current) { const firstButton = popoverRef.current.querySelector('button'); @@ -121,26 +118,12 @@ export const VerseActionPopover: FC = ({ } }, [open]); - // Build color circles: - // 1. Active highlights with X (to clear) - always shown first/left - // 2. Decide which apply colors to show: - // - If all selected verses have the SAME single color: hide that color's apply button - // - Otherwise (multiple colors OR unhighlighted verses): show all 5 apply colors const activeColors = HIGHLIGHT_COLORS.filter((c) => activeHighlights.has(c)); - - // Count unhighlighted verses const highlightedVerseCount = selectedVerses.filter((v) => highlightedVerses[v]).length; const unHighlightedCount = selectedVerses.length - highlightedVerseCount; - - // Check if all 5 colors are already active highlights const allColorsActive = activeHighlights.size === HIGHLIGHT_COLORS.length; - - // Show all 5 apply colors when there are unhighlighted verses or multiple active colors - // (unless all 5 colors are already active) const showAllApplyColors = !allColorsActive && (unHighlightedCount > 0 || activeHighlights.size > 1); - - // Choose apply colors based on the scenario const colorsToApply = showAllApplyColors ? HIGHLIGHT_COLORS : HIGHLIGHT_COLORS.filter((c) => !activeHighlights.has(c)); @@ -175,7 +158,6 @@ export const VerseActionPopover: FC = ({ transform: 'translateX(-50%)', }} > - {/* Highlight color circles */}
{colorCircles.map(({ color, showX, key }) => ( = ({ {/* Separator */}