diff --git a/.changeset/checkmark-active-swatch.md b/.changeset/checkmark-active-swatch.md new file mode 100644 index 00000000..5df3f80e --- /dev/null +++ b/.changeset/checkmark-active-swatch.md @@ -0,0 +1,7 @@ +--- +'@youversion/platform-react-ui': patch +--- + +BibleReader's verse action popover now marks active swatches with a checkmark (YPE-1034 PR3, still behind the internal `HIGHLIGHTS_LIVE` flag). + +- **Checkmark swap**: active/remove swatches now render a 24px checkmark (`icons/check`) instead of the X, matching iOS (platform-sdk-swift #179). Same theme-invariant `#121212` fill and identical behavior — tapping still removes the highlight. diff --git a/docs/adr/YPE-642-verse-action-popover.md b/docs/adr/YPE-642-verse-action-popover.md index 97a2376b..eb522547 100644 --- a/docs/adr/YPE-642-verse-action-popover.md +++ b/docs/adr/YPE-642-verse-action-popover.md @@ -146,6 +146,11 @@ Selection is always enabled in BibleReader (no opt-out prop for now; YAGNI). - Tunable; swap to a ring/bg later if Figma says otherwise. ## As-built notes (deviations from the design above) +- **ADR-005 active-swatch icon (YPE-1034 PR3):** the 24px **X** on active/remove + swatches was replaced with a 24px **checkmark** (`icons/check`), matching iOS + (platform-sdk-swift #179). Same Text/Everdark (`--yv-gray-50` = `#121212`, + theme-invariant) fill and identical behavior — tapping still removes the + highlight; the swatch's `Clear highlight` aria-label is unchanged. - **ADR-004 revised:** selection + highlights live in `Content`, **not** Root context. Copy/Share/anchor all need the rendered verse DOM (which lives in Content), so Root ownership would fragment the feature. BibleTextView stays diff --git a/packages/ui/src/components/icons/check.tsx b/packages/ui/src/components/icons/check.tsx new file mode 100644 index 00000000..9f9faaa1 --- /dev/null +++ b/packages/ui/src/components/icons/check.tsx @@ -0,0 +1,20 @@ +import type { ComponentProps, ReactElement } from 'react'; + +export function CheckIcon(props: ComponentProps<'svg'>): ReactElement { + return ( + + + + ); +} diff --git a/packages/ui/src/components/verse-action-popover.test.tsx b/packages/ui/src/components/verse-action-popover.test.tsx index db04c396..672f8673 100644 --- a/packages/ui/src/components/verse-action-popover.test.tsx +++ b/packages/ui/src/components/verse-action-popover.test.tsx @@ -480,6 +480,34 @@ describe('VerseActionPopover', () => { .filter((btn) => btn.getAttribute('aria-label')?.includes('Clear')); } + describe('Active swatch checkmark', () => { + // The checkmark path (Swift #179) starts at these coords — asserts we render + // the check, not the old X. + const CHECK_PATH_PREFIX = 'M19.6627'; + + it('renders a checkmark (not an X) on the active/remove swatch', () => { + render( + ([HIGHLIGHT_COLORS[0]])} + selectedVerses={[1]} + highlightedVerses={{ 1: HIGHLIGHT_COLORS[0] }} + />, + ); + + const removeButton = screen.getByRole('button', { name: /Clear highlight/ }); + const path = removeButton.querySelector('svg path'); + expect(path?.getAttribute('d')).toContain(CHECK_PATH_PREFIX); + }); + + it('apply swatches render no icon', () => { + render(); + applyButtons().forEach((btn) => { + expect(btn.querySelector('svg')).toBeNull(); + }); + }); + }); + describe('Highlights disabled (flag off)', () => { it('hides the color row and remove circles but keeps Copy / Share', () => { render( diff --git a/packages/ui/src/components/verse-action-popover.tsx b/packages/ui/src/components/verse-action-popover.tsx index e8a30764..ac91bf1f 100644 --- a/packages/ui/src/components/verse-action-popover.tsx +++ b/packages/ui/src/components/verse-action-popover.tsx @@ -5,7 +5,7 @@ import i18n from '@/i18n'; import { cn } from '../lib/utils'; import { BoxStackIcon } from './icons/box-stack'; import { BoxArrowUpIcon } from './icons/box-arrow-up'; -import { XIcon } from './icons/x'; +import { CheckIcon } from './icons/check'; type Measurable = { getBoundingClientRect: () => DOMRect }; @@ -47,12 +47,12 @@ type VerseActionPopoverProps = { type ColorCircleProps = { color: string; - showX: boolean; + showRemove: boolean; label: string; onClick: () => void; }; -function ColorCircle({ color, showX, label, onClick }: ColorCircleProps) { +function ColorCircle({ color, showRemove, label, onClick }: ColorCircleProps) { return ( ); } @@ -201,10 +203,10 @@ export const VerseActionPopover: FC = ({ ? HIGHLIGHT_COLORS : HIGHLIGHT_COLORS.filter((c) => !activeHighlights.has(c)); - // X (remove) circles come first, then apply circles in canonical order. + // Remove (checkmark) circles come first, then apply circles in canonical order. const colorCircles = [ - ...activeColors.map((color) => ({ color, showX: true, key: `${color}-clear` })), - ...colorsToApply.map((color) => ({ color, showX: false, key: `${color}-apply` })), + ...activeColors.map((color) => ({ color, showRemove: true, key: `${color}-clear` })), + ...colorsToApply.map((color) => ({ color, showRemove: false, key: `${color}-apply` })), ]; // Snapshot of everything the Content renders. While open we keep it fresh; the @@ -291,13 +293,13 @@ export const VerseActionPopover: FC = ({ role="group" aria-label={t('highlightColorsAriaLabel')} > - {view.colorCircles.map(({ color, showX, key }) => ( + {view.colorCircles.map(({ color, showRemove, key }) => ( (showX ? onClearHighlight(color) : onHighlight(color))} + showRemove={showRemove} + label={showRemove ? t('clearHighlightAriaLabel') : t('applyHighlightAriaLabel')} + onClick={() => (showRemove ? onClearHighlight(color) : onHighlight(color))} /> ))}