Rework pinned icon size into device-adaptive presets#86
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d3b87c3aba
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| fun launcherInteriorPx(context: Context): Int { | ||
| val res = context.resources | ||
| val dm = res.displayMetrics | ||
| val shortEdgePx = min(dm.widthPixels, dm.heightPixels) |
There was a problem hiding this comment.
When HomeActivity receives a handled screenSize configuration change (the manifest handles keyboardHidden|orientation|screenSize, and onConfigurationChanged only reapplies the dash columns), this new display-metrics-derived size is never pushed through the launcher views. Resizing the window/display therefore leaves existing launcher icons and ibPanelDashClose at the old size until a preference change or activity restart, so the launcher sizing needs to be rerun from onConfigurationChanged or the activity should be recreated for that config change.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 6291dda, by removing the staleness class rather than re-applying in onConfigurationChanged: launcherInteriorPx now derives the short edge as smallestScreenWidthDp × density — the same stable anchor the slot count already uses — instead of min(displayMetrics.width, height). Every input the computed size depends on (smallestScreenSize, density, theme) is one the manifest does not handle, so changing any of them recreates the activity and re-sizes everything; a handled screenSize-only change (rotation, long-axis resize) leaves the size genuinely unchanged by design. Pinned by the new interiorAnchoredToSmallestWidth test.
Generated by Claude Code
Addresses the Codex review on #86: the manifest handles screenSize changes without recreating, and nothing re-applied the launcher sizing there, so a size derived from min(displayMetrics.width, height) could go stale (that min can also drift across rotation with 3-button nav). Deriving the short edge as smallestScreenWidthDp × density — the same stable anchor the slot count already uses — makes every input the size depends on (smallestScreenSize, density, theme) one that recreates the activity when it changes, so a computed size can never go stale and no onConfigurationChanged re-apply is needed. https://claude.ai/code/session_01Hg7YREsBsCX87ZsrRf6MNw
Replace the raw dp "Pinned icon size" slider (LAUNCHERICON_WIDTH, the opaque (48 + value) * density formula) with five presets — Huge, Large, Default, Small, Tiny — whose pixel size is computed at runtime and never stored. The preset maps to a whole slot count derived from the screen alone (smallestScreenWidthDp + a target physical icon size), so the same preset yields the same count on every theme and the default adapts to the device instead of being calibrated to one phone. The icon size is then chosen so exactly that many slots tile the launcher's usable interior on the screen's shortest edge — subtracting the theme's launcher margins and its launcher background 9-patch insets — so no icon is cut off and no sliver of an extra one shows. The launcher scroll viewports are floored to a whole multiple of the slot size to enforce that. The BFB counts as one of the slots. New LauncherIconGrid mirrors DashGrid: clamping the count (not the size) keeps icons within sane dp bounds while preserving the exact division. No migration — a new preference key is read, so existing installs land on "Default". Covered by LauncherIconGridTest (adaptive count, exact fit per theme, cross-theme count consistency, physical-size sanity, clamping and viewport clipping edge cases) and an updated ReactivePrefsTest. https://claude.ai/code/session_01Hg7YREsBsCX87ZsrRf6MNw
Exercise the actual framework measure pass on ClippingScrollView / ClippingHorizontalScrollView to confirm the viewport floors to a whole multiple of the icon slot (the no-sliver guarantee), beyond the pure viewportClipPx arithmetic. https://claude.ai/code/session_01Hg7YREsBsCX87ZsrRf6MNw
- Clip the launcher scroll viewports only when the content actually overflows: without overflow no partial icon is possible, and PinnedAppsBar measures to a FRACTIONAL length during the per-desktop swipe morph so an auto-sizing launcher resizes smoothly — flooring that snapped the animation in whole-icon steps. - Memoise LauncherIconGrid's interior resolution (theme margins + launcher-background 9-patch insets): it is reached from onMeasure, which runs per frame during the morph, and previously inflated a 9-patch drawable and two TypedArrays on every call. - Resolve the icon size once in AppLauncher.init() and derive the height from it instead of running the full resolution twice. - Single-source the preset default (Preference.LAUNCHER_ICON_PRESET now uses LauncherIconGrid.DEFAULT_PRESET; the customise seekbar's XML progress/max, always overwritten by CustomiseModeUi, are dropped) and index the launcher_margin array with plain literals matching LauncherEdgeController's convention. - Strengthen the tests: assert the panel-close button actually changes size (not just matches the formula), pin explicit -port/-land qualifiers with preconditions on the rotation-stability tests, drop a mirror test that recomputed the implementation's own formula, and cover the new fits-without-overflow (no clip) behaviour. - Update AGENTS.md per its maintenance rule (LauncherIconGrid section, renamed ViewModel flow). https://claude.ai/code/session_01Hg7YREsBsCX87ZsrRf6MNw
Scroll views measure their child with an UNSPECIFIED spec along the scroll axis, so the tests' empty 5000px LinearLayout measured to zero and the new overflow-only clip correctly saw "content fits" — making the floors-to-whole-slots tests fail. Minimum sizes ARE honoured under UNSPECIFIED (in the real launcher the length comes from summing the icon children), so the fakes now set those, with preconditions so the tests fail loudly instead of going vacuous if measurement semantics change again. https://claude.ai/code/session_01Hg7YREsBsCX87ZsrRf6MNw
Addresses the Codex review on #86: the manifest handles screenSize changes without recreating, and nothing re-applied the launcher sizing there, so a size derived from min(displayMetrics.width, height) could go stale (that min can also drift across rotation with 3-button nav). Deriving the short edge as smallestScreenWidthDp × density — the same stable anchor the slot count already uses — makes every input the size depends on (smallestScreenSize, density, theme) one that recreates the activity when it changes, so a computed size can never go stale and no onConfigurationChanged re-apply is needed. https://claude.ai/code/session_01Hg7YREsBsCX87ZsrRf6MNw
6291dda to
9a8ac3f
Compare
2 participants
What
Replaces the raw dp "Pinned icon size" slider with five presets — Huge · Large · Default · Small · Tiny — whose pixel size is computed at runtime and never stored.
smallestScreenWidthDpagainst a target physical icon size), so the same preset yields the same count on every theme, and the default adapts to the device instead of being calibrated to one phone (~5 slots on a Galaxy S24-class screen, fewer on small phones, more on tablets — icons stay roughly the same physical size everywhere). The BFB counts as one of the slots.launcher_marginand its launcher-background 9-patch insets (resolved at runtime viaDrawable.getPadding) — so no icon is cut off and no sliver of an extra one shows, on any theme.ClippingScrollView/ClippingHorizontalScrollView) floor themselves to a whole multiple of the slot size, enforcing the no-sliver guarantee. While everything fits they leave the measure untouched, preservingPinnedAppsBar's fractional morph measure (smooth auto-sizing resize during per-desktop swipes).LauncherIconGridmirrorsDashGrid: clamping the count (not the size) keeps icons within sane dp bounds while preserving the exact division. The customise slider shows the preset label plus the resolved count for the current screen (e.g. "Default · 5 icons").launcher_icon_preset) is read; oldlaunchericon_widthvalues are ignored, so existing installs land on "Default".Self-review findings addressed
A multi-angle review pass surfaced and fixed:
onMeasureevery frame during morphs → memoised keyed on (theme, short edge, density);AppLauncher.init()resolves once instead of twice.AGENTS.mdper its maintenance rule, strengthened tautology-prone tests (before/after assertions, pinned-port/-landqualifiers with preconditions).Known limitations (flagged, intentionally not addressed here)
AppLauncher.init()sizing but keeps its plainScrollView(no whole-slot clip), as before.Testing
LauncherIconGridTest(23 cases): device-adaptive counts (S24-class ⇒ default 5, presets 3·4·5·6·7; adapts across 320–1200 dp), exact fit across all 8 themes with real resolved interiors, cross-theme count consistency, physical-size sanity (near-constant dp across screens, within [48, 160] dp), count clamping/degenerate ranges, rotation stability with pinned-port/-landconfigs.ClippingScrollViewTest: real framework measure passes — floors to whole slots on overflow (both axes), collapses sub-slot viewports to zero, leaves fitting (fractional, mid-morph) content untouched.ReactivePrefsTest: preset change resizes live without recreate, asserting the size actually changed.🤖 Generated with Claude Code
https://claude.ai/code/session_01Hg7YREsBsCX87ZsrRf6MNw
Generated by Claude Code