Skip to content

Commit d03b3de

Browse files
SurviveMSurviveM
committed
fix: harden deferred tab refresh and aria semantics
1 parent 24002e4 commit d03b3de

5 files changed

Lines changed: 108 additions & 79 deletions

File tree

‎.github/workflows/coderabbit-review.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ jobs:
2727
script: |
2828
const pr = context.payload.pull_request;
2929
const body = [
30-
"@coderabbitai If the current suggestion is not adopted, will it lead to disruptive changes? Is there any user experience regression?",
30+
"@coderabbitai review - If these changes are applied, will they cause disruptive behavior or user experience regressions?",
3131
].join("\n");
3232
3333
await github.rest.issues.createComment({

‎tests/unit/config-tabs-ui.test.mjs‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,9 @@ test('config template keeps expected config tabs in top and side navigation', ()
3333
assert.match(html, /data-config-mode=\"codex\"/);
3434
assert.match(html, /isMainTabNavActive\('settings'\)/);
3535
assert.match(html, /isConfigModeNavActive\('codex'\)/);
36+
assert.doesNotMatch(html, /:aria-pressed=/);
37+
assert.match(html, /:aria-selected="mainTab === 'sessions'"/);
38+
assert.match(html, /:aria-selected="mainTab === 'config' && configMode === 'codex'"/);
3639
assert.match(html, /v-memo="\[activeSessionExportKey === getSessionExportKey\(session\)/);
3740
assert.match(html, /v-memo="\[msg\.text,\s*msg\.timestamp,\s*msg\.roleLabel,\s*msg\.normalizedRole\]"/);
3841
assert.match(html, /v-memo="\[sessionTimelineActiveKey === node\.key,\s*node\.safePercent,\s*node\.title\]"/);
@@ -65,6 +68,7 @@ test('web ui script defines provider mode metadata for codex only', () => {
6568
assert.match(appScript, /onConfigTabPointerDown\(mode\)/);
6669
assert.match(appScript, /onMainTabClick\(tab\)/);
6770
assert.match(appScript, /onConfigTabClick\(mode\)/);
71+
assert.match(appScript, /if \(pointerType === 'touch'\) {/);
6872
assert.match(appScript, /node\.classList\.toggle\('nav-intent-active'/);
6973
assert.match(appScript, /node\.classList\.toggle\('nav-intent-inactive'/);
7074
assert.match(appScript, /node\.classList\.remove\('nav-intent-active'\)/);
@@ -81,6 +85,8 @@ test('web ui script defines provider mode metadata for codex only', () => {
8185
assert.match(appScript, /if \(this\.mainTab !== 'sessions' \|\| !this\.sessionPreviewRenderEnabled\) {/);
8286
assert.match(appScript, /const scrollRect = scrollEl && typeof scrollEl\.getBoundingClientRect === 'function'/);
8387
assert.match(appScript, /top = scrollTop \+ \(messageRect\.top - scrollRect\.top\);/);
88+
assert.match(appScript, /if \(!current \|\| current\.ticket !== this\.sessionTabRenderTicket\) {/);
89+
assert.match(appScript, /bindSessionMessageRef\(messageKey,\s*el,\s*ticket = this\.sessionTabRenderTicket\)/);
8490
assert.match(appScript, /this\.getMainTabForNav\(\) !== 'sessions'/);
8591
assert.match(appScript, /scheduleIdleTask\(task,\s*timeoutMs = 160\)/);
8692
assert.match(appScript, /scheduleSessionTabDeferredTeardown\(task\)/);
@@ -108,3 +114,10 @@ test('web ui script defines provider mode metadata for codex only', () => {
108114
assert.match(configModeComputed, /isProviderConfigMode\(\)/);
109115
assert.match(configModeComputed, /activeProviderModelPlaceholder\(\)/);
110116
});
117+
118+
test('session helper deferred claude refresh validates live tab and mode before running', () => {
119+
const helperScript = readProjectFile('web-ui/session-helpers.mjs');
120+
assert.match(helperScript, /const expectedTab = nextTab;/);
121+
assert.match(helperScript, /const expectedConfigMode = this\.configMode;/);
122+
assert.match(helperScript, /if \(this\.mainTab !== expectedTab \|\| this\.configMode !== expectedConfigMode\) return;/);
123+
});

‎web-ui/app.js‎

Lines changed: 71 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -903,19 +903,24 @@ import { createSkillsMethods } from './modules/skills.methods.mjs';
903903
localStorage.setItem(storageKey, '1');
904904
},
905905

906-
switchConfigMode(mode) {
907-
const normalizedMode = typeof mode === 'string'
908-
? mode.trim().toLowerCase()
909-
: '';
910-
this.configMode = CONFIG_MODE_SET.has(normalizedMode) ? normalizedMode : 'codex';
911-
if (this.mainTab === 'config') {
912-
if (this.configMode === 'claude') {
913-
const refresh = () => {
914-
this.refreshClaudeModelContext();
915-
};
916-
if (typeof this.scheduleAfterFrame === 'function') {
917-
this.scheduleAfterFrame(refresh);
918-
} else {
906+
switchConfigMode(mode) {
907+
const normalizedMode = typeof mode === 'string'
908+
? mode.trim().toLowerCase()
909+
: '';
910+
this.configMode = CONFIG_MODE_SET.has(normalizedMode) ? normalizedMode : 'codex';
911+
if (this.mainTab === 'config') {
912+
if (this.configMode === 'claude') {
913+
const expectedMainTab = 'config';
914+
const expectedConfigMode = 'claude';
915+
const refresh = () => {
916+
if (this.mainTab !== expectedMainTab || this.configMode !== expectedConfigMode) {
917+
return;
918+
}
919+
this.refreshClaudeModelContext();
920+
};
921+
if (typeof this.scheduleAfterFrame === 'function') {
922+
this.scheduleAfterFrame(refresh);
923+
} else {
919924
refresh();
920925
}
921926
}
@@ -1039,34 +1044,46 @@ import { createSkillsMethods } from './modules/skills.methods.mjs';
10391044
}
10401045
return (Date.now() - Number(state.at || 0)) <= 1000;
10411046
},
1042-
onMainTabPointerDown(tab) {
1043-
const event = arguments.length > 1 ? arguments[1] : null;
1044-
if (event && typeof event.button === 'number' && event.button !== 0) {
1045-
return;
1046-
}
1047+
onMainTabPointerDown(tab) {
1048+
const event = arguments.length > 1 ? arguments[1] : null;
1049+
if (event && typeof event.button === 'number' && event.button !== 0) {
1050+
return;
1051+
}
10471052
const normalizedTab = typeof tab === 'string' ? tab.trim().toLowerCase() : '';
10481053
if (!normalizedTab) return;
10491054
this.setMainTabSwitchIntent(normalizedTab);
1050-
this.applyImmediateNavIntent(normalizedTab);
1051-
const shouldHideSessionPanel = this.mainTab === 'sessions' && normalizedTab !== 'sessions';
1052-
this.setSessionPanelFastHidden(shouldHideSessionPanel);
1053-
this.recordPointerNavCommit('main', normalizedTab);
1054-
this.switchMainTab(normalizedTab);
1055-
},
1056-
onConfigTabPointerDown(mode) {
1057-
const event = arguments.length > 1 ? arguments[1] : null;
1055+
this.applyImmediateNavIntent(normalizedTab);
1056+
const shouldHideSessionPanel = this.mainTab === 'sessions' && normalizedTab !== 'sessions';
1057+
this.setSessionPanelFastHidden(shouldHideSessionPanel);
1058+
const pointerType = event && typeof event.pointerType === 'string'
1059+
? event.pointerType.trim().toLowerCase()
1060+
: '';
1061+
if (pointerType === 'touch') {
1062+
return;
1063+
}
1064+
this.recordPointerNavCommit('main', normalizedTab);
1065+
this.switchMainTab(normalizedTab);
1066+
},
1067+
onConfigTabPointerDown(mode) {
1068+
const event = arguments.length > 1 ? arguments[1] : null;
10581069
if (event && typeof event.button === 'number' && event.button !== 0) {
10591070
return;
10601071
}
10611072
const normalizedMode = typeof mode === 'string' ? mode.trim().toLowerCase() : '';
10621073
if (!normalizedMode) return;
1063-
this.setMainTabSwitchIntent('config');
1064-
this.applyImmediateNavIntent('config', normalizedMode);
1065-
const shouldHideSessionPanel = this.mainTab === 'sessions';
1066-
this.setSessionPanelFastHidden(shouldHideSessionPanel);
1067-
this.recordPointerNavCommit('config', normalizedMode);
1068-
this.switchConfigMode(normalizedMode);
1069-
},
1074+
this.setMainTabSwitchIntent('config');
1075+
this.applyImmediateNavIntent('config', normalizedMode);
1076+
const shouldHideSessionPanel = this.mainTab === 'sessions';
1077+
this.setSessionPanelFastHidden(shouldHideSessionPanel);
1078+
const pointerType = event && typeof event.pointerType === 'string'
1079+
? event.pointerType.trim().toLowerCase()
1080+
: '';
1081+
if (pointerType === 'touch') {
1082+
return;
1083+
}
1084+
this.recordPointerNavCommit('config', normalizedMode);
1085+
this.switchConfigMode(normalizedMode);
1086+
},
10701087
onMainTabClick(tab) {
10711088
const normalizedTab = typeof tab === 'string' ? tab.trim().toLowerCase() : '';
10721089
if (!normalizedTab) return;
@@ -2027,15 +2044,20 @@ import { createSkillsMethods } from './modules/skills.methods.mjs';
20272044
}
20282045
return refreshedNodes;
20292046
},
2030-
getSessionMessageRefBinder(messageKey) {
2031-
if (!this.isSessionTimelineNodeKey(messageKey)) return null;
2032-
if (!this.sessionMessageRefBinderMap[messageKey]) {
2033-
this.sessionMessageRefBinderMap[messageKey] = (el) => {
2034-
this.bindSessionMessageRef(messageKey, el);
2035-
};
2036-
}
2037-
return this.sessionMessageRefBinderMap[messageKey];
2038-
},
2047+
getSessionMessageRefBinder(messageKey) {
2048+
if (!this.isSessionTimelineNodeKey(messageKey)) return null;
2049+
const current = this.sessionMessageRefBinderMap[messageKey];
2050+
if (!current || current.ticket !== this.sessionTabRenderTicket) {
2051+
const ticket = this.sessionTabRenderTicket;
2052+
this.sessionMessageRefBinderMap[messageKey] = {
2053+
ticket,
2054+
bind: (el) => {
2055+
this.bindSessionMessageRef(messageKey, el, ticket);
2056+
}
2057+
};
2058+
}
2059+
return this.sessionMessageRefBinderMap[messageKey].bind;
2060+
},
20392061
updateSessionTimelineOffset() {
20402062
const container = this.sessionPreviewContainerEl || this.$refs.sessionPreviewContainer;
20412063
if (!container || !container.style) return;
@@ -2046,12 +2068,13 @@ import { createSkillsMethods } from './modules/skills.methods.mjs';
20462068
const offset = headerHeight > 0 ? (headerHeight + 12) : 72;
20472069
container.style.setProperty('--session-preview-header-offset', `${offset}px`);
20482070
},
2049-
bindSessionMessageRef(messageKey, el) {
2050-
if (!this.sessionTimelineEnabled) return;
2051-
if (!messageKey) return;
2052-
if (el) {
2053-
if (!this.isSessionTimelineNodeKey(messageKey)) return;
2054-
if (this.sessionMessageRefMap[messageKey] === el) return;
2071+
bindSessionMessageRef(messageKey, el, ticket = this.sessionTabRenderTicket) {
2072+
if (!this.sessionTimelineEnabled) return;
2073+
if (!messageKey) return;
2074+
if (ticket !== this.sessionTabRenderTicket) return;
2075+
if (el) {
2076+
if (!this.isSessionTimelineNodeKey(messageKey)) return;
2077+
if (this.sessionMessageRefMap[messageKey] === el) return;
20552078
this.sessionMessageRefMap[messageKey] = el;
20562079
this.invalidateSessionTimelineMeasurementCache();
20572080
} else {

‎web-ui/index.html‎

Lines changed: 20 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -58,9 +58,8 @@
5858
role="tab"
5959
data-main-tab="config"
6060
data-config-mode="codex"
61-
:tabindex="isConfigModeNavActive('codex') ? 0 : -1"
62-
:aria-selected="isConfigModeNavActive('codex')"
63-
:aria-pressed="isConfigModeNavActive('codex')"
61+
:tabindex="mainTab === 'config' && configMode === 'codex' ? 0 : -1"
62+
:aria-selected="mainTab === 'config' && configMode === 'codex'"
6463
aria-controls="panel-config-provider"
6564
:class="{ active: isConfigModeNavActive('codex') }"
6665
@pointerdown="onConfigTabPointerDown('codex', $event)"
@@ -70,9 +69,8 @@
7069
role="tab"
7170
data-main-tab="config"
7271
data-config-mode="claude"
73-
:tabindex="isConfigModeNavActive('claude') ? 0 : -1"
74-
:aria-selected="isConfigModeNavActive('claude')"
75-
:aria-pressed="isConfigModeNavActive('claude')"
72+
:tabindex="mainTab === 'config' && configMode === 'claude' ? 0 : -1"
73+
:aria-selected="mainTab === 'config' && configMode === 'claude'"
7674
aria-controls="panel-config-claude"
7775
:class="{ active: isConfigModeNavActive('claude') }"
7876
@pointerdown="onConfigTabPointerDown('claude', $event)"
@@ -82,9 +80,8 @@
8280
role="tab"
8381
data-main-tab="config"
8482
data-config-mode="openclaw"
85-
:tabindex="isConfigModeNavActive('openclaw') ? 0 : -1"
86-
:aria-selected="isConfigModeNavActive('openclaw')"
87-
:aria-pressed="isConfigModeNavActive('openclaw')"
83+
:tabindex="mainTab === 'config' && configMode === 'openclaw' ? 0 : -1"
84+
:aria-selected="mainTab === 'config' && configMode === 'openclaw'"
8885
aria-controls="panel-config-openclaw"
8986
:class="{ active: isConfigModeNavActive('openclaw') }"
9087
@pointerdown="onConfigTabPointerDown('openclaw', $event)"
@@ -93,9 +90,8 @@
9390
id="tab-sessions"
9491
role="tab"
9592
data-main-tab="sessions"
96-
:tabindex="isMainTabNavActive('sessions') ? 0 : -1"
97-
:aria-selected="isMainTabNavActive('sessions')"
98-
:aria-pressed="isMainTabNavActive('sessions')"
93+
:tabindex="mainTab === 'sessions' ? 0 : -1"
94+
:aria-selected="mainTab === 'sessions'"
9995
aria-controls="panel-sessions"
10096
:class="{ active: isMainTabNavActive('sessions') }"
10197
@pointerdown="onMainTabPointerDown('sessions', $event)"
@@ -104,9 +100,8 @@
104100
id="tab-settings"
105101
role="tab"
106102
data-main-tab="settings"
107-
:tabindex="isMainTabNavActive('settings') ? 0 : -1"
108-
:aria-selected="isMainTabNavActive('settings')"
109-
:aria-pressed="isMainTabNavActive('settings')"
103+
:tabindex="mainTab === 'settings' ? 0 : -1"
104+
:aria-selected="mainTab === 'settings'"
110105
aria-controls="panel-settings"
111106
:class="{ active: isMainTabNavActive('settings') }"
112107
@pointerdown="onMainTabPointerDown('settings', $event)"
@@ -153,9 +148,8 @@
153148
data-main-tab="config"
154149
data-config-mode="codex"
155150
aria-controls="panel-config-provider"
156-
:tabindex="isConfigModeNavActive('codex') ? 0 : -1"
157-
:aria-selected="isConfigModeNavActive('codex')"
158-
:aria-pressed="isConfigModeNavActive('codex')"
151+
:tabindex="mainTab === 'config' && configMode === 'codex' ? 0 : -1"
152+
:aria-selected="mainTab === 'config' && configMode === 'codex'"
159153
:class="['side-item', { active: isConfigModeNavActive('codex') }]"
160154
@pointerdown="onConfigTabPointerDown('codex', $event)"
161155
@click="onConfigTabClick('codex', $event)">
@@ -171,9 +165,8 @@
171165
data-main-tab="config"
172166
data-config-mode="claude"
173167
aria-controls="panel-config-claude"
174-
:tabindex="isConfigModeNavActive('claude') ? 0 : -1"
175-
:aria-selected="isConfigModeNavActive('claude')"
176-
:aria-pressed="isConfigModeNavActive('claude')"
168+
:tabindex="mainTab === 'config' && configMode === 'claude' ? 0 : -1"
169+
:aria-selected="mainTab === 'config' && configMode === 'claude'"
177170
:class="['side-item', { active: isConfigModeNavActive('claude') }]"
178171
@pointerdown="onConfigTabPointerDown('claude', $event)"
179172
@click="onConfigTabClick('claude', $event)">
@@ -189,9 +182,8 @@
189182
data-main-tab="config"
190183
data-config-mode="openclaw"
191184
aria-controls="panel-config-openclaw"
192-
:tabindex="isConfigModeNavActive('openclaw') ? 0 : -1"
193-
:aria-selected="isConfigModeNavActive('openclaw')"
194-
:aria-pressed="isConfigModeNavActive('openclaw')"
185+
:tabindex="mainTab === 'config' && configMode === 'openclaw' ? 0 : -1"
186+
:aria-selected="mainTab === 'config' && configMode === 'openclaw'"
195187
:class="['side-item', { active: isConfigModeNavActive('openclaw') }]"
196188
@pointerdown="onConfigTabPointerDown('openclaw', $event)"
197189
@click="onConfigTabClick('openclaw', $event)">
@@ -210,9 +202,8 @@
210202
id="side-tab-sessions"
211203
data-main-tab="sessions"
212204
aria-controls="panel-sessions"
213-
:tabindex="isMainTabNavActive('sessions') ? 0 : -1"
214-
:aria-selected="isMainTabNavActive('sessions')"
215-
:aria-pressed="isMainTabNavActive('sessions')"
205+
:tabindex="mainTab === 'sessions' ? 0 : -1"
206+
:aria-selected="mainTab === 'sessions'"
216207
:class="['side-item', { active: isMainTabNavActive('sessions') }]"
217208
@pointerdown="onMainTabPointerDown('sessions', $event)"
218209
@click="onMainTabClick('sessions', $event)">
@@ -231,9 +222,8 @@
231222
id="side-tab-settings"
232223
data-main-tab="settings"
233224
aria-controls="panel-settings"
234-
:tabindex="isMainTabNavActive('settings') ? 0 : -1"
235-
:aria-selected="isMainTabNavActive('settings')"
236-
:aria-pressed="isMainTabNavActive('settings')"
225+
:tabindex="mainTab === 'settings' ? 0 : -1"
226+
:aria-selected="mainTab === 'settings'"
237227
:class="['side-item', { active: isMainTabNavActive('settings') }]"
238228
@pointerdown="onMainTabPointerDown('settings', $event)"
239229
@click="onMainTabClick('settings', $event)">

0 commit comments

Comments
 (0)