feat(web,mobile): environment icons can be an emoji, a monogram, or colored - #3
amanthanvi wants to merge 8 commits into
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @amanthanvi, you've used your own review budget of 250,000 diff characters for the last 7 days.
You can request another review in 1 day and 19 hours by commenting @sourcery-ai review. Upgrade to get a review now.
github-actions
Bot
added
the
vouch:trusted
Reviewer's GuideIntroduces synchronized environment icon customization across web and mobile, supporting colored named icons, emoji, and validated two-character monograms; shared contracts govern persistence and legacy compatibility, while renderer-specific sizing, accessibility, caching, capability locks, lazy loading, and adaptive themes keep the UI consistent. Sequence diagram for saving a customized environment iconsequenceDiagram
actor User
participant Dialog as EnvironmentIconPickerDialog
participant Logic as EnvironmentIconPickerLogic
participant Settings as EnvironmentSettings
participant Server
participant Row as EnvironmentConnectionRow
User->>Dialog: Choose icon, emoji, or monogram
Dialog->>Logic: resolveEnvironmentIconDialogWrite()
Logic-->>Dialog: write icon or invalid reason
alt valid selection
Dialog->>Settings: onSelect(icon)
Settings->>Server: updateSettings()
Server-->>Settings: Persist environment icon
Settings-->>Row: resolveEnvironmentIcon()
Row-->>User: Render customized icon
else invalid monogram or emoji
Dialog-->>User: Show validation reason
end
Flow diagram for environment icon selection and compatibilityflowchart LR
Current[Current environment icon] --> Dialog[Picker dialog]
Dialog --> Choice{Selected variant}
Choice --> Named[Named icon]
Choice --> Emoji[Emoji]
Choice --> Monogram[One or two character monogram]
Named --> Detect{Matches detected machine kind?}
Detect -->|Yes| Null[Store null for automatic detection]
Detect -->|No| Reference[environmentIconForCuratedId]
Emoji --> Rich[Store rich icon object]
Monogram --> Validate[Normalize and validate bound]
Validate --> Rich
Null --> Server[Environment server settings]
Reference --> Server
Rich --> Server
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
f06c172 to
9647698
Compare
Thread transfer impact
This comment will update automatically after the next completed run. |
9647698 to
1d8fcd8
Compare
9e87c26 to
eaacbd5
Compare
eaacbd5 to
a68f851
Compare
8ce70d9 to
48cae25
Compare
|
Pushed
The pattern is now The fallback still splits a Devanagari conjunct that Tests in |
48cae25 to
37dd387
Compare
Rebased onto current mainRebased the whole stack onto main at
A clean textual replay does not prove the stack still holds together, so I went looking for the hazard that would hide behind one. Layer 2 renames Each layer is its own pull request, so I verified each one standing alone rather than only at the tip:
No review thread was open when I rebased, so the force push moved commits rather than answers. Line comments from earlier rounds now anchor to the old SHAs. |
|
@sourcery-ai review |
|
Sorry @amanthanvi, you've used your own review budget of 250,000 diff characters for the last 7 days. You can request another review in 28 minutes by commenting |
37dd387 to
23f445b
Compare
Rebased onto current mainRebased the stack onto main at What main changed that this stack had to follow
That made one commit unnecessary. The layer 3 commit that deduplicates the icon submenu's lock row said it existed for the ceiling. It still removes a duplicated row, so it stays with a message that says only that. Conflicts
Review findingsSourcery reviewed the last push. One finding was real and is fixed on layer 6: the web dialog let Save run while an image was still encoding, which wrote the previous image and dropped the new pick. The other two have replies on the threads. One asks mobile to hide the emoji glyph from screen readers, but mobile glyphs have been labeled on main all along. The other asks to reject a photo whose dimensions the picker did not report, which is a trade-off the code already records. Layer 6 also drops an Per-layer verificationEach layer is its own pull request, so each was checked standing alone.
Layer 1 runs one test fewer than last time because main removed one of its own tests from The force push moved commits, so line comments from earlier rounds now anchor to the old SHAs. |
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="apps/mobile/src/lib/grapheme.ts" line_range="2" />
<code_context>
+/**
+ * First user-perceived character of `text`, taken as a base character with the
+ * combining marks and joiners that bind to it. That is right for the letters
+ * and digits a monogram is made of, and it is the count `isMonogramLength`
+ * holds the text to, so the tiles never disagree with the bound the picker
+ * enforced. `Intl.Segmenter` would be exact and Hermes does not ship it.
+ *
+ * The joiners matter because `MonogramText` admits them. Without them
</code_context>
<issue_to_address>
**nitpick:** The doc comment claims `firstGrapheme` returns a user-perceived character, but the implementation deliberately splits scripts such as Devanagari conjuncts that `Intl.Segmenter` treats as one grapheme; the comment therefore describes behavior the function does not provide.
**Suggested fix:** Describe this as the contract's simplified base-plus-mark/joiner segmentation rather than user-perceived grapheme segmentation.
```suggestion
* First simplified base-plus-mark/joiner segment of `text`: a base character with the
```
</issue_to_address>Sourcery assessment
Needs a human reviewer. If the icon schema, picker logic, or rendering is wrong, an environment can retain an incorrect icon that is visible across connected devices even after this code is reverted. The stored value is bounded and can be repaired by selecting another icon, but reverting alone does not remove existing overrides.
aa4f96b to
e582cba
Compare
|
@sourcery-ai review |
|
Sorry @amanthanvi, you've used your own review budget of 250,000 diff characters for the last 7 days. You can request another review in 5 days and 9 hours by commenting |
e582cba to
84396b9
Compare
Rebased onto current mainRebased the stack onto main at Main added four web lint errors in that range: Two changes follow main:
Each layer typechecks with 0 errors in every package it changes, lints with 0 errors, and passes its tests, from 4 files and 244 tests at layer 1 to 12 files and 327 tests at layer 6. The fix commits cited in earlier review replies have new SHAs, and those replies now point at them. |
84396b9 to
b1e1f06
Compare
Rebased onto orchestrator V2Rebased the stack onto main at What the rebase had to adapt:
Layer 1 has one new fix. An inline PNG must now carry all eight signature bytes, since Macroscope found that the old prefix check let a seven-byte value through. Every layer passes typecheck, lint, format, and its own tests. At the top of the stack, I picked icons on web and in the iOS simulator against isolated dev state, and each client showed the other's pick. |
|
@sourcery-ai review |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Invalid custom emoji input can save a different value, and mobile decorative icons create duplicate screen-reader announcements.
Review effort: Balanced
Findings: 2
Open (2)
What changed in this PR
Adds customizable environment icons across web and mobile, including emojis, monograms, and colors.
Changes:
- Adds a web icon-picker dialog with compatibility checks and validation.
- Extends web/mobile renderers for rich environment icons.
- Adds shared selection logic, mobile theme colors, tests, and documentation.
| File | Description |
|---|---|
packages/contracts/src/server.ts |
Centralizes curated-icon resolution. |
docs/user/project-settings.md |
Documents environment icon customization. |
apps/web/src/projectIconOptions.ts |
Moves emoji utilities out of Lucide-dependent code. |
apps/web/src/projectIconOptions.test.ts |
Updates emoji utility imports. |
apps/web/src/iconEmoji.ts |
Defines emoji choices and parsing. |
apps/web/src/components/Sidebar.tsx |
Allows selected icon colors to override tint. |
apps/web/src/components/settings/ProjectIconPickerDialog.tsx |
Uses extracted emoji utilities. |
apps/web/src/components/settings/EnvironmentIconPickerDialog.tsx |
Implements the web picker dialog. |
apps/web/src/components/settings/EnvironmentIconPicker.tsx |
Replaces the submenu with a lazy-loaded dialog host. |
apps/web/src/components/settings/EnvironmentIconPicker.test.ts |
Tests picker write and validation rules. |
apps/web/src/components/settings/EnvironmentIconPicker.logic.ts |
Encapsulates picker compatibility and write logic. |
apps/web/src/components/settings/ConnectionsSettings.tsx |
Integrates the picker into connection rows. |
apps/web/src/components/EnvironmentMachineIcon.tsx |
Renders colored, emoji, and monogram icons. |
apps/mobile/src/lib/grapheme.ts |
Adds Hermes-compatible character splitting. |
apps/mobile/src/lib/grapheme.test.ts |
Tests grapheme splitting. |
apps/mobile/src/components/environmentMonogram.ts |
Splits monograms into display characters. |
apps/mobile/src/components/environmentMonogram.test.ts |
Tests monogram rendering input. |
apps/mobile/src/components/EnvironmentMachineSymbol.tsx |
Renders rich mobile environment icons. |
apps/mobile/src/components/environmentIconColors.ts |
Maps icon colors to adaptive theme classes. |
apps/mobile/scripts/generate-uniwind-themes.mts |
Adds adaptive palette variables. |
apps/mobile/generated-uniwind-themes.css |
Regenerates mobile theme output. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| <Text | ||
| accessibilityLabel={icon.emoji} | ||
| allowFontScaling={false} | ||
| numberOfLines={1} |
| aria-invalid={customEmoji.trim().length > 0 && firstEmoji(customEmoji) === null} | ||
| placeholder="Paste an emoji" | ||
| onChange={(event) => { | ||
| const value = event.currentTarget.value; | ||
| setCustomEmoji(value); | ||
| const nextEmoji = firstEmoji(value); | ||
| if (nextEmoji) setEmoji(nextEmoji); |
|
Sorry @amanthanvi, you've used your own review budget of 250,000 diff characters for the last 7 days. You can request another review in 4 days and 23 hours by commenting |
b1e1f06 to
7508198
Compare
…olored Fourteen named glyphs still cannot say which of three dev boxes is which, and a machine's role often has no icon at all. Both renderers now draw the emoji and monogram variants and apply a chosen color to a named icon. On web, emoji and monograms render as a span sized like a bare svg, since the menu and button rules that size icons match only `svg`; the option-slot helper caches wrapper components by value so a filter menu rebuilt each render keeps its component identity. On mobile, emoji and monograms get an explicit width, because emoji advance widths vary per glyph and platform and these sit in flex rows, and a monogram draws both characters at every size as web does, so the same machine never reads `K` on the phone and `K8` on the desktop. The mobile grapheme split does not assume `Intl.Segmenter`, which nothing in the repo proves this Hermes build ships. Emoji, monograms, and images are hidden from assistive technology like the svg glyphs are; the row's label already names the machine. A locked environment prints the reason beneath the disabled menu item, since a disabled item takes no pointer events and nothing hover-based could carry it. The text variants carry a plain `size-4` so a caller's size always wins. A responsive base like the menu's `size-4.5 sm:size-4` would survive the class merge as a separate variant and re-grow every icon at the breakpoint. A chosen color replaces the caller's resting tint. Rows draw connection state beside the glyph (a dot, the subtitle, or dimming the whole row), never on it, so a red icon never reads as a failed one. The web picker becomes a dialog opened from the row menu, since fourteen ids plus color swatches and a text input do not fit a radio submenu. The dialog is mounted beside the menu, not inside it, because a menu popup unmounts its children the moment an item is clicked. The monogram bound is enforced in the picker and not only the schema. An environment has no decider to catch what the picker lets through, so this is what keeps that error away from the user. Every decision the dialog makes is a pure function with tests. The emoji list and parser move to a Lucide-free module. The mobile theme generator gains one 600/400 pair per pickable color, matching what web draws in light and dark. Claude Fable 5.1 via Claude Code
`MonogramText` admits ZWJ and ZWNJ, but the fallback `firstGrapheme` uses when `Intl.Segmenter` is missing matched a base character and its combining marks only. So "AB" split into "A" and the bare joiner. The second tile drew an invisible character and the "B" was gone. `Intl.Segmenter` binds the joiner to the character before it, so the two paths disagreed on text a user can type into the picker. Bind the joiners in the fallback too. A Devanagari conjunct still splits where `Intl.Segmenter` keeps it whole, which the test now names as the known limit rather than an incidental result.
`firstGrapheme` reached `Intl.Segmenter` through a type assertion that restated the lib declaration. Reading the member directly keeps the same `typeof` guard, which a Hermes build that defines the property as undefined still needs. Also rewrites four comments that used a colon as a mid-sentence connector.
…gram tile The web picker built its own `Intl.Segmenter` at module scope to count graphemes, so a runtime without `Intl.Segmenter` failed to evaluate the module rather than falling back. Contracts already exports `isMonogramLength`, which guards the constructor and counts code points minus combining marks otherwise, so the picker calls that instead. The mobile monogram tile set `accessibilityLabel` on a `View` without `accessible`, which leaves the label unreachable. Every other branch in that component is already reachable. Also corrects the `monogramCharacters` comment, which claimed the function returns the whole text when it cuts anything past two graphemes. Model: Claude Opus 5 via T3 Code.
`environmentMachineIcon` built a string key for every icon so a module-level `Map` could cache the wrapper component. `resolveEnvironmentIcon` already returns a stored override by reference and memoizes a detected icon per machine kind, so the icon object is stable for as long as the settings snapshot it came from. A `WeakMap` keyed on it hits on the same renders the string key did, drops the key builder, and holds nothing once the snapshot is replaced. The plain curated case no longer skips the wrapper. That branch returned the Lucide component itself, and the wrapper it avoided renders the same svg with the same two props. Also drops the `firstEmoji` and `PROJECT_EMOJIS` re-export this layer added to `projectIconOptions.ts`. That module reads the whole Lucide name list at import time, which is why the emoji helpers moved to `iconEmoji.ts`, so re-exporting them from it points readers back at the module the split avoids. Its two importers now read `iconEmoji.ts` directly. Model: Claude Opus 5 via T3 Code.
…tores A pick from the curated grid follows two rules. A machine kind has to go through `environmentIconForMachineKind`, because rows are memoized on the icon object and because that variant encodes to the bare string an older server accepts. A role has neither property and stays a named icon. The ternary that carried both was copied at the write rule and again at the grid preview, with only one copy commenting on the wire form, and the mobile picker two layers up adds two more copies. Move it beside the reference cache it depends on, where the comment can state both halves once. Model: Claude Opus 5 via T3 Code.
`firstGrapheme` used `Intl.Segmenter` where it existed. This file only ever runs on Hermes, which ships none, so that branch was live in vitest on Node and dead in the app, which left the tests covering a path that never shipped. It is also why `t3code/no-hermes-unsupported-apis` reports the constructor under `apps/mobile`, at error severity. Taking a base character with its combining marks and joiners is what the app was already doing, and it matches how `isMonogramLength` counts, so two tiles show exactly what the picker accepted. The stubs that shadowed `Intl.Segmenter` go with it.
EnvironmentIconPickerHost renders nothing while closed, so the dialog mounts fresh on every open and its useState initializers already start from the current icon. The effect that re-applied the same state on open, its ref, and the open prop that was always true are gone. The pattern came from the project icon dialog, which stays mounted. Claude Opus 5.5 via Claude Code
7508198 to
8103351
Compare
What changed
Both renderers draw the emoji and monogram variants and apply a chosen color to a named icon.
svg. The option-slot helper caches its wrapper components on the icon object, which the resolver hands back by reference, so a filter menu rebuilt on each keystroke keeps one component identity per environment and its icon subtree does not remount. Keying on the object rather than a rendered string also means the image variant two layers up does not put a 32 KB data URL in a map key.Kon the phone andK8on the desktop. The grapheme split never usesIntl.Segmenter, which Hermes does not ship, and it counts the way the contract's monogram bound counts, so two tiles show exactly what the picker accepted. The theme generator gains one 600/400 pair per pickable color.projectIconOptions.tsreads the whole Lucide name list at import, and the environment picker should not.Why
Fourteen named glyphs still cannot say which of three dev boxes is which, and a machine's role often has no icon at all.
Stacked on #2. Layer 4 of 6.
Opened on the fork because GitHub only accepts a base branch that lives in the base repository; it will be re-targeted to pingdotgg/t3code once its base merges. The entry point upstream is pingdotgg#15511.
UI changes
Verification
Two sandbox servers paired into one web client, with state seeded from a snapshot and never the live install:
settings.jsonas{"kind":"icon","name":"database","color":"violet"}; the row's svg carriedlucide-database … text-violet-600 dark:text-violet-400.{"kind":"emoji","emoji":"🚀"}; the row rendered the sized span.k8saved as{"kind":"monogram","text":"K8","color":"teal"}with the svg textK8in teal.nulland the key left the file.EnvironmentIconPicker.test.ts,projectIconOptions.test.ts, mobilegrapheme.test.ts,environmentMonogram.test.ts, and the theme generator freshness test.A device check is still missing for two-character monogram legibility at the 10 to 12 point thread-list sizes, which no typecheck can hold.
An adversarial review of this layer found and fixed, before opening: the lock reason had become invisible on a disabled menu item; the emoji span's responsive size variant survived the class merge and re-grew every icon to 16 px on desktop; mobile collapsed monograms to one character while web drew both; and emoji were read aloud by screen readers.
Claude Fable 5.1 via Claude Code
Summary by Sourcery
Enable users to distinguish connected environments with customizable icons, emojis, monograms, and colors across web and mobile.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests: