Skip to content

fix(ui): resolve verified stale-state QA findings - #1909

Open
JoeJoeflyn wants to merge 8 commits into
mainfrom
fix/1862-stale-ui-qa-findings
Open

JoeJoeflyn wants to merge 8 commits into
mainfrom
fix/1862-stale-ui-qa-findings

Conversation

@JoeJoeflyn

@JoeJoeflyn JoeJoeflyn commented Oct 27, 2026 •

Copy link
Copy Markdown
Collaborator

Description

Resolve all eight findings from #1862 by reusing existing watch, listener, bookmark, and frame helpers:

  • Rebind Omarchy palette watches after symlink switches; retain hidden filesystem changes without rescanning.
  • Share completed startup/manual update results with existing and newly opened Settings pages, without extra status-only requests.
  • Localize unclassified network-location failures while preserving operation guidance, native-volume errors, and diagnostic privacy.
  • Wait for persisted bookmark publication before showing numbered pin feedback.
  • Keep deep breadcrumbs readable and scrollable; separate translated density presets.
  • Distinguish neutral sidebar hover from the active-location accent when navigation leaves the pointer over a previous place.

Extend existing behavioral coverage and retain main's scroll-before-click precondition in the media-Properties scenario. Sidebar navigation state was already correct; its nearly identical hover/active styling caused the apparent double selection.

Visual evidence

whole-issue-before-after

How to test

  1. Follow an Omarchy theme, switch its symlink, then edit and atomically replace the new palette's colors.toml.
  2. Let the startup update check finish, then open Settings → Updates without pressing Check now. Repeat with two Settings windows open.
  3. With hidden files off, create, rename, and remove dotfiles externally; toggle Show Hidden in Columns, Icons, and List, including after restart.
  4. Enable 10xer mode and pin folders with g +; inspect confirmations for slots 1–4, 9, an unnumbered tenth pin, and a duplicate.
  5. Open a deep path in a narrow window, submit/cancel its location editor, and scroll the breadcrumbs. With Russian and larger text, exercise both Frame and Content density presets.
  6. With GVfs installed and a non-English interface, try an unavailable network location; also check missing-backend and permission-denied guidance.
  7. Click Desktop, keep the pointer there, and press Alt+Up. Repeat with Back and location-bar navigation while the pointer remains over the previous place.

Expected result: Changes stay live without refresh/restart or redundant update requests; pin confirmations reflect saved slots; breadcrumbs and presets remain usable; only the current sidebar location has the accent fill; network errors use localized application wording without exposing URI secrets or losing actionable guidance.

Related issue

Closes #1862.

Reuse existing theme watches, update listeners, bookmark persistence, and frame scheduling to repair seven verified findings without extra directory scans or status-only update requests. Preserve localized backend fallbacks and meaningful operation/device guidance.

Refs #1862. Sidebar tint remains unconfirmed and unchecked.
@github-actions github-actions Bot added the fix label Oct 27, 2026
Preserve the upstream menu-wheel and settle precondition instead of stacking two equivalent scroll actions. Keep the measured scenario cost and all preview, RTF, and document fixes from main.
@JoeJoeflyn

Copy link
Copy Markdown
Collaborator Author

Resolved and pushed the conflict with main in 622de01e. Retained upstream's menu-wheel/settle precondition instead of duplicating equivalent scrolling, preserving all preview/Properties assertions.

Post-merge validation: 9 selected Rust regressions and 10 QA runs passed; pinned formatting, Clippy, and whitespace checks passed. Earlier full-suite findings and their scoped recoveries are recorded in #1862; the full suites were not repeated merely to obtain a green summary.

GitHub now reports mergeable=true. The PR remains draft: metadata checks pass, while full CI jobs are draft-gated/skipped, not passed.

The requested before/after screenshots were shown in the conversation and copied into /home/giogio/Project/strata/target/issue-1862/evidence/ as breadcrumbs-before.png, breadcrumbs-after.png, density-before.png, and density-after.png. These are local files, not GitHub attachments; they are not committed. The owner's Visual evidence section was preserved.

Seven original issue checkboxes are checked; sidebar tint remains unchecked/unconfirmed. No merge or ready-for-review action was taken.

Reserve the accent fill for the current place and use one shared neutral hover rule for place and device rows. Keeping the pointer on the previous place after Alt+Up, Back, or address-bar navigation no longer resembles a second active location.

Refs #1862
@JoeJoeflyn

Copy link
Copy Markdown
Collaborator Author

All eight findings are now implemented, with the last sidebar fix pushed as 0f623f42. Retaining the pointer on the previous place reproduced the apparent stale double tint; neutral hover now remains distinct from the active-location accent, without changing navigation state.

Whole-issue evidence and exact validation details: #1862 (comment). The complete local before/after gallery is target/issue-1862/evidence/whole-issue-before-after.png; its README links every full original, all four pin slots, and all three sidebar navigation routes. These are local files, not uploaded attachments. The owner's Visual evidence section was preserved.

The bounded final sidebar change passed four targeted QA runs plus pinned formatting/Clippy and whitespace checks. Earlier full-suite recovery and merge-validation results remain recorded; full CI is still draft-gated/skipped, not passed. The original issue checklist now marks all eight verified on this branch, not merged or released.

@JoeJoeflyn
JoeJoeflyn marked this pull request as ready for review October 27, 2026 19:04
Reuse the existing 寬鬆 wording from the density status for its Airy preset button. Keep the remaining eleven reviewed catalogs unchanged.

Refs #1862
@wmfeht

wmfeht commented Oct 27, 2026

Copy link
Copy Markdown
Collaborator

Request changes

This fixes the eight stale-state items from #1862: Omarchy palette rebind, shared update-check results, hidden files staying in the monitor, g+ pin slot feedback after bookmark publication, breadcrumb widths and scroll, density snap labels, a translated generic network error, and sidebar hover versus active. Two user-visible gaps should be fixed before merge.

Check now clears other Updates rows

run_check calls clear_cached_update_notice() (src/ui/settings.rs:1923), which publishes None to every bound row. View::apply treats None as idle when that row is not itself checking and no install is underway (src/ui/settings/update_status.rs:80), so it drops pending and the Install button. The initiating row is safe because it sets checking first. A second open Settings window loses the install offer until the new result arrives. The comment at line 1919 only needs the sidebar notice cleared.

Keep the last rendered offer on rows that did not start this check, or publish an in-flight state. Clear only the sidebar notice immediately.

G_IO_ERROR_FAILED loses a known permission reason

gio_backend_error_message (src/services/file_source.rs:226) maps every unclassified kind, including Failed, to "An unexpected system error occurred". uri_validation_result (src/adapters/local_files.rs:298) and remote_directory_failure (src/adapters/local_files.rs:466) use it. Mount still treats GVfs SMB Failed plus "permission denied" or "authentication failed" as an auth retry (src/ui/browser/location.rs:694, checked at lines 1807 and 1860 before the generic message). Opening or listing a location that fails the same way shows the generic sentence.

Before the generic fallback, map those same phrases to the existing "Permission denied" catalog string, and keep the sanitized message in the debug log.

Smaller notes

  • Breadcrumb reveal clears breadcrumb_reveal_pending before the idle callback knows upper is usable (src/ui/browser.rs:455). Leave it set until upper > page_size.
  • A row with install_underway ignores a newer snapshot (src/ui/settings/update_status.rs:72). Cancel and failure (src/ui/settings.rs:2076) do not re-apply CURRENT.
  • Startup publishes release_channel() (src/ui/settings.rs:257) while the row accepts only effective_update_channel (src/ui/settings/update_status.rs:75). Pacman and Omarchy with a non-stable preference still show idle after startup.
  • The new sidebar test checks the active class (src/ui/window/tests/sidebar.rs:126). The hover and active split is CSS (src/style.css:2372).
  • start_watch always passes include_hidden: true (src/app/browser.rs:4174), so hidden churn is queried on network folders too.
  • ellipsize_crumb_label now sets width_chars on hierarchy menu rows (src/ui/browser/location.rs:37, call at line 2671).

Omarchy rebind, pin publication before the slot flash, density gap math, and the typed GIO reasons look right.

wmfeht
wmfeht previously approved these changes Oct 28, 2026
Starting a check cleared the shared result and reset every bound Updates
row to idle, so a second Settings window lost its install offer until the
new result arrived. Only the sidebar notice and the shared result are
cleared now; rows that did not start the check keep their last rendering.

Startup checks also published the raw channel preference while rows
filter on the effective channel, so Pacman and Omarchy installations with
a non-stable preference ignored the startup result.
GVfs SMB reports refused credentials as G_IO_ERROR_FAILED with the reason
only in its message. Mount retries already recognised those phrases;
location validation and directory listing now show the Permission denied
catalog text for them instead of the generic system error.
The sidebar follows browser model events, so repeating the navigation
routes per view mode exercised nothing new.
@l0gicgate

Copy link
Copy Markdown
Contributor

Review

Merged main at 73b3258ff (02f4ec794). The merge was textually clean, but #1885 added two pin-handler test closures in keyboard_dispatch/direct_commands.rs with the old two-argument signature; they are adapted to the completion callback inside the merge commit.

Fixed (from the request-changes feedback)

  • Check now no longer resets other Updates rows (153e8171f). run_check now drops only the shared result and the sidebar notice; rows that did not start the check keep their last rendering, including the Install button, until the new result lands. The preference-change path still resets every row. Covered by a_check_started_in_one_row_keeps_other_rows_offers_until_its_result.
  • Startup results reach package-managed rows (same commit). complete_due_update_check published the raw channel preference while rows filter on the effective channel, so Pacman and Omarchy installs with a non-stable preference stayed idle after startup. The startup test now runs with the Preview preference to cover this.
  • G_IO_ERROR_FAILED credential rejections read as Permission denied (016302241). The SMB phrase list moved from location.rs into services::describes_refused_credentials, shared by the mount retry check and gio_backend_error_message, so validation and listing failures show the catalog text instead of the generic system error. Covered in untyped_gio_failures_use_localized_catalog_text_instead_of_backend_details.

Test and comment audit

  • navigation_and_history_keep_only_the_current_sidebar_place_active looped over the three view modes, but the sidebar follows browser model events, so the loop exercised nothing new; it now runs once (0cfb5357e). The test remains as state coverage for the parent/back/location routes; the hover/active split itself is CSS and is not unit-tested, per the no-layout-tests rule.
  • The other new tests (Omarchy symlink rebind, startup result sharing, untyped GIO text, pin slot feedback scenario, hidden-file monitor scenario) are behavioural and were kept.
  • Every added comment carries non-obvious information; none were removed.

Not changed (non-blocking notes)

  • Rows with an install underway ignore newer snapshots, and cancel/failure do not re-apply the current result. Pre-existing behaviour; the next publish resynchronises the row.
  • start_watch always includes hidden entries, including on network folders. Only the monitor event filter is affected; the freshness poll is unchanged and the chooser still passes its own flag.
  • ellipsize_crumb_label also sets width_chars on hierarchy menu rows. Harmless: the minimum equals the text width the popover already requests.
  • The breadcrumb reveal flag is consumed in the frame-clock layout handler, which runs after GTK's own layout for that frame, and the scroll itself is deferred to an idle; it behaved correctly in the QA runs below.

Verification

Native headless tests (scripts/test-headless.py): services::file_source::tests, ui::settings::tests, ui::theme::tests, ui::browser::location::tests, ui::window::tests::sidebar, ui::window::tests::keyboard_dispatch::{direct_commands,place_chords,overlay_focus}, adapters::local_files::tests::remote_backend, i18n:: — 253 passed, 0 failed. Pinned-container QA: hidden-files-persist-across-restart (columns, icons, list), tenxer-pin-feedback-identifies-slot, media-details-properties-only — 5 passed. cargo fmt --check, cargo clippy --all-targets --all-features -D warnings, and git diff --check are clean. No UI change was made in this review, so no screenshots were taken.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Stale UI and cosmetic issues from QA sweep

3 participants