feat(ui): recent highlight colors + checkmark active swatch (YPE-1034, PR 3/3)#286
Draft
cameronapak wants to merge 2 commits into
Draft
Conversation
…watch Render the server's recent-colors list in BibleReader's verse action popover, and swap the active/remove swatch's X for a checkmark to match iOS (platform-sdk-swift #179). Still behind the internal HIGHLIGHTS_LIVE flag (YPE-1034 PR3). - useBibleReaderHighlights fetches getRecentColors once the feature is live (flag on + auth provider + signed in) and exposes recentColors; it resets to null on sign-out / flag-off and on fetch failure so the row falls back to the default palette. - VerseActionPopover takes an optional recentColors prop: normalized (lowercase, strip leading #, drop non-6-hex), deduped first-occurrence-wins, never reordered (server order is truth), and horizontally scrollable on overflow. Falls back to HIGHLIGHT_COLORS when recents are absent/empty. Active colors outside the palette still get a remove circle. - HighlightColor widened from the 5-literal union to string (loosening, non-breaking); HIGHLIGHT_COLORS still exported as the default palette. - New icons/check; ADR-005 as-built notes updated. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review follow-ups on the recent-colors popover change: - The row's new `overflow-x: auto` forces overflow-y out of `visible` (CSS spec), clipping the swatches' focus-visible ring (ring-2 + offset-2, ~4px overpaint) and hover scale-110 — including for prod users with HIGHLIGHTS_LIVE off, since the popover itself is not flag-gated. Add 6px padding with compensating negative margin so the overpaint stays inside the scroll box without changing layout, and a regression test guarding the padding/overflow pairing. - Rework the primary recent-colors integration test to the production auth timing: mount signed out (as YouVersionAuthProvider hydrates async), assert no fetch, then flip signed-in and assert the fetch fires — the sync signed-in mount shape both prior PRs were dinged for. - Document the deferred account-switch-without-sign-out staleness at the fetch effect (recents keyed on `live` only, not user identity). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: eef219b The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
PR 3 of 3 in the YPE-1034 stack (stacked on #285 → #283 — merge in order; retarget as layers land). Final layer: the popover color row shows the user's recent highlight colors, and active swatches show a checkmark instead of an X.
Recent colors
GET /v1/highlights/recent-colors(server-ordered: recents first, then defaults) via the existingHighlightsClient.getRecentColors/useHighlightssurface — no new network code#, drop non-6-hex), dedupe first-occurrence, never reorder, no cap; row scrolls horizontallyHighlightColorwidened from the 5-literal union tostring(internal type; loosening only). Active colors not in the palette now correctly get a remove circleCheckmark swap
Review notes
Adversarially reviewed. Fixed before opening: the scrollable row's
overflow-x: autoclipped the swatches' focus-visible ring (would have shipped to prod — the popover isn't flag-gated); fixed with padding + negative-margin bleed room and a regression test. Recent-colors integration test reworked to real async auth-provider timing. Deferred with code comment: account switch without sign-out keeps prior user's recents (follow-up: key fetch on user identity).Verification
854 tests green (core 334, hooks 285, ui 235), typecheck + lint clean. Changeset included (patch, unified versioning).
Related Issues
Relates to YPE-1034
Stacked on #285
🤖 Generated with Claude Code