fix(chat): restore the mars rover backdrop in inbox search - #29603
Merged
Merged
Conversation
Two independent bugs kept the desktop search pane's parallax scene off screen. Zero-area lazy images never load. ImageIconDesktop emits an <img> with no width/height attribute, and the scene's three layers are absolutely positioned with only bottom/left, so each had a 0x0 layout box until its bitmap arrived -- but Chromium never fires lazy-load for a zero-area box, so the image that would have given it a size could never load. A silent deadlock: complete:false, naturalWidth:0, no request, no console error. Confirmed with a cache-busted matrix in the running app; sized or eager both break the cycle, position does not matter (the rover sprite escaped only by luck of its in-flow wrapper). Give each layer the asset's own pixel size. The wrapper styles are split from the image styles so the names mean what they say, which also drops roverLayer: its bottom/left were dead, overwritten inline by transRoverY/transRoverX every render. srcSet advertised candidates with no file behind them. It was hardcoded to 1x/2x/3x for every icon, but 22 icons in iconMeta don't ship all three -- the mars trio has no @3x. The browser picks one candidate up front and never falls back, so at devicePixelRatio 3 (Windows at 300% scaling) all three 404'd and the scene stayed missing even once sized. Worse, a sized failed image draws a broken-image glyph where an unsized one drew nothing. The generator already read the full directory listing and threw the @2x/@3x information away; it now records the available multiples as `mults`, emitted only when it isn't the default. image-icon builds srcSet from that, so a missing @3x falls back to @2x. The dark <source> now resolves its dir and extension from the dark asset's own name rather than the light icon's. Along the way: icon-placeholder-avatar-lighter-32@2.png and @3.png are really 64px and 96px retina art that lost the trailing x in their filenames, so the generator read them as separate 1x icons and was about to bake mults:[1] into the constants. Renamed. Verified in the running app against a cold cache at DPR 2 and DPR 3: all three layers load and render 539x379 / 74x78 / 539x90, with zero 4xx image responses app-wide.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The generator-updated icon.constants-gen.d.ts declarations don’t currently include the new multsFor export, which can break typed consumers relying on that .d.ts.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Restores the desktop inbox-search Mars rover parallax backdrop by (1) preventing srcSet from advertising non-existent density candidates and (2) ensuring the parallax layers have non-zero layout size so Chromium lazy-loading will actually fetch them.
Changes:
- Generate and consume per-asset available retina multipliers (
mults) so desktopsrcSetonly includes files that exist (including correct resolution for dark variants). - Adjust inbox-search rover backdrop layout so each layer has explicit pixel dimensions and uses a dedicated positioning wrapper.
- Rename/add corrected retina placeholder-avatar assets so the generator doesn’t misclassify them as separate 1x icons.
File summaries
| File | Description |
|---|---|
| shared/desktop/yarn-helper/font.mts | Generator now detects which @Nx variants exist per asset and emits mults metadata. |
| shared/common-adapters/icon.constants-gen.tsx | Adds multsFor() helper to expose per-icon multipliers with a default fallback. |
| shared/common-adapters/icon.constants-gen.shared.tsx | Generated metadata now includes mults for assets missing some multipliers (e.g. Mars rover layers). |
| shared/common-adapters/icon.constants-gen.d.ts | Updates generated declaration types for mults/nameDark (but currently missing a multsFor export declaration). |
| shared/common-adapters/image-icon.tsx | Desktop srcSet now built from multsFor() and resolves dark <source> candidates using the dark asset’s own dir/extension. |
| shared/chat/inbox-search/background.tsx | Adds explicit sizing and wrapper styles so lazy-loaded absolutely-positioned layers are fetchable/rendered. |
| shared/images/icons/icon-placeholder-avatar-lighter-32@2x.png | Updated/renamed asset to correctly represent a 2x retina candidate. |
| shared/images/icons/icon-placeholder-avatar-lighter-32@3x.png | Updated/renamed asset to correctly represent a 3x retina candidate. |
Review details
- Files reviewed: 5/8 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
207
to
214
| gridSize?: number | ||
| extension?: string | ||
| charCode?: number | ||
| nameDark?: string | ||
| mults?: ReadonlyArray<number> | ||
| nameDark?: IconType | ||
| imagesDir?: string | ||
| require?: ReqOut | ||
| requireDark?: ReqOut |
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.
The parallax mars scene at the bottom of the desktop inbox search pane was gone. Two independent bugs, either of which alone was enough to hide it.
Zero-area lazy images never load
ImageIconDesktopemits an<img>with nowidth/heightattribute. The scene's three layers are absolutely positioned with onlybottom/left, so each had a 0×0 layout box until its bitmap arrived — and Chromium never fires lazy-load for a zero-area box, so the image that would have given it a size could never load. Silent:complete:false,naturalWidth:0, no network request, no console error.Confirmed with a cache-busted A/B matrix in the running app:
So it's the zero area, not
position: absolute. The rover sprite escaped only by luck of its in-flow wrapper.Fix: give each layer the asset's own pixel size. The wrapper styles are split from the image styles so the names mean what they say, which also drops
roverLayer— itsbottom/leftwere dead, overwritten inline bytransRoverY/transRoverXevery render.srcSet advertised candidates with no file behind them
srcSetwas hardcoded to1x, 2x, 3xfor every icon, but 22 icons iniconMetadon't ship all three — the mars trio has no@3x. The browser picks one candidate up front and never falls back if it 404s, so atdevicePixelRatio3 (Windows at 300% scaling) all three 404'd and the scene stayed missing even once sized. Worse, a sized failed image draws a broken-image glyph where an unsized one drew nothing, so fixing the first bug alone would have traded "invisible" for "three broken glyphs".The generator already read the full directory listing and threw the
@2x/@3xinformation away. It now records the available multiples asmults, emitted only when it isn't the default[1, 2, 3], andimage-iconbuildssrcSetfrom that — so a missing@3xfalls back to@2x. The dark<source>now resolves its directory and extension from the dark asset's own name rather than the light icon's (latent, but an all-404 dark srcSet for any asset that differed).Also
icon-placeholder-avatar-lighter-32@2.pngand@3.pngare really 64px and 96px retina art that lost the trailingxin their filenames, so the generator read them as separate 1x icons and was about to bakemults: [1]into the constants, pinning that icon to the 32px asset forever. Renamed.Verification
In the running app against a cold cache (the probes warm these URLs, so a warm run is a false green), reloaded under the DPR override —
setDeviceMetricsOverridealone does not re-run srcset selection on already-loaded images:@2x, loads, renders 539×379 / 74×78 / 539×90. Previously 404 on all three.<source>still resolves all three candidates against the dark asset name.yarn lint:allclean — eslint quiet, 1727 compiled / 0 bailed out / 0 whole-props deps, tsc green on both desktop and native.