Repository navigation
Conversation
github-actions
Bot
added
the
[Type] Bug
jkmassel
force-pushed
the
jkmassel/native-inserter-no-redux-flash
branch
2 times, most recently
from
May 11, 2026 21:44
857a294 to
e84a41c
Compare
XCFramework BuildThis PR's XCFramework is available for testing. Add the following to your .package(url: "https://github.com/wordpress-mobile/GutenbergKit", branch: "pr-build/487")Built from 5dec7e8 |
jkmassel
force-pushed
the
jkmassel/native-inserter-no-redux-flash
branch
from
May 12, 2026 20:44
e84a41c to
045288c
Compare
oguzkocer
reviewed
May 12, 2026
oguzkocer
left a comment
oguzkocer
left a comment
Contributor
There was a problem hiding this comment.
@jkmassel Do you have a recording of the flash this is fixing? Because I tried recording before and after and I don't see any change. I am working with Claude to see maybe it's just a build issue on my end because I tested the fix before I tested the base branch. Here are the recordings:
Recording from: jkmassel/android-block-picker:
487-before.webm
Recording from jkmassel/native-inserter-no-redux-flash:
487-after.webm
oguzkocer
approved these changes
May 12, 2026
oguzkocer
left a comment
oguzkocer
left a comment
Contributor
There was a problem hiding this comment.
I think maybe the reason I am not seeing a flash is this:
a visible flash on slower devices.
So, I am going to approve the PR on the code change and the fact that I am not observing a flash after the change. 🤷
jkmassel
force-pushed
the
jkmassel/native-inserter-no-redux-flash
branch
from
May 13, 2026 14:50
045288c to
bceeae1
Compare
jkmassel
force-pushed
the
jkmassel/android-block-picker
branch
from
May 13, 2026 14:52
340121d to
d262201
Compare
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.
jkmassel
force-pushed
the
jkmassel/native-inserter-no-redux-flash
branch
from
May 13, 2026 16:07
bceeae1 to
5dec7e8
Compare
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.
Member
|
@jkmassel based on my understanding of the native inserter JavaScript code, I believe this change may not actually have the desired impact. Gutenberg's own The only visible flash I was able to reproduce appears related to animating the position of the native inserter while the virtual keyboard is open. This occurred with and without the Screen.Recording.2026-07-16.at.15.24.47.mov |
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
4 participants
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.
Summary
Tapping the
+button currently callssetIsInserterOpened(true)on the editor store, which momentarily renders Gutenberg's web inserter UI before the native dialog covers it — a visible flash on slower devices. CallprepareAndShowInserter()directly instead; the redux flag isn't useful when the inserter is rendered natively.Test plan
enableNativeBlockInserteron, tap the+toolbar buttononToggle(false)fromwindow.blockInserter.onClose)