feat(android): render icons and wrap labels in the block inserter#481
Merged
jkmassel merged 3 commits intoApr 30, 2026
Merged
Conversation
4caa2d2 to
84b00c3
Compare
Replaces the tonal placeholder Box in `BlockPickerDialog.BlockTile` with the actual SVG icon for each block, sourced from `SvgIconCache` (added in #461 alongside this dialog but never wired up). Without this change the inserter renders rows of identical solid chips — every block is visually indistinguishable apart from its label. The cache lives at the `BlockGridContent` level and is keyed on `(renderSizePx, chipColor)` so a font-scale or theme change recomputes both the bitmap resolution and the contrast surrogate the tinting decision in `SvgIconCache.shouldTint` is measured against. The render size is the inner glyph (`BLOCK_TILE_ICON_GLYPH_SIZE_DP`, 32dp), not the 56dp chip — the bitmap pixels need to be sharp at the displayed size, not at the chip dimensions. Branded icons (Pocket Casts, Pinterest, Reddit) flow through with no tint; monochrome and low-contrast brand icons get `ColorFilter.tint(onSurface)` so they read against the chip. Disabled blocks dim via `Image.alpha` rather than baking the alpha into the tint, so branded icons fade uniformly instead of going monochrome on disable.
…erter `AutoShrinkTileLabel` previously shrank labels down to 9sp until they fit on a single line. Three issues with the result: 1. **Single-word and multi-word were treated identically.** "Featured Image" was shrunk to fit on one line when it could have wrapped at the space at full size and stayed legible. 2. **Mid-word breaks on long single words.** Naively switching to `maxLines = 2` made Compose character-break "Preformatted" into "Preform" / "atted" rather than shrink it. The default soft-wrap will break inside a word to satisfy the line budget, so we now measure single-word labels with `maxLines = 1` and render with `softWrap = false` to force the shrink path. 3. **Loose vertical rhythm on wrapped labels.** With no explicit `lineHeight`, the gap between the two wrapped lines tracked the font's default leading and read as too tall. `Material 3`'s typography presets fix this with `LineHeightStyle.Trim.Both`, but custom `sp + lineHeight` doesn't pick that up — set it explicitly so labels sit centred against the reserved two-line height. `minLines = 2` reserves two lines worth of vertical space at the resolved size so tile heights stay consistent across the grid, even when neighbouring labels need different line counts.
84b00c3 to
fd4b364
Compare
6 tasks
|
A quick bug/fix: |
adalpari
approved these changes
Apr 30, 2026
adalpari
left a comment
There was a problem hiding this comment.
Left a comment, about a possible bug. But other that that LGTM!
`rememberSvgIconCache` correctly returns a new `SvgIconCache` instance when `sizePx` or `surfaceArgb` changes — i.e. on a theme or font-scale switch. `BlockTile` was keying its `remember` only on `block.id`, so it returned the previously-rendered `Icon` from the now-stale cache. Visible result: icons keep using the old contrast surrogate (and the old bitmap pixels) until the tile leaves and re-enters composition. Adding `iconCache` to the key forces the lambda to re-run whenever the cache reference changes, which re-renders each SVG against the new surface and rebuilds the tinting decision. The new bitmap is the correct one to display going forward. Caught by @adalpari in #481 review.
Contributor
Author
Nice thanks! |
2 tasks
jkmassel
added a commit
that referenced
this pull request
May 11, 2026
* feat(android): render block icons in the native inserter Replaces the tonal placeholder Box in `BlockPickerDialog.BlockTile` with the actual SVG icon for each block, sourced from `SvgIconCache` (added in #461 alongside this dialog but never wired up). Without this change the inserter renders rows of identical solid chips — every block is visually indistinguishable apart from its label. The cache lives at the `BlockGridContent` level and is keyed on `(renderSizePx, chipColor)` so a font-scale or theme change recomputes both the bitmap resolution and the contrast surrogate the tinting decision in `SvgIconCache.shouldTint` is measured against. The render size is the inner glyph (`BLOCK_TILE_ICON_GLYPH_SIZE_DP`, 32dp), not the 56dp chip — the bitmap pixels need to be sharp at the displayed size, not at the chip dimensions. Branded icons (Pocket Casts, Pinterest, Reddit) flow through with no tint; monochrome and low-contrast brand icons get `ColorFilter.tint(onSurface)` so they read against the chip. Disabled blocks dim via `Image.alpha` rather than baking the alpha into the tint, so branded icons fade uniformly instead of going monochrome on disable. * feat(android): wrap two-line labels with tight line height in the inserter `AutoShrinkTileLabel` previously shrank labels down to 9sp until they fit on a single line. Three issues with the result: 1. **Single-word and multi-word were treated identically.** "Featured Image" was shrunk to fit on one line when it could have wrapped at the space at full size and stayed legible. 2. **Mid-word breaks on long single words.** Naively switching to `maxLines = 2` made Compose character-break "Preformatted" into "Preform" / "atted" rather than shrink it. The default soft-wrap will break inside a word to satisfy the line budget, so we now measure single-word labels with `maxLines = 1` and render with `softWrap = false` to force the shrink path. 3. **Loose vertical rhythm on wrapped labels.** With no explicit `lineHeight`, the gap between the two wrapped lines tracked the font's default leading and read as too tall. `Material 3`'s typography presets fix this with `LineHeightStyle.Trim.Both`, but custom `sp + lineHeight` doesn't pick that up — set it explicitly so labels sit centred against the reserved two-line height. `minLines = 2` reserves two lines worth of vertical space at the resolved size so tile heights stay consistent across the grid, even when neighbouring labels need different line counts. * fix(android): re-render block icons when the SVG cache is replaced `rememberSvgIconCache` correctly returns a new `SvgIconCache` instance when `sizePx` or `surfaceArgb` changes — i.e. on a theme or font-scale switch. `BlockTile` was keying its `remember` only on `block.id`, so it returned the previously-rendered `Icon` from the now-stale cache. Visible result: icons keep using the old contrast surrogate (and the old bitmap pixels) until the tile leaves and re-enters composition. Adding `iconCache` to the key forces the lambda to re-run whenever the cache reference changes, which re-renders each SVG against the new surface and rebuilds the tinting decision. The new bitmap is the correct one to display going forward. Caught by @adalpari in #481 review.
jkmassel
added a commit
that referenced
this pull request
May 13, 2026
* feat(android): render block icons in the native inserter Replaces the tonal placeholder Box in `BlockPickerDialog.BlockTile` with the actual SVG icon for each block, sourced from `SvgIconCache` (added in #461 alongside this dialog but never wired up). Without this change the inserter renders rows of identical solid chips — every block is visually indistinguishable apart from its label. The cache lives at the `BlockGridContent` level and is keyed on `(renderSizePx, chipColor)` so a font-scale or theme change recomputes both the bitmap resolution and the contrast surrogate the tinting decision in `SvgIconCache.shouldTint` is measured against. The render size is the inner glyph (`BLOCK_TILE_ICON_GLYPH_SIZE_DP`, 32dp), not the 56dp chip — the bitmap pixels need to be sharp at the displayed size, not at the chip dimensions. Branded icons (Pocket Casts, Pinterest, Reddit) flow through with no tint; monochrome and low-contrast brand icons get `ColorFilter.tint(onSurface)` so they read against the chip. Disabled blocks dim via `Image.alpha` rather than baking the alpha into the tint, so branded icons fade uniformly instead of going monochrome on disable. * feat(android): wrap two-line labels with tight line height in the inserter `AutoShrinkTileLabel` previously shrank labels down to 9sp until they fit on a single line. Three issues with the result: 1. **Single-word and multi-word were treated identically.** "Featured Image" was shrunk to fit on one line when it could have wrapped at the space at full size and stayed legible. 2. **Mid-word breaks on long single words.** Naively switching to `maxLines = 2` made Compose character-break "Preformatted" into "Preform" / "atted" rather than shrink it. The default soft-wrap will break inside a word to satisfy the line budget, so we now measure single-word labels with `maxLines = 1` and render with `softWrap = false` to force the shrink path. 3. **Loose vertical rhythm on wrapped labels.** With no explicit `lineHeight`, the gap between the two wrapped lines tracked the font's default leading and read as too tall. `Material 3`'s typography presets fix this with `LineHeightStyle.Trim.Both`, but custom `sp + lineHeight` doesn't pick that up — set it explicitly so labels sit centred against the reserved two-line height. `minLines = 2` reserves two lines worth of vertical space at the resolved size so tile heights stay consistent across the grid, even when neighbouring labels need different line counts. * fix(android): re-render block icons when the SVG cache is replaced `rememberSvgIconCache` correctly returns a new `SvgIconCache` instance when `sizePx` or `surfaceArgb` changes — i.e. on a theme or font-scale switch. `BlockTile` was keying its `remember` only on `block.id`, so it returned the previously-rendered `Icon` from the now-stale cache. Visible result: icons keep using the old contrast surrogate (and the old bitmap pixels) until the tile leaves and re-enters composition. Adding `iconCache` to the key forces the lambda to re-run whenever the cache reference changes, which re-renders each SVG against the new surface and rebuilds the tinting decision. The new bitmap is the correct one to display going forward. Caught by @adalpari in #481 review.
jkmassel
added a commit
that referenced
this pull request
May 13, 2026
* feat(android): wire up showBlockInserter bridge Adds the `@JavascriptInterface showBlockInserter(String)` entry point on `GutenbergView` plus the `presentBlockInserter` / `insertBlock` / `dismissBlockInserter` round-trip back to JS via `window.blockInserter.insertBlock` / `onClose`. Bad payloads surface a Toast (from `R.string.gbk_block_inserter_failure`) so a broken tap isn't silent. Sets `resourcePrefix = "gbk_"` so AGP flags any unprefixed library resource at lint time. The dialog itself lands in #480; this commit only wires the bridge so that PR can build atop it. * feat(android): add native block inserter (#480) Compose-based bottom sheet that replaces the legacy WebView block picker. Variation B handoff: drag handle + header, tonal Material 3 palette (dynamic on API 31+, brand-seeded fallback below), 5-column tile grid with auto-shrinking labels, scrollable category-tab chips, and a rounded search field. Block tiles render plain tonal rounded-rect placeholders for now — SVG icon rendering lands in #468 (which adds `SvgIconCache` and pipes `iconForeground` through the JS payload + iOS/Android models). This PR deliberately stops at the shell so #468 can be reviewed independently. Tab filter, search filter, photo/camera tiles, and recent-photo strip ship in #478 / #479 — the chips and search bar are intentionally non-functional in this PR so the visual shell can be reviewed in isolation. * feat(android): render branded block icons with contrast-aware tinting (#468) * feat(android): render SVG icons in block inserter 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. * feat(android): contrast-aware tinting for branded block icons 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. `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). 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. `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. - [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. * refactor(android): address review feedback in BlockInserterDialog Three fixes from #468 review: - **resolveTextColorPrimary fallback** — `resolveAttribute` can return false (attribute absent) or leave `typed.resourceId == 0`, in which case `getColorStateList(0, …)` throws. Mirror the pattern already used by `resolveDialogSurface` and fall back to `Color.BLACK`. - **BlockViewHolder positional access** — `(container.getChildAt(0) as FrameLayout).getChildAt(0) as ImageView` silently breaks if the view hierarchy is reordered. `buildBlockView` now returns a `BlockRowViews` struct holding direct references; the ViewHolder takes that struct rather than the root `View`. - **rightMargin → marginEnd** — `rightMargin` is physical, `marginEnd` is layout-direction-aware so the chip leads correctly under RTL. Verified manually on Pixel 9 Pro XL: most-used and embed sections still render correctly with chip-backed icons. * refactor(android): drop over-defensive fallback in resolveTextColorPrimary The Color.BLACK fallback only triggered when theme.resolveAttribute returned false — i.e. the host app's theme genuinely doesn't define android.R.attr.textColorPrimary, which has been part of the framework since API 1. Standard theme parents (AppCompat, Material*, even bare @android:style/Theme) all define it. Bailing out a host that's already broken in many other ways isn't our job. Keep the typed.resourceId != 0 branch — that handles legitimate inline literal themes (`<item name="...">#DD000000</item>`), which is real real-world theme construction, not misconfiguration. * feat(android): render icons and wrap labels in the block inserter (#481) * feat(android): render block icons in the native inserter Replaces the tonal placeholder Box in `BlockPickerDialog.BlockTile` with the actual SVG icon for each block, sourced from `SvgIconCache` (added in #461 alongside this dialog but never wired up). Without this change the inserter renders rows of identical solid chips — every block is visually indistinguishable apart from its label. The cache lives at the `BlockGridContent` level and is keyed on `(renderSizePx, chipColor)` so a font-scale or theme change recomputes both the bitmap resolution and the contrast surrogate the tinting decision in `SvgIconCache.shouldTint` is measured against. The render size is the inner glyph (`BLOCK_TILE_ICON_GLYPH_SIZE_DP`, 32dp), not the 56dp chip — the bitmap pixels need to be sharp at the displayed size, not at the chip dimensions. Branded icons (Pocket Casts, Pinterest, Reddit) flow through with no tint; monochrome and low-contrast brand icons get `ColorFilter.tint(onSurface)` so they read against the chip. Disabled blocks dim via `Image.alpha` rather than baking the alpha into the tint, so branded icons fade uniformly instead of going monochrome on disable. * feat(android): wrap two-line labels with tight line height in the inserter `AutoShrinkTileLabel` previously shrank labels down to 9sp until they fit on a single line. Three issues with the result: 1. **Single-word and multi-word were treated identically.** "Featured Image" was shrunk to fit on one line when it could have wrapped at the space at full size and stayed legible. 2. **Mid-word breaks on long single words.** Naively switching to `maxLines = 2` made Compose character-break "Preformatted" into "Preform" / "atted" rather than shrink it. The default soft-wrap will break inside a word to satisfy the line budget, so we now measure single-word labels with `maxLines = 1` and render with `softWrap = false` to force the shrink path. 3. **Loose vertical rhythm on wrapped labels.** With no explicit `lineHeight`, the gap between the two wrapped lines tracked the font's default leading and read as too tall. `Material 3`'s typography presets fix this with `LineHeightStyle.Trim.Both`, but custom `sp + lineHeight` doesn't pick that up — set it explicitly so labels sit centred against the reserved two-line height. `minLines = 2` reserves two lines worth of vertical space at the resolved size so tile heights stay consistent across the grid, even when neighbouring labels need different line counts. * fix(android): re-render block icons when the SVG cache is replaced `rememberSvgIconCache` correctly returns a new `SvgIconCache` instance when `sizePx` or `surfaceArgb` changes — i.e. on a theme or font-scale switch. `BlockTile` was keying its `remember` only on `block.id`, so it returned the previously-rendered `Icon` from the now-stale cache. Visible result: icons keep using the old contrast surrogate (and the old bitmap pixels) until the tile leaves and re-enters composition. Adding `iconCache` to the key forces the lambda to re-run whenever the cache reference changes, which re-renders each SVG against the new surface and rebuilds the tinting decision. The new bitmap is the correct one to display going forward. Caught by @adalpari in #481 review. * feat(android): wire up block inserter search and tab filtering (#478) Activates the chips and search field shipped as a non-functional shell in the previous PR. Tab selection narrows the grid to a fixed set of block ids per category (Text/Media/Design/Widgets/Theme/Embeds), and the Recent tab surfaces blocks from the payload's `gbk-most-used` section when present, falling back to all blocks otherwise. Search runs against the active tab's results with a 150ms debounce and a fuzzy scorer that weights name/title/keyword/category matches. Empty result sets render the No results / "No blocks match X" copy. * fix(js): skip redux toggle when opening native block inserter (#487) Tapping the `+` button currently calls `setIsInserterOpened(true)` in the editor store, which momentarily renders Gutenberg's web inserter before the native dialog covers it — visible as a flash on slower devices. Call `prepareAndShowInserter()` directly instead; the redux flag isn't useful when the inserter is rendered natively.
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.
Two changes layered on top of #461 — wire up the
SvgIconCachethat landed there but was never called from the dialog, and tighten the tile label layout so multi-word entries wrap at whitespace while single-word entries shrink instead of mid-word breaking.Tab filtering / search / empty state are out of scope — #478 covers them.
Screenshots
Pixel 9 Pro XL, Recent tab. Single-word labels stay on one line at 12sp; multi-word labels wrap at whitespace; tile heights stay aligned across rows.
Render block icons
Replaces the tonal placeholder
BoxinBlockPickerDialog.BlockTilewith the real block icon, sourced fromSvgIconCache(added alongside the dialog in #461 but never referenced from it). Without this the inserter ships as rows of identical solid chips: every block looks the same apart from its label.BLOCK_TILE_ICON_GLYPH_SIZE_DP = 32— inner glyph size, distinct from the 56dp chip. The cache renders at glyph pixels, not chip pixels, so bitmaps stay sharp at the displayed size.rememberSvgIconCache(chipColor)instantiates the cache once perBlockGridContentcomposition, keyed on(renderSizePx, chipColor). A font-scale or theme change recomputes both the bitmap resolution and the contrast surrogate thatSvgIconCache.shouldTintmeasures the brand colour against.tintable == true(monochrome / low-contrast brand) →ColorFilter.tint(onSurface).tintable == false(multi-colour / sufficient-contrast brand) → render as-is.block.isDisabled→ applied viaImage.alpha, not folded into the tint, so branded icons fade uniformly instead of going monochrome on disable.Wrap labels with tight line height
AutoShrinkTileLabelpreviously shrank labels down to 9sp until they fit on a single line. Three things to fix:maxLines = 2made Compose character-break "Preformatted" into "Preform" / "atted". Compose's default soft-wrap will break inside a word to satisfy the line budget, so single-word labels now measure withmaxLines = 1and render withsoftWrap = falseto force the shrink path instead.lineHeight, the gap between two wrapped lines tracked the font's default leading and read too tall. M3 typography presets handle this viaLineHeightStyle.Trim.Both, but customsp + lineHeightdoesn't pick it up — set it explicitly so labels sit centred against the reserved two-line height.minLines = 2reserves two lines worth of vertical space at the resolved size so tile heights stay consistent across the grid even when neighbouring labels need different line counts.Test plan
./gradlew :Gutenberg:test detekt :Gutenberg:lintDebug :app:compileDebugKotlin— BUILD SUCCESSFULenableNativeBlockInserteron: icons render correctly, single-word labels (e.g. "Preformatted") shrink rather than break, multi-word labels (e.g. "Featured Image") wrap at the space, two-line labels have tight line spacing, tile heights stay aligned across the grid.Related