fix: address PR review feedback on sidebar tab visibility
Three tweaks from reviewer: 1. Harden _applyTabVisibility to skip always-visible panels even if they appear in hidden_tabs (localStorage tampering, stale server data). Forces shouldHide=false so stale nav-tab-hidden classes on chat/settings get removed, not just skipped. 2. Add synchronous inline <script> flash-prevention after sidebar-nav in index.html. On slow networks, defer scripts run after the browser incrementally renders the DOM, causing hidden tabs to flash visible before JS executes. The inline script reads hermes-webui-hidden-tabs from localStorage and applies nav-tab-hidden classes before first paint, mirroring the existing theme/skin/font-size pattern. The boot.js IIFE becomes a secondary fallback (comment updated). 3. Remove _settingsHiddenTabsOnOpen dead state. It was tracked but never read for revert — _revertSettingsPreview is intentionally a no-op for appearance autosave. Removing the tracking makes the code honest about what it actually does. Also removes the test_settings_session_tracking test that validated this dead code.
This commit is contained in:
parent
7f1feca3fe
commit
5491a54285
4 changed files with 13 additions and 22 deletions
|
|
@ -284,9 +284,11 @@ function expandSidebar(){
|
|||
_syncSidebarAria();
|
||||
})();
|
||||
// ── Boot-time tab visibility ────────────────────────────────────────────────
|
||||
// Apply hidden tabs from localStorage before first paint. If the active tab
|
||||
// is hidden, switch focus to chat. Runs after DOM is ready (unlike theme/skin
|
||||
// flash-prevention, tab elements aren't available in <head>).
|
||||
// Apply hidden tabs from localStorage. The primary flash-prevention is an
|
||||
// inline <script> in index.html (after sidebar-nav) that runs synchronously
|
||||
// before first paint. This IIFE is a secondary fallback: it ensures consistency
|
||||
// after panels.js is loaded and handles the active-tab switch. No-op if
|
||||
// panels.js hasn't loaded yet (typeof guard).
|
||||
(function _restoreTabVisibility(){
|
||||
try{
|
||||
if(typeof _applyTabVisibility==='function'&&typeof _getHiddenTabs==='function'){
|
||||
|
|
|
|||
|
|
@ -178,6 +178,11 @@
|
|||
<!-- Settings button mirrored here for mobile (rail is desktop-only via @media >=768px). Keep in sync with rail entry. -->
|
||||
<button class="nav-tab has-tooltip has-tooltip--bottom" data-panel="settings" onclick="switchPanel('settings',{fromRailClick:true})" data-tooltip="Settings" data-i18n-title="tab_settings"><svg width="18" height="18" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" aria-hidden="true"><circle cx="12" cy="12" r="3"/><path d="M19.4 15a1.65 1.65 0 0 0 .33 1.82l.06.06a2 2 0 0 1-2.83 2.83l-.06-.06a1.65 1.65 0 0 0-1.82-.33 1.65 1.65 0 0 0-1 1.51V21a2 2 0 0 1-4 0v-.09A1.65 1.65 0 0 0 9 19.4a1.65 1.65 0 0 0-1.82.33l-.06.06a2 2 0 0 1-2.83-2.83l.06-.06a1.65 1.65 0 0 0 .33-1.82 1.65 1.65 0 0 0-1.51-1H3a2 2 0 0 1 0-4h.09A1.65 1.65 0 0 0 4.6 9a1.65 1.65 0 0 0-.33-1.82l-.06-.06a2 2 0 0 1 2.83-2.83l.06.06a1.65 1.65 0 0 0 1.82.33H9a1.65 1.65 0 0 0 1-1.51V3a2 2 0 0 1 4 0v.09a1.65 1.65 0 0 0 1 1.51 1.65 1.65 0 0 0 1.82-.33l.06-.06a2 2 0 0 1 2.83 2.83l-.06.06a1.65 1.65 0 0 0-.33 1.82V9a1.65 1.65 0 0 0 1.51 1H21a2 2 0 0 1 0 4h-.09a1.65 1.65 0 0 0-1.51 1z"/></svg></button>
|
||||
</div>
|
||||
<!-- Flash-prevention: apply hidden tab visibility from localStorage synchronously
|
||||
before any paint. Mirrors the theme/skin/font-size inline scripts in <head>.
|
||||
The boot.js _restoreTabVisibility IIFE is a secondary fallback that also
|
||||
handles the active-tab switch. -->
|
||||
<script>(function(){try{var h=localStorage.getItem('hermes-webui-hidden-tabs');if(!h)return;var p=JSON.parse(h);if(!Array.isArray(p))return;var a=new Set(['chat','settings']);p.forEach(function(id){if(a.has(id))return;document.querySelectorAll('[data-panel="'+id+'"]').forEach(function(el){el.classList.add('nav-tab-hidden');});});try{var ac=document.querySelector('.rail .rail-btn.nav-tab.active[data-panel]')||document.querySelector('.sidebar-nav .nav-tab.active[data-panel]');if(ac&&ac.classList.contains('nav-tab-hidden')){ac.classList.remove('active');var c=document.querySelector('.rail .rail-btn.nav-tab[data-panel="chat"]')||document.querySelector('.sidebar-nav .nav-tab[data-panel="chat"]');if(c)c.classList.add('active');}}catch(_){}}catch(e){}})();</script>
|
||||
<!-- Chat panel -->
|
||||
<div class="panel-view active" id="panelChat">
|
||||
<div class="panel-head">
|
||||
|
|
|
|||
|
|
@ -152,7 +152,6 @@ function _beginSettingsPanelSession() {
|
|||
_settingsThemeOnOpen = localStorage.getItem('hermes-theme') || 'dark';
|
||||
_settingsSkinOnOpen = localStorage.getItem('hermes-skin') || 'default';
|
||||
_settingsFontSizeOnOpen = localStorage.getItem('hermes-font-size') || 'default';
|
||||
_settingsHiddenTabsOnOpen = _getHiddenTabs().slice();
|
||||
_pendingSettingsTargetPanel = null;
|
||||
if (_settingsAppearanceAutosaveTimer) {
|
||||
clearTimeout(_settingsAppearanceAutosaveTimer);
|
||||
|
|
@ -5110,7 +5109,6 @@ let _settingsHermesDefaultModelOnOpen = '';
|
|||
let _settingsSection = 'conversation';
|
||||
let _currentSettingsSection = 'conversation';
|
||||
let _settingsAppearanceAutosaveTimer = null;
|
||||
let _settingsHiddenTabsOnOpen = null; // track hidden_tabs at open time for discard revert
|
||||
let _settingsAppearanceAutosaveRetryPayload = null;
|
||||
let _settingsPreferencesAutosaveTimer = null;
|
||||
let _settingsPreferencesAutosaveRetryPayload = null;
|
||||
|
|
@ -5135,6 +5133,8 @@ function _applyTabVisibility(hidden){
|
|||
var panel=el.dataset.panel;
|
||||
if(!panel)return;
|
||||
var shouldHide=hidden.indexOf(panel)!==-1;
|
||||
// Never hide always-visible panels (chat, settings) even if present in hidden_tabs
|
||||
if(_ALWAYS_VISIBLE_TABS.has(panel)) shouldHide=false;
|
||||
el.classList.toggle('nav-tab-hidden',shouldHide);
|
||||
});
|
||||
// If the currently active tab is hidden, switch to chat
|
||||
|
|
@ -5342,7 +5342,6 @@ function _rememberAppearanceSaved(payload){
|
|||
_settingsThemeOnOpen=payload.theme||localStorage.getItem('hermes-theme')||'dark';
|
||||
_settingsSkinOnOpen=payload.skin||localStorage.getItem('hermes-skin')||'default';
|
||||
_settingsFontSizeOnOpen=payload.font_size||localStorage.getItem('hermes-font-size')||'default';
|
||||
_settingsHiddenTabsOnOpen=Array.isArray(payload.hidden_tabs)?payload.hidden_tabs:[];
|
||||
}
|
||||
|
||||
function _scheduleAppearanceAutosave(){
|
||||
|
|
@ -6477,7 +6476,6 @@ function _applySavedSettingsUi(saved, body, opts){
|
|||
_settingsHermesDefaultModelOnOpen=body.default_model||_settingsHermesDefaultModelOnOpen||'';
|
||||
// Sync window._defaultModel so newSession() uses the just-saved default without a reload (#908).
|
||||
if(body.default_model) window._defaultModel=body.default_model;
|
||||
_settingsHiddenTabsOnOpen=_getHiddenTabs().slice();
|
||||
if(typeof clearMessageRenderCache==='function') clearMessageRenderCache();
|
||||
renderMessages();
|
||||
if(typeof syncTopbar==='function') syncTopbar();
|
||||
|
|
|
|||
|
|
@ -102,18 +102,4 @@ def test_i18n_coverage():
|
|||
assert label_count >= 11, f"Expected ≥11 locales, found {label_count}"
|
||||
assert desc_count >= 11, f"Expected ≥11 locales, found {desc_count}"
|
||||
assert label_count == desc_count, \
|
||||
f"Label ({label_count}) and desc ({desc_count}) counts must match"
|
||||
|
||||
|
||||
def test_settings_session_tracking():
|
||||
"""Settings open/close lifecycle must track hidden_tabs for discard revert."""
|
||||
# _beginSettingsPanelSession snapshots hidden_tabs
|
||||
begin_block = PANELS_JS[PANELS_JS.find("function _beginSettingsPanelSession"):]
|
||||
begin_body = begin_block[:begin_block.find("\nfunction ", 1) or 2000]
|
||||
assert "_settingsHiddenTabsOnOpen" in begin_body
|
||||
|
||||
# _applySavedSettingsUi updates the baseline on full save
|
||||
apply_block = PANELS_JS[PANELS_JS.find("function _applySavedSettingsUi"):]
|
||||
apply_body = apply_block[:apply_block.find("\nfunction ", 1) or 5000]
|
||||
assert "_settingsHiddenTabsOnOpen" in apply_body, \
|
||||
"_applySavedSettingsUi must update _settingsHiddenTabsOnOpen after full save"
|
||||
f"Label ({label_count}) and desc ({desc_count}) counts must match"
|
||||
Loading…
Reference in a new issue