Skip to content

Commit 0fc0594

Browse files
committed
fix(ui): close model picker and overflow via real dismiss helpers
closeAllPopups and Escape looked for a non-existent .open class on the model picker and overflow menus (they use .hidden/.closing). Opening overflow only set visibility:hidden on the picker, leaving zombie open state. Export closeModelPicker, register it on the Escape menu stack (so Escape works while #message is focused), and route dismiss through the real close helpers. Co-authored-by: Arda Tuğsat <arda-tugsat@users.noreply.github.com>
1 parent a35384e commit 0fc0594

2 files changed

Lines changed: 127 additions & 30 deletions

File tree

static/app.js

Lines changed: 32 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -398,9 +398,22 @@ function initializeEventListeners() {
398398
// windows are deliberately NOT touched here — those close via their own
399399
// controls.
400400
window.closeAllPopups = function closeAllPopups(except) {
401+
// These menus use the `.open` class.
401402
document.querySelectorAll(
402-
'.export-dropdown-menu.open, .overflow-menu.open, .model-picker-menu.open, .doc-overflow-menu.open'
403+
'.export-dropdown-menu.open, .doc-overflow-menu.open'
403404
).forEach(m => { if (m !== except) m.classList.remove('open'); });
405+
// Overflow + model picker use `.hidden` / `.closing` — never `.open`.
406+
// Calling the real close helpers plays their fold-in animations and
407+
// clears zombie open state (visibility:hidden alone used to leave the
408+
// picker "open" under the overflow menu).
409+
const overflowMenu = document.getElementById('overflow-menu');
410+
if (overflowMenu && overflowMenu !== except && typeof window.closeOverflowMenu === 'function') {
411+
window.closeOverflowMenu();
412+
}
413+
const modelPickerMenu = document.getElementById('model-picker-menu');
414+
if (modelPickerMenu && modelPickerMenu !== except && typeof window.closeModelPicker === 'function') {
415+
window.closeModelPicker();
416+
}
404417
document.querySelectorAll(
405418
'.skill-kebab-menu, .note-reminder-menu, .task-dropdown, .doclib-card-dropdown, .email-card-dropdown, .msg-overflow-menu'
406419
).forEach(m => { if (m !== except) m.remove(); });
@@ -714,10 +727,21 @@ function initializeEventListeners() {
714727
return;
715728
}
716729

717-
// Model picker popup — close before opening any modals
730+
// Model picker popup — uses .hidden/.closing, not .open. Prefer the
731+
// real close helper (also registered on the Escape menu stack so this
732+
// path is usually already handled when focus is in #message).
718733
const modelPickerMenu = document.getElementById('model-picker-menu');
719-
if (modelPickerMenu && modelPickerMenu.classList.contains('open')) {
720-
modelPickerMenu.classList.remove('open');
734+
if (modelPickerMenu && !modelPickerMenu.classList.contains('hidden')) {
735+
if (typeof window.closeModelPicker === 'function') window.closeModelPicker();
736+
else {
737+
modelPickerMenu.classList.add('hidden');
738+
modelPickerMenu.classList.remove('closing');
739+
}
740+
return;
741+
}
742+
const overflowMenuEsc = document.getElementById('overflow-menu');
743+
if (overflowMenuEsc && !overflowMenuEsc.classList.contains('hidden') && typeof window.closeOverflowMenu === 'function') {
744+
window.closeOverflowMenu();
721745
return;
722746
}
723747

@@ -2055,8 +2079,9 @@ function initializeEventListeners() {
20552079
menu.classList.remove('hidden');
20562080
plusBtn.classList.add('expanded');
20572081
document.body.appendChild(menu); // escape the composer's container-type trap
2058-
// Hide pill bar label so it doesn't show through the menu
2059-
if (pickerWrap) pickerWrap.style.visibility = 'hidden';
2082+
// Actually close the model picker (don't just hide it with visibility —
2083+
// that left a zombie open state when overflow closed).
2084+
if (typeof window.closeModelPicker === 'function') window.closeModelPicker();
20602085
// Keep the textarea focused so the keyboard stays up if it was open (the
20612086
// pointerdown handler above prevents the focus-steal). Still watch
20622087
// visualViewport so the menu follows the chevron if the viewport shifts.
@@ -2079,7 +2104,6 @@ function initializeEventListeners() {
20792104
// scales back into the chevron) before flipping to display:none.
20802105
menu.classList.add('closing');
20812106
plusBtn.classList.remove('expanded');
2082-
if (pickerWrap) pickerWrap.style.visibility = '';
20832107
// Item delays max at 0.18s + 0.20s anim = 0.38s for items, container
20842108
// delay 0.16s + 0.22s = 0.38s. 400ms covers both with margin.
20852109
setTimeout(() => {
@@ -2088,6 +2112,7 @@ function initializeEventListeners() {
20882112
if (ownerWrap) ownerWrap.appendChild(menu); // restore from <body> portal
20892113
}, 400);
20902114
}
2115+
window.closeOverflowMenu = closeOverflowMenu;
20912116
// Close menu when clicking any item inside it. preventDefault on pointerdown
20922117
// so tapping an item (e.g. Attach files) doesn't steal focus from the message
20932118
// box — keeps the mobile keyboard up.

static/js/modelPicker.js

Lines changed: 95 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,21 @@ import { providerLogo } from './providers.js';
55
import uiModule from './ui.js';
66
import settingsModule from './settings.js';
77
import { sortModelObjects } from './modelSort.js';
8+
import { registerMenuDismiss } from './escMenuStack.js';
89

910
const API_BASE = window.location.origin;
1011

12+
// Real close path for the picker (uses .hidden/.closing, not .open).
13+
// Stashed so closeAllPopups / Escape / overflow can dismiss without poking classes.
14+
let _closePickerFn = null;
15+
let _unregisterPickerEsc = () => {};
16+
17+
/** Close the model picker if open. Safe to call when closed or before init. */
18+
export function closeModelPicker() {
19+
if (typeof _closePickerFn === 'function') _closePickerFn();
20+
}
21+
if (typeof window !== 'undefined') window.closeModelPicker = closeModelPicker;
22+
1123
// ── Recent + Favorites persistence ──
1224
// Recent is auto-tracked (last 5 picks, most-recent-first) and lives in its
1325
// own key. Favorites is the SAME key the sidebar Models section uses, so a
@@ -181,28 +193,102 @@ function _initModelPickerDropdown() {
181193
const searchRow = menu ? menu.querySelector('.model-picker-search-row') : null;
182194
const refreshBtn = document.getElementById('model-picker-refresh-btn');
183195
if (!wrap || !btn || !menu || !search || !listEl) return;
196+
// Guard against double-wiring when sessions.js is loaded twice under
197+
// different module URLs (bare vs ?v=) — second handler would open then close.
198+
if (btn.dataset.modelPickerWired === '1') return;
199+
btn.dataset.modelPickerWired = '1';
200+
201+
let _closeTimer = null;
202+
let _ignoreCloseUntil = 0;
203+
let _docClickHandler = null;
204+
let _onCloseAnimEnd = null;
205+
206+
function _detachDocClick() {
207+
if (!_docClickHandler) return;
208+
document.removeEventListener('click', _docClickHandler, true);
209+
_docClickHandler = null;
210+
}
211+
212+
function _cancelCloseAnim() {
213+
if (_onCloseAnimEnd) {
214+
menu.removeEventListener('animationend', _onCloseAnimEnd);
215+
_onCloseAnimEnd = null;
216+
}
217+
if (_closeTimer) {
218+
clearTimeout(_closeTimer);
219+
_closeTimer = null;
220+
}
221+
}
184222

185223
function _close() {
186224
if (menu.classList.contains('hidden')) return;
225+
_unregisterPickerEsc();
226+
_unregisterPickerEsc = () => {};
227+
_detachDocClick();
187228
// Restore scroll button
188229
const _scrollBtn = document.getElementById('scroll-bottom-btn');
189230
if (_scrollBtn) _scrollBtn.style.display = '';
231+
_cancelCloseAnim();
190232
menu.classList.add('closing');
191-
menu.addEventListener('animationend', function _onDone() {
233+
_onCloseAnimEnd = function _onDone() {
192234
menu.removeEventListener('animationend', _onDone);
235+
_onCloseAnimEnd = null;
193236
menu.classList.remove('closing');
194237
menu.classList.add('hidden');
195238
search.value = '';
196-
}, { once: true });
197-
// Fallback if animationend doesn't fire
198-
setTimeout(() => {
239+
};
240+
menu.addEventListener('animationend', _onCloseAnimEnd);
241+
// Fallback if animationend doesn't fire — keep id so _open() can cancel
242+
_closeTimer = setTimeout(() => {
243+
_closeTimer = null;
244+
if (_onCloseAnimEnd) {
245+
menu.removeEventListener('animationend', _onCloseAnimEnd);
246+
_onCloseAnimEnd = null;
247+
}
199248
if (!menu.classList.contains('hidden')) {
200249
menu.classList.remove('closing');
201250
menu.classList.add('hidden');
202251
search.value = '';
203252
}
204253
}, 200);
205254
}
255+
_closePickerFn = _close;
256+
257+
function _open() {
258+
// Cancel any in-flight close so a stale animationend/timer cannot re-hide us
259+
_cancelCloseAnim();
260+
menu.classList.remove('closing', 'hidden');
261+
// Block toggle/outside close briefly (mobile ghost-click + same-turn doc click)
262+
_ignoreCloseUntil = Date.now() + 350;
263+
_populate('');
264+
if (window.modelsModule && window.modelsModule.refreshModels) {
265+
window.modelsModule.refreshModels().then(() => {
266+
if (!menu.classList.contains('hidden')) _populate(search.value || '');
267+
updateModelPicker();
268+
}).catch(() => {});
269+
}
270+
if (window.innerWidth >= 768) search.focus();
271+
const _scrollBtn = document.getElementById('scroll-bottom-btn');
272+
if (_scrollBtn) _scrollBtn.style.display = 'none';
273+
274+
// Register with Escape stack BEFORE the textarea guard in ui.js — so Escape
275+
// closes the picker even when focus stays on #message (mobile keyboard up).
276+
_unregisterPickerEsc();
277+
_unregisterPickerEsc = registerMenuDismiss(() => { _close(); });
278+
279+
// Defer outside-click (capture) so the opening click cannot immediately dismiss
280+
_detachDocClick();
281+
_docClickHandler = (e) => {
282+
if (Date.now() < _ignoreCloseUntil) return;
283+
if (menu.classList.contains('hidden')) return;
284+
if (!wrap.contains(e.target)) _close();
285+
};
286+
setTimeout(() => {
287+
if (_docClickHandler && !menu.classList.contains('hidden')) {
288+
document.addEventListener('click', _docClickHandler, true);
289+
}
290+
}, 0);
291+
}
206292

207293
function _openPickerShortcut(kind) {
208294
_close();
@@ -650,23 +736,14 @@ function _initModelPickerDropdown() {
650736
if (match) await _pick(match);
651737
});
652738

739+
// Keep composer focus (mobile keyboard) — same pattern as overflow + button
740+
btn.addEventListener('pointerdown', (e) => { e.preventDefault(); });
653741
btn.addEventListener('click', (e) => {
742+
e.preventDefault();
654743
e.stopPropagation();
655744
if (menu.classList.contains('hidden') || menu.classList.contains('closing')) {
656-
// Force-clear any in-progress close animation
657-
menu.classList.remove('closing', 'hidden');
658-
_populate('');
659-
if (window.modelsModule && window.modelsModule.refreshModels) {
660-
window.modelsModule.refreshModels().then(() => {
661-
if (!menu.classList.contains('hidden')) _populate(search.value || '');
662-
updateModelPicker();
663-
}).catch(() => {});
664-
}
665-
if (window.innerWidth >= 768) search.focus();
666-
// Hide scroll button so it doesn't overlap
667-
const _scrollBtn = document.getElementById('scroll-bottom-btn');
668-
if (_scrollBtn) _scrollBtn.style.display = 'none';
669-
} else {
745+
_open();
746+
} else if (Date.now() >= _ignoreCloseUntil) {
670747
_close();
671748
}
672749
});
@@ -703,11 +780,6 @@ function _initModelPickerDropdown() {
703780
_openPickerShortcut('models');
704781
});
705782
}
706-
document.addEventListener('click', (e) => {
707-
if (!menu.classList.contains('hidden') && !menu.contains(e.target) && e.target !== btn) {
708-
_close();
709-
}
710-
});
711783
}
712784

713785
/**

0 commit comments

Comments
 (0)