Repository navigation
fix(chat-ao): use distinct Coworker icon for New chat button - #790
Open
yevheniia0831 wants to merge 16 commits into
Open
yevheniia0831 wants to merge 16 commits into
yevheniia0831 wants to merge 16 commits into
Conversation
Was reusing the generic 'add' icon shared with the attach-files button. Adds S2_Icon_Coworker_20_N.svg (sourced from the AI Mark asset Markus shared) and wires it into the New chat trigger only. Fix adobe/da-live#1355 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
A rejected promise from a module-scope 'const x = await loadHrefSvg(...)' (as in chat-ao.js and whatsnew.js) fails the whole module's evaluation, so customElements.define never runs and the component never upgrades. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…Base codeBase is set by the host (da-live) to its own origin, independent of which nx2 branch is loaded via ?nx=. That's why the Coworker icon (which only exists in da-nx, not yet published to da-live) 404'd and silently fell back to the old '+' icon. ICONS_BASE resolves relative to nx2's own import.meta.url, so it correctly follows an ?nx= branch override. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch.
|
| // Uses ICONS_BASE (nx2's own origin), not codeBase: codeBase is da-live's | ||
| // origin (see da-live/scripts/scripts.js), so an icon not yet published to | ||
| // da-live must be loaded from wherever this nx2 bundle itself is hosted. | ||
| const newChatIcon = (await loadHrefSvg(`${ICONS_BASE}S2_Icon_Coworker_20_N.svg`)) ?? icon('add'); |
Contributor
There was a problem hiding this comment.
Lets upload the coworker icon on da live content and use the same pattern as the other icons. I can help uploading if needed.
Contributor
Author
There was a problem hiding this comment.
@sharanyavinod, not sure I have CDN access. Can you tell me how to get it/upload the icon?
Contributor
There was a problem hiding this comment.
@yevheniia0831 I added it https://main--da-live--adobe.aem.live/img/icons/s2-icon-coworker-20-n.svg is the url
Contributor
There was a problem hiding this comment.
also please remove the comment. One const des not need 5 lines of comments :-)
Contributor
Author
There was a problem hiding this comment.
must have sneaked from copilot, sorry
Contributor
There was a problem hiding this comment.
Yes, it happens for me as well. I think LLM likes long comments. We maybe should add this to Claude MD to make sure they keep the comments short.
Contributor
Author
There was a problem hiding this comment.
It's there, but it gets ignored.
Is adding a lint rule for this overengineering?
mhaack
reviewed
Oct 6, 2026
Comment on lines
+789
to
+807
| ## New chat icon | ||
|
|
||
| The "New chat" button uses a distinct Coworker icon | ||
| (`nx2/img/icons/S2_Icon_Coworker_20_N.svg`) instead of the shared `+` icon | ||
| used elsewhere, so it's visually distinguishable as starting a fresh | ||
| Coworker session rather than a generic add action. It's loaded via | ||
| `loadHrefSvg` (`nx2/utils/svg.js`) rather than the CDN `icon()` helper | ||
| because the asset isn't (yet) published to the shared icon CDN. If the | ||
| fetch fails for any reason, the button falls back to the old `+` icon | ||
| (`icon('add')`) rather than rendering blank. | ||
|
|
||
| It's fetched from `ICONS_BASE` (`nx2/utils/svg.js`), not `codeBase`. | ||
| `codeBase` is set by the host (e.g. da-live's `scripts/scripts.js`) to the | ||
| *host's own* origin, independent of which nx2 branch is loaded via `?nx=` | ||
| — existing CDN icons resolve through it only because they're already | ||
| published on the host's side. `ICONS_BASE` instead resolves relative to | ||
| wherever this nx2 bundle itself was loaded from, so it correctly follows | ||
| an `?nx=` branch override for icons that only exist in this repo. | ||
|
|
Contributor
There was a problem hiding this comment.
can you shorten this? reads more like a work log then a spec. maybe remove it the icon does not need extra docs.
mhaack
reviewed
Oct 6, 2026
Comment on lines
+11
to
+13
| // Never reject: a caller awaiting this at module top level (e.g. a | ||
| // module-scope `const icon = await loadHrefSvg(...)`) would otherwise | ||
| // fail the whole module's evaluation on a network hiccup. |
Contributor
There was a problem hiding this comment.
Same as the other file, please remove the long comments
Icon is now published to da-live's shared icon CDN (s2-icon-coworker-20-n.svg), so switch to the standard icon() pattern used by every other icon instead of the ICONS_BASE/loadHrefSvg fallback. Also trims verbose comments per review feedback. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…arent one Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… Coworker product reference Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…agnostic naming Same CDN asset (s2-icon-coworker-20-n.svg), but the internal icon key no longer ties the UI to one backend. Addresses Sharanya's PR #790 concern that the chat UI is being extended to support CMA alongside Coworker. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… glyph shape Matches existing ICON_NAMES convention (keys describe the icon shape, e.g. add/clear/close/send/stop/mic), not a product/backend name. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…real product Adds .nx-btn-ai modifier: full pill radius (--s2-corner-radius-800) and a pink/orange/yellow pastel gradient background, matching the actual Coworker app's New chat button (screenshot reference), using existing design tokens so light/dark mode works automatically. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Real product's computed styles show a plain background-color (no gradient property on the button), so the pastel look is a blurred colored box-shadow glow around a plain pill, not a gradient fill. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ence Keep the pastel gradient fill matching the reference screenshot, and pick up font-weight: 700 from the real button's own computed styles. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This branch was successfully deployed
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
3 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.
Fix adobe/da-live#1355
Test URLs: