Skip to content

Commit 2cc29e1

Browse files
esokulluclaude
andcommitted
Announce the planner failure card and stop duplicating its Retry
Three loose ends around the recovery card: `addErrorRetryButton` only looked for the error and cost-allowance classes, so a message that also took an error update could grow a second Retry affordance next to the card's own; the reconnect path recomputed a fallback that its own `assistantEl` already applied; and the card said nothing to a screen reader. The role goes on the emptied container before its content, since readers announce insertions into a live region rather than a region that shows up already populated — which also keeps a restored card silent. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 364efc9 commit 2cc29e1

3 files changed

Lines changed: 26 additions & 8 deletions

File tree

src/chrome/src/ui/sidepanel.js

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3740,11 +3740,10 @@ async function adoptRestoredRunState(tabId, state) {
37403740
});
37413741
const returnedPlannerFailure = plannerRequestFailureUpdate(res?.updates);
37423742
if (returnedPlannerFailure && sameTabId(currentTabId, tabId) && !isTabAbortRequested(tabId)) {
3743-
const targetAssistantEl = assistantEl || currentAssistantEl;
37443743
renderPlannerRequestFailure(
3745-
targetAssistantEl,
3744+
assistantEl,
37463745
returnedPlannerFailure.data,
3747-
retryPayloadForRunAssistant(targetAssistantEl),
3746+
retryPayloadForRunAssistant(assistantEl),
37483747
);
37493748
}
37503749
const returnedErrorUpdate = Array.isArray(res?.updates)
@@ -5251,6 +5250,10 @@ function renderPlannerRequestFailure(assistantEl, data, retryPayload = null) {
52515250
assistantEl.classList.add('planner-request-failure');
52525251
textEl.classList.add('planner-request-failure-content');
52535252
textEl.replaceChildren();
5253+
// Set the role on the empty container first: screen readers announce
5254+
// content inserted into an existing live region, not a region that arrives
5255+
// already populated. A restored card stays silent, which is what we want.
5256+
textEl.setAttribute('role', 'alert');
52545257

52555258
const message = document.createElement('div');
52565259
message.className = 'planner-request-failure-message';
@@ -8860,7 +8863,8 @@ function configureRetryButton(btn, retryPayload) {
88608863
}
88618864

88628865
function addErrorRetryButton(msgEl, retryPayload) {
8863-
if (!msgEl || !retryPayload?.text || msgEl.querySelector('.error-retry-btn, .cost-allowance-retry-btn')) return;
8866+
if (!msgEl || !retryPayload?.text
8867+
|| msgEl.querySelector('.error-retry-btn, .cost-allowance-retry-btn, .planner-request-failure-retry-btn')) return;
88648868
msgEl.classList.add('retryable');
88658869
const btn = document.createElement('button');
88668870
btn.type = 'button';

src/firefox/src/ui/sidepanel.js

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3589,11 +3589,10 @@ async function adoptRestoredRunState(tabId, state) {
35893589
});
35903590
const returnedPlannerFailure = plannerRequestFailureUpdate(res?.updates);
35913591
if (returnedPlannerFailure && sameTabId(currentTabId, tabId) && !isTabAbortRequested(tabId)) {
3592-
const targetAssistantEl = assistantEl || currentAssistantEl;
35933592
renderPlannerRequestFailure(
3594-
targetAssistantEl,
3593+
assistantEl,
35953594
returnedPlannerFailure.data,
3596-
retryPayloadForRunAssistant(targetAssistantEl),
3595+
retryPayloadForRunAssistant(assistantEl),
35973596
);
35983597
}
35993598
const returnedErrorUpdate = Array.isArray(res?.updates)
@@ -5088,6 +5087,10 @@ function renderPlannerRequestFailure(assistantEl, data, retryPayload = null) {
50885087
assistantEl.classList.add('planner-request-failure');
50895088
textEl.classList.add('planner-request-failure-content');
50905089
textEl.replaceChildren();
5090+
// Set the role on the empty container first: screen readers announce
5091+
// content inserted into an existing live region, not a region that arrives
5092+
// already populated. A restored card stays silent, which is what we want.
5093+
textEl.setAttribute('role', 'alert');
50915094

50925095
const message = document.createElement('div');
50935096
message.className = 'planner-request-failure-message';
@@ -8506,7 +8509,8 @@ function configureRetryButton(btn, retryPayload) {
85068509
}
85078510

85088511
function addErrorRetryButton(msgEl, retryPayload) {
8509-
if (!msgEl || !retryPayload?.text || msgEl.querySelector('.error-retry-btn, .cost-allowance-retry-btn')) return;
8512+
if (!msgEl || !retryPayload?.text
8513+
|| msgEl.querySelector('.error-retry-btn, .cost-allowance-retry-btn, .planner-request-failure-retry-btn')) return;
85108514
msgEl.classList.add('retryable');
85118515
const btn = document.createElement('button');
85128516
btn.type = 'button';

test/run.js

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49737,6 +49737,16 @@ test('planner request failures expose provider settings and retry actions in bot
4973749737
/if \(data\.provider\) \{[\s\S]*?providerName\.className = 'planner-request-failure-provider';[\s\S]*?providerName\.textContent = data\.provider;/,
4973849738
`${label}: the failing provider is never named, so "open Providers" does not say which one to fix`,
4973949739
);
49740+
assert.match(
49741+
panel,
49742+
/textEl\.replaceChildren\(\);[\s\S]*?textEl\.setAttribute\('role', 'alert'\);[\s\S]*?const message = document\.createElement\('div'\);/,
49743+
`${label}: the failure card is not announced, and the role must precede its content to be spoken`,
49744+
);
49745+
assert.match(
49746+
panel,
49747+
/msgEl\.querySelector\('\.error-retry-btn, \.cost-allowance-retry-btn, \.planner-request-failure-retry-btn'\)/,
49748+
`${label}: a planner failure card can be given a second, duplicate Retry affordance`,
49749+
);
4974049750
assert.match(css, /\.planner-request-failure-actions \{/, `${label}: planner failure action row is not styled`);
4974149751
assert.match(css, /\.planner-request-failure-action\.primary \{/, `${label}: planner failure primary action is not styled`);
4974249752
assert.match(css, /\.planner-request-failure-provider \{/, `${label}: planner failure provider label is not styled`);

0 commit comments

Comments
 (0)