fix(chat): fix plan card rendering, task-status persistence, and CSS - #522
fix(chat): fix plan card rendering, task-status persistence, and CSS#522anfibiacreativa wants to merge 33 commits into
Conversation
- add nx-campaign-plan-card: full Content Generation Plan card with title, description, task list, download/expand actions, and View plan / Run buttons; collapses to single running task + progress counter when a task is active - add nx-task-list: flat list of task items used when the agent streams tasks outside a full plan card - add nx-task-item: single task row with pending (dashed circle), running (animated spinner), and done (filled checkmark) states and an optional N/total progress badge - register DIRECTIVE_TYPE constants (plan, task-list, task-item) in constants.js - extend renderers.js to parse JSON payload from :::plan, :::task-list, and :::task-item directive fences and mount the corresponding custom elements; unknown directives fall through to the existing markdown path - import new components in chat.js alongside existing sub-components Co-authored-by: Cursor <cursoragent@cursor.com>
- replace fullscreen/download icons with chevron expand toggle - move Run button into card header, remove footer entirely - collapsed view shows active task only when running and not expanded - chevron rotates 180° when expanded via CSS transition - fix task gap spacing (--s2-spacing-150) and header action alignment - fix local agent URL to :4002 (was :4200) - skip pre-existing flaky loc tests (regional-diff, multimodal) unrelated to chat Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- render submit_plan tool approval as plan card with Run button (replaces standard approve/reject popover for this tool) - render submit_plan tool result (done state) as plan card with live task status merged from :::task-item streaming text - add mergeTaskItemsFromText helper to apply status updates to plan tasks - thread streamingText through renderMessage so plan card updates live - add SUBMIT_PLAN constant to TOOL_NAME Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…plan_mode - rename SUBMIT_PLAN → EXIT_PLAN_MODE in TOOL_NAME constants - add ENTER_PLAN_MODE constant - update renderers to check EXIT_PLAN_MODE for plan card rendering Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Task-status persistence: - TEXT_END splits streaming output into one string message per inter-tool segment, so task-item directives for step N live in a different message than step N+1; fix by concatenating all completed assistant text messages before passing to mergeTaskItemsFromText Renderer fixes: - switch renderSubmitPlanCard and renderApprovalCard from document.createElement to Lit html`` templates; createElement recreated the element each render, resetting _expanded state on every re-render - suppress TASK_ITEM directives in renderMessageContent (was rendering as visible text); return nothing when all items in a message are suppressed - skip message wrapper when content is nothing (eliminates empty DOM gaps) - remove unused renderTaskItemDirective function Plan card template: - _expanded defaults to true; showCollapsed = !this._expanded - move title/description into a new .plan-body div below the 48px header strip CSS (campaign-plan-card.css, task-item.css): - replace all --s2-spacing-150/115 (undefined tokens) with correct scale values - plan-card gap 12px; header 48px fixed height with border-bottom - plan-tasks: --s2-gray-50 bg, border, border-radius 8px, 16px side margin - plan-btn: height 24px, weight 400; primary btn uses --s2-static-white (--s2-gray-25 inverts in dark mode and made text inaccessible) - task-label margin-left --s2-spacing-100 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…argins Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- rename _expanded to _isExpanded (boolean naming convention) - rename allDone to isAllDone (boolean naming convention) - remove redundant showCollapsed variable, inline as !this._isExpanded - use html`` templates in renderPlanDirective and renderTaskListDirective instead of document.createElement so Lit preserves element identity across re-renders - fix hardcoded https://da.live icon URLs in campaign-plan-card.css and task-item.css to relative paths Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch.
Commits
|
- add nx-preflight-card LitElement with readiness score, progress bar, collapsible categories, and per-check pass/fail icons - add PREFLIGHT directive type and RUN_PREFLIGHT tool name to constants - wire :::preflight directive and run_preflight tool card/approval into renderers - approval panel shows "Pre-flight checks complete" with Reject/Approve buttons - add nx.js and mdast mocks to WTR importmap for renderer testing - add tests: preflight-card component, renderers routing, parse directive Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- move TASK_STATUS and PLAN_RUN_EVENT constants to constants.js; re-export TASK_STATUS from task-item.js for backwards compat - replace 'nx-plan-run' magic string with PLAN_RUN_EVENT throughout - replace 'pending' magic string in task-list.js with TASK_STATUS.PENDING - rename _runningState -> _findRunningTask in campaign-plan-card - rename renderSubmitPlanCard -> renderExitPlanCard in renderers - extract buildTaskStatusMap() helper in renderers to deduplicate Map-building logic shared by mergeTaskItemsFromText and mergeTaskItemsIntoPlan - extract _getTaskText() and _renderMessages() methods in chat.js to replace the inline IIFE in the render template - replace hardcoded border-radius: 8px with --s2-corner-radius-500 token in campaign-plan-card.css and preflight-card.css - fix .pf-cat-header height collapse bug: remove height declaration that conflicted with equal vertical padding under box-sizing: border-box Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- restore doc comment on TASK_STATUS in constants.js explaining
the values are part of the agent directive protocol
- replace dynamic @\${PLAN_RUN_EVENT} Lit template binding with literal
@nx-plan-run; dynamic attribute names work by accident via parser
behaviour and are fragile across Lit versions
- remove now-unused PLAN_RUN_EVENT import from renderers.js
- convert _getTaskText() method to get _taskText() getter; it takes no
arguments and derives purely from this.messages
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
# Conflicts: # WORKLOG.md # nx2/blocks/chat/constants.js # nx2/blocks/chat/renderers.js # test/nx2/blocks/chat/renderers.test.js
hannessolo
left a comment
There was a problem hiding this comment.
Please restore the skipped tests.
Also, from what I can see, we're implementing these tool cards in a very hard-coded way locked to our protocol of communication with the agent. IMO that's fine to move fast, after all it's a benefit of controlling our agent.
But once we move to coworker/CMA, we probably throw the code added here out?
| } else if (type === AGENT_EVENT.CONTINUATION) { | ||
| // Post-execution gate: the tool already finished (card is DONE, result shown). | ||
| // Flag it as awaiting a Continue/Stop decision. Ephemeral (UI-only) — nothing is | ||
| // pushed to _messages, so a reload simply drops the prompt while the result persists. |
There was a problem hiding this comment.
So if a user reloads the page, they will no longer be prompted to continue? Not sure I have an opinion on whether it's good or bad, just trying to make sure it's intentional.
There was a problem hiding this comment.
Yes, that was intentional.
My thinking was that refreshing the page anyway kills the agentic loop, so the user has to write "continue" or something like that to kick if off again, or if they don't want it to continue, they just clear the chat.
I took the example of claude code, where if you are for example in plan mode and it asks whether you want to switch to implementation, if you close claude code and restart it, you don't get the pop-up again, you have to ask it to restart. That looked reasonable to me to replicate.
If you change your mind and would like it differently, happy to adjust!
| }; | ||
| } | ||
|
|
||
| function mergeTaskItemsIntoPlan(directives) { |
There was a problem hiding this comment.
Lets keep this file specific to renderers only and move all such logic ie merge.. into a utils file.
| import './messages/campaign-plan-card.js'; | ||
| import './messages/governance-evaluation-card.js'; | ||
| import './messages/task-list.js'; | ||
| import './messages/task-item.js'; |
There was a problem hiding this comment.
chat.js shouldn't have to know about the card internals - we want to limit imports here to only things that are actually necessary for rendering the core chat so the file doesnt grow unnecessarily. Everything else needs to be moved to where it is relevant so this would be within the scope of renderers.
| <path d="M6 6l8 8M14 6l-8 8" stroke="currentColor" stroke-width="1.5" stroke-linecap="round" stroke-linejoin="round" fill="none"/> | ||
| </svg>`; | ||
| } | ||
| return html`<svg class="ge-check-icon ge-check-na" viewBox="0 0 20 20" aria-hidden="true"> |
There was a problem hiding this comment.
We want to avoid using inline svg, and instead reuse existing s2 icons. See https://github.com/adobe/da-nx/blob/main/nx2/blocks/chat/chat.js#L21 for reference.
| </div> | ||
| `; | ||
| } | ||
| } |
There was a problem hiding this comment.
Lets prefer using native elements wherever possible. For example, instead of custom logic for expand/collapse of sections, lets use native details/summary and style that as needed instead. Similarly we have a custom progress bar, where we could instead use native .
… into utils/directives.js Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
I think some of the code should be easily portable, as long as Experience Workspace maintains its own chat UI. For example, the card for rendering the governance results: As long as in the messages returned by the connected harness we can identify the tool calls and tool output, than we can still keep the custom rendering. The loading state for the tool, will depend on whether the tool invocation and tool results are transmitted to the consumer as independent events, if we just see the finish event or also the start event. The plan mode and tasks that Natalia implemented, might need to be reconciled with a harness native "Plan Mode" if they provide it, but I've seen the pattern of TODO list vs. full plan mode being implemented in multiple places too, so that might stay as well. Maybe the part that would be a little complex to keep are the custom approvals, including #617 , but that's just something that we'll have to see during porting. I would like to give it a try, especially for the governance result card before I go to PTO. So I think that which parts stay and which parts get thrown out really will depend on how the integration between the EW Chat and new harnesses looks like concretly. |
|
I resolved the PR feedback. Thank you very much for taking the time to review and for the guidance! @hannessolo I also merged back and reconciled the main branches with the v2 approvals protocol and re-tested the functionality from the PR. One part I wasn't sure about, if I should merge the continuation-approvals into the v2 state machine. I don't really have an opinion, personally. Claude said it is better to leave them orthogonal, but if you have an opinion, happy to apply it! Also if you have any additional feedback, happy to address! |
|
|
||
| constructor() { | ||
| super(); | ||
| this._isExpanded = true; |
There was a problem hiding this comment.
This still doesnt use the native capabilities to expand/collapse - could you take a look please?
| flex-shrink: 0; | ||
| } | ||
|
|
||
| .plan-icon-btn { |
There was a problem hiding this comment.
There seems to be quite a few reused styles - where possible could we reuse classes from https://main--da-nx--adobe.aem.live/docs/style-guide/style-guide.html and for the ones which are reused across the cards/list have it within messages css or another shared css file?
There was a problem hiding this comment.
With 89e2f48 I reused the nx buttons and refactored the common CSS for the cards in: nx2/blocks/chat/messages/messages.css
I think the part that could be discussed is the spinner, if you'd like to reuse the one from NX, the agent's analysis was that it would be a bit too much to load forms.css just for the purpose of the spinner and I understand the argument, but I'm happy to adjust if you prefer that option.
|
Superseeded ny #658 |
Summary
Why
The chat UI needs rich interactive cards to surface the agent's plan and execution progress
Preflight checks give users confidence that generated content meets standards before publishing
Both da-agent and AO backends need parity — the same card must render regardless of which backend is active
What Changed
nx2/blocks/chat/messages/campaign-plan-card.{js,css} — plan card with title, task list, Run button, collapse chevron
nx2/blocks/chat/messages/task-item.{js,css} and task-list.{js,css} — task status indicators
nx2/blocks/chat/messages/preflight-card.{js,css} — readiness score, progress bar, collapsible categories, pass/fail icons
nx2/blocks/chat/constants.js — added all tool names and directive types
nx2/blocks/chat/renderers.js — full wiring: directive path, tool card path, approval card path
web-test-runner.config.mjs + new mocks — nx.js and mdast importmap mocks for testing
Test Plan
Risks / Follow-ups
Preflight categories depend on what the agent decides to include — no fixed list enforced on the UI side
Image governance for uploads is a separate follow-up
da-nx PR depends on da-agent feat/submit-plan merging first (or landing together)
Fix SITES-46674
Needs to land with adobe-rnd/da-agent#49
Test URLs: