Skip to content

fix(mobile): adapt settings dialog and message controls for mobile screens - #919

Merged
nesquena-hermes merged 2 commits into
masterfrom
fix/915-mobile-settings
Apr 23, 2026
Merged

nesquena-hermes merged 2 commits into
masterfrom
fix/915-mobile-settings

Conversation

@nesquena-hermes

Copy link
Copy Markdown
Collaborator

Mobile usability fix for settings and chat. Clean rebase from PR #915 (@bsgdigital).

1. Settings dialog: replaces vertical tab strip with a native <select> dropdown on mobile viewports. Tab strip hidden on mobile, dropdown shown; both sync with switchSettingsSection().

2. Provider buttons: Save/Remove become icon-only on mobile (labels hidden via CSS) so the API key password input fills the remaining width.

3. Message controls: timestamps, copy, and edit buttons forced opacity: 1 on touch screens — these are permanently invisible without this since mobile has no hover state.

2003 tests passing.

Co-authored-by: bsgdigital

@nesquena nesquena left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review — end-to-end ✅ (clean, no fixes needed)

Traced against upstream hermes-agent

Fresh nousresearch/hermes-agent tarball pulled. Confirmed this PR is entirely webui-internal (HTML + CSS + panels.js only). No Python, no config.yaml, no session state, no AIAgent interaction. Cross-tool safety is trivial.

End-to-end trace

1. Settings dropdown replaces tab strip on mobile (static/index.html:449-455, static/style.css:1891-1908, static/panels.js:1291-1296)

  • New <select id="settingsSectionDropdown"> with five hardcoded <option> values matching the five internal section keys (conversation, appearance, preferences, providers, system)
  • switchSettingsSection(name) at panels.js:1291-1293 syncs the dropdown value after normalizing — bidirectional binding confirmed
  • switchSettingsSection was already dispatching on these exact keys (panels.js:1131 in existing code), so the new dropdown doesn't need a new dispatch path
  • CSS rules: .settings-section-dropdown hidden by default (line 1484), shown only in the @media(max-width:640px) block (line 1903); .settings-tabs hidden on mobile via the same block
  • .settings-shell flips from grid to flex-direction:column on mobile so the panel occupies full width ✓

2. Provider buttons go icon-only on mobile (static/panels.js:1593-1619, static/style.css:1508-1513)

  • Replaces inline-styled saveBtn.textContent=... with innerHTML building SVG + <span class="provider-btn-label">
  • t('providers_save') / t('providers_remove') — I verified all six locales in i18n.js have hardcoded values 'Save' and 'Remove' (no HTML-meaningful chars, no XSS vector via innerHTML concatenation)
  • Buttons get aria-label + title for accessibility when labels are hidden
  • CSS .provider-btn-label{} and mobile override .provider-btn-label{display:none} hide the text while preserving the icon
  • flex:none on both buttons + min-width:0 on the input = password input fills remaining width ✓

3. Message controls always visible on touch screens (static/style.css:1905-1908)

  • .msg-row[data-role="..."] .msg-foot and .msg-actions forced to opacity:1 inside @media(max-width:640px)
  • Matches existing mobile-first assumption: touch = no hover state, so hover-reveal controls need an explicit fallback

Cross-tool (CLI) check

All changes are client-side HTML/CSS/JS. CLI unaffected. ✓

Security audit

  • XSS risk via innerHTML (panels.js:1598, 1612): innerHTML='<svg>...</svg><span>'+t('providers_save')+'</span>'. Safe because t() returns hardcoded i18n strings. All six locales' providers_save/providers_remove values are plain 'Save'/'Remove' — no HTML-meaningful chars. Verified: grep -n "providers_save\|providers_remove" static/i18n.js shows only these two values across 6 locales. ✓
  • Dropdown onchange injection: onchange="switchSettingsSection(this.value)" where this.value is constrained to the five hardcoded <option value="..."> strings. No attacker-controlled input. ✓
  • No Python, no auth gate changes, no file-serving. ✓

Edge-case trace

Scenario Behaviour
Desktop (>640px) — Settings open Tab strip visible (flex-row), dropdown hidden (display:none) ✅
Mobile (≤640px) — Settings open Dropdown visible, tab strip hidden, panel full-width ✅
Mobile — switch section via tab strip (shouldn't be possible since it's hidden) N/A
Mobile — switch via dropdown onchange fires switchSettingsSection(this.value) → existing handler toggles panes + syncs dropdown back to same value (idempotent) ✅
Desktop — switch section via tab switchSettingsSection(name) toggles panes, also writes dd.value = section to an element that's display:none — harmless ✅
Mobile — provider buttons Icon-only, aria-label + title preserved for screen readers ✅
Mobile — very long API key in password input min-width:0; flex:1 on input + flex:none on buttons → input scrolls horizontally, buttons don't shrink ✅
Mobile — message timestamp/copy/edit controls All visible at opacity:1 regardless of hover ✅
Viewport exactly at 640px max-width:640px includes 640 → mobile rules apply. Matches existing breakpoint ✅

Tests

  • 24/24 pass in tests/test_mobile_layout.py (the 640px breakpoint + sidebar nav + right-panel slide-over + overlay markup assertions)
  • Full local suite: 1945 passed, 47 skipped, 0 failed
  • node --check static/panels.js — clean

Minor observations (non-blocking)

  • <option> text in the mobile dropdown is hardcoded English ("Conversation", "Appearance", etc.) without data-i18n attributes. The existing tab strip has the same inconsistency (only settingsTabProviders has data-i18n="providers_tab_title"; the others are plain English). Worth a sweep to add data-i18n keys across both the tab strip and dropdown for full localization, but this PR is consistent with the existing pattern — not a regression.
  • The new dropdown ships a native <select> look (browser-default styling). On some platforms this may contrast with the surrounding dark-themed settings UI. Acceptable for mobile where native controls are preferred for accessibility; a polished skin could come later.
  • settingsSectionDropdown writes dd.value even when the dropdown isn't visible. No functional issue, but a display:none guard would avoid the unnecessary work on desktop. Trivial.

Recommendation

Three mobile usability fixes, all correctly scoped, all pass the mobile-layout test suite. t()-based label indirection is safe via verified i18n hardcoded values. No cross-tool or security concerns. Nothing to push back on.

✅ Approved. Ready for merge + v0.50.177 tag.

@nesquena-hermes
nesquena-hermes merged commit 07caaec into master Apr 23, 2026
3 checks passed
@nesquena-hermes
nesquena-hermes deleted the fix/915-mobile-settings branch April 24, 2026 01:43
JKJameson pushed a commit to JKJameson/hermes-webui that referenced this pull request Apr 25, 2026
…reens (nesquena#919)

* fix(mobile): adapt settings dialog and message controls for mobile screens (nesquena#915)

Co-authored-by: bsgdigital

* fix(mobile): adapt settings dialog and message controls for mobile screens (v0.50.177, nesquena#915)

Co-authored-by: bsgdigital

---------

Co-authored-by: nesquena-hermes <nesquena-hermes@users.noreply.github.com>
SysAdminDoc pushed a commit to SysAdminDoc/hermes-webui that referenced this pull request Jun 26, 2026
…reens (nesquena#919)

* fix(mobile): adapt settings dialog and message controls for mobile screens (nesquena#915)

Co-authored-by: bsgdigital

* fix(mobile): adapt settings dialog and message controls for mobile screens (v0.50.177, nesquena#915)

Co-authored-by: bsgdigital

---------

Co-authored-by: nesquena-hermes <nesquena-hermes@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants