feat(android): render branded block icons with contrast-aware tinting#468
Draft
jkmassel wants to merge 3 commits intojkmassel/android-block-pickerfrom
Draft
feat(android): render branded block icons with contrast-aware tinting#468jkmassel wants to merge 3 commits intojkmassel/android-block-pickerfrom
jkmassel wants to merge 3 commits intojkmassel/android-block-pickerfrom
Conversation
4 tasks
88d07a8 to
932e7ed
Compare
Adopts AndroidSVG (com.caverock:androidsvg-aar:1.4) — the same rendering engine Coil-SVG wraps, used directly to avoid pulling in Coil — and wires it into the block inserter dialog introduced in #461. Mirrors `BlockIconCache` on iOS: parse each block's inline SVG once, keyed by `BlockType.id`, and cache the rendered bitmap so RecyclerView rebinds don't re-render on scroll. Three @wordpress/icons patterns the browser handles via CSS that AndroidSVG does not: 1. **Missing `viewBox`** (e.g. `core/site-tagline`) — intrinsic width/height are declared but paths render at native coordinate size inside our larger viewport, so the icon appears tiny. Synthesise a viewBox from the intrinsic dimensions and set document width/height to 100%. 2. **`fill="none"` at root** (e.g. `core/icon`) — paths without an explicit fill inherit `none` and render invisibly. The web editor's `@wordpress/components` CSS injects `fill: currentColor`; we do the same via `RenderOptions.css` at render time. 3. **Multi-fill branded icons** (e.g. Pocket Casts, Animoto) — these rely on colour contrast between inner/outer paths. A monochrome PorterDuff SRC_IN tint flattens them into a silhouette. Detect hex fills in the raw SVG string and skip the tint when present, letting branded icons render as-is. Pocket Casts still renders black-and-white rather than brand red — its foreground colour lives in JS metadata (`icon.foreground`) that `getBlockIcon` at `src/utils/blocks.js:44` drops. Fixing this properly requires piping `foreground` through the bridge and is deferred to a follow-up. - [x] `./gradlew :Gutenberg:test detekt :Gutenberg:lintDebug :app:compileDebugKotlin` — BUILD SUCCESSFUL - [x] Manually verified on Pixel 9 Pro XL with `enableNativeBlockInserter` on: open inserter, scroll through all sections, confirmed Site Tagline (no viewBox), Icon block (root `fill="none"`), and Pocket Casts/Animoto/Bluesky (branded colours) all render correctly. - [ ] Reviewer: verify iOS behaviour unchanged.
Follow-up to #461 / 444c640 that addresses the "known limitation" called out in that commit: branded icons (Pocket Casts, Animoto, Bluesky) and single-colour brand foregrounds (X, Dailymotion, WordPress Embed) now render correctly against either light or dark dialog surfaces. ## Bridge `getBlockIcon` at `src/utils/blocks.js:44` previously dropped `icon.foreground` — the JS metadata that the web editor applies as CSS `color`, which paths inside the SVG pick up via `currentColor`. Adds `getBlockIconForeground` and includes it in the serialised inserter payload, with matching `iconForeground: String?` fields on the Android `BlockType` data class and the iOS `BlockType` struct (iOS gets the field for parity; no rendering change). ## Chip background Wraps each icon in a 44dp `RoundedRectangle` chip filled with ~12% of the theme's primary text colour, mirroring the iOS `BlockIconView` treatment (`Color(.label).mask` over a `secondarySystemFill` chip). Without this, low-contrast brand icons (X's `#000000`, Dailymotion's `#333436`) rendered as near-invisible smudges on the dark bottom sheet. ## Tinting decision `SvgIconCache.shouldTint` decides per-icon whether to apply the theme tint: 1. **No declared colours** — pure monochrome; tint to text colour. 2. **Multiple declared colours** (Pocket Casts red+white, Animoto, Bluesky) — render as-is. A PorterDuff SRC_IN tint would flatten internal contrast into a silhouette. 3. **Single declared colour** — keep it if it has at least 3:1 contrast (WCAG 2.x SC 1.4.11 — minimum for UI graphics) against the surface the icon actually sits on; otherwise strip the brand colour and tint. The contrast reference is the chip fill **composited over the dialog surface**, not the bare surface. Measuring against bare black makes marginal colours like WordPress blue (#0073AA) appear to pass at 3.16:1 while reading as dim against the actual ~`#3B3B3D` chip surrogate (2.1:1). `resolveDialogSurface()` looks up `?attr/colorSurface` then `?android:attr/colorBackground` so the surrogate matches what the user sees. ## Test plan - [x] `./gradlew :Gutenberg:test detekt :Gutenberg:lintDebug :app:compileDebugKotlin` — BUILD SUCCESSFUL - [x] Manually verified on Pixel 9 Pro XL with `enableNativeBlockInserter` on, dark theme: Pocket Casts renders red+white, Animoto/Bluesky render branded, X / Dailymotion / WordPress Embed render white (tint fallback), Site Tagline / Icon block still render correctly. - [ ] Reviewer: verify in light theme; verify iOS behaviour unchanged.
buildBlockView was 62 lines — 2 over the 60-line threshold. The chip FrameLayout is a self-contained piece, so pulling it into a helper is the natural breakpoint rather than suppressing the rule.
ae080ac to
f19065a
Compare
4 tasks
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
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.
Stacks on top of #461 (native block inserter). Renders block icons via AndroidSVG, with contrast-aware tinting so branded icons read correctly against either a light or dark dialog surface.
This PR is two related slices that land together — splitting them would just be churn:
com.caverock:androidsvg-aar:1.4) and mirrorsBlockIconCacheon iOS. Handles three @wordpress/icons quirks the browser papers over with CSS: missingviewBox(e.g.core/site-tagline), rootfill="none"(e.g.core/icon), and multi-fill branded icons that would flatten under a PorterDuff tint.icon.foregroundthrough the JS bridge so single-colour brand icons (Pocket Casts red) render correctly, and adds a 3:1 WCAG contrast check so low-contrast brands (X#000000, Dailymotion#333436, WordPress Embed#0073AA) fall back to the theme tint instead of disappearing into the surface.Screenshots
Pixel 9 Pro XL. The embed section is the interesting one — it exercises every branch of the tinting decision (monochrome, multi-colour branded, single-colour branded with good/bad contrast).
Embeds
In dark mode, WordPress (
#0073AA), X (#000000), and Dailymotion (#333436) fall back to the theme tint — all three fail the 3:1 contrast check against the chip surrogate. In light mode the same colours pass and render as-is. Pocket Casts / Reddit / Pinterest / Spotify / YouTube / Vimeo keep their brand colours in both modes (multi-colour or sufficient contrast).Most used
Bridge
getBlockIconatsrc/utils/blocks.js:44previously droppedicon.foreground— the JS metadata that the web editor applies as CSScolor, which paths inside the SVG pick up viacurrentColor. AddsgetBlockIconForegroundand includes it in the serialised inserter payload, with matchingiconForeground: String?fields on the AndroidBlockTypedata class and the iOSBlockTypestruct (iOS gets the field for parity; no rendering change).Chip background
Wraps each icon in a 44dp
RoundedRectanglechip filled with ~12% of the theme's primary text colour, mirroring the iOSBlockIconViewtreatment (Color(.label).maskover asecondarySystemFillchip). Without this, low-contrast brand icons rendered as near-invisible smudges on the dark bottom sheet.Tinting decision
SvgIconCache.shouldTintdecides per-icon whether to apply the theme tint:SRC_INtint would flatten internal contrast into a silhouette.The contrast reference is the chip fill composited over the dialog surface, not the bare surface. Measuring against bare black makes marginal colours like WordPress blue (
#0073AA) appear to pass at 3.16:1 while reading as dim against the actual~#3B3B3Dchip surrogate (2.1:1).resolveDialogSurface()looks up?attr/colorSurfacethen?android:attr/colorBackgroundso the surrogate matches what the user sees.Test plan
./gradlew :Gutenberg:test detekt :Gutenberg:lintDebug :app:compileDebugKotlin— BUILD SUCCESSFULenableNativeBlockInserteron, dark theme: Pocket Casts renders red+white, Animoto/Bluesky render branded, X / Dailymotion / WordPress Embed render white (tint fallback), Site Tagline / Icon block still render correctly.Related