refactor(desktop): give the windowed-list footer one primitive - #215
Merged
Conversation
Six views render only the first `render_limit` matches, and each had grown
its own copy of the same "Showing N of M / Show K more" footer: audit
all-apps, audit findings, the audit dashboard, and the Sites, Key Vault and
Mailboxes resource-access panels. Identical apart from the noun they count
("matching rows", "affected", "apps").
All six hardcoded `class="audit-show-more"` — including the Key Vault,
Sites and Mailboxes panels, which have nothing to do with the audit. A
class name travelling into unrelated views is the tell that the pattern had
no owner: `components/ui/` holds fourteen primitives and had none for this,
so the sixth copy was as easy to write as reusing anything.
Now `components::ui::ShowMore`, styled by `.show-more` (renamed with it —
no test or other rule referenced the old name). It renders nothing when
everything fits, so each call site drops its own `(total > limit).then(..)`
wrapper too: 131 lines deleted for 59 added, and two files lost their last
use of `Button`/`ButtonAppearance` entirely.
`next_page_size` is split out so the one piece of arithmetic here is
testable on the host — the browser suite is the frontend's only other gate,
and it cannot run on a box without Chrome. It saturates so the function is
total on its own; the component's guard already made the subtraction safe,
as it did in all six copies (no underflow was reachable), but a helper
that is only correct when its caller checks first is a trap for the next
caller.
The mailboxes panel's *second* use of the old class was not a copy of this
footer — it is the "show apps with no access" toggle, which only borrowed
the layout. It keeps its own markup and follows the rename.
AGENTS.md's "one primitive per UI pattern" list now names it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Six views render only the first
render_limitmatches, and each had grown its own copy of the same "Showing N of M / Show K more" footer — identical apart from the noun they count:audit_view/mod.rs(all-apps)audit_view/findings.rscomponents/audit_dashboard.rsresource_access/sites.rsresource_access/keyvault.rsresource_access/mailboxes.rsAll six hardcoded
class="audit-show-more"— including the Key Vault, Sites and Mailboxes panels, which have nothing to do with the audit. A class name travelling into unrelated views is the tell that the pattern had no owner:components/ui/holds fourteen primitives and had none for this, so writing a sixth copy was as easy as reusing anything.Now
components::ui::ShowMore, styled by.show-more(renamed with it — no test, rule or other view referenced the old name). It renders nothing when everything already fits, so each call site drops its own(total > limit).then(..)wrapper as well.131 lines deleted for 59 added, and two files lost their last use of
Button/ButtonAppearanceoutright.A correction to my own scan
I originally counted seven copies. Six. The seventh
audit-show-moreinmailboxes.rsis the "show apps with no access" toggle — a different control that only borrowed the layout class. It keeps its own markup and follows the rename.On testing this
next_page_sizeis split out so the one piece of arithmetic here is testable on the host, since the browser suite is the frontend's only other gate and cannot run on a box without Chrome.It saturates so the function is total on its own. To be precise about what that is and isn't: the component's
total > limitguard already made the subtraction safe, as it did in all six copies — no underflow was reachable. This is not a latent bug I found. It is that a helper which is only correct when its caller checks first is a trap for the next caller.Test plan
just verify— all gates greencargo test --locked show_morein web-rs — 3 new host-side tests pass (full page, short last page, window wider than the row set)just web-clippyclean, including the two now-unusedthawimports the extraction exposedrg audit-show-more— no references remain outside the new component's own doc comment explaining the renamejust web-itestcould not run locally — there is no Chrome or chromedriver on this machine at all. CI's Frontend (Leptos/WASM) check is the gate for this change, and it is the check that matters most here: this is markup across six views. I grepped the 27 GUI test files for the class and the on-screen strings and found no references, so nothing should break — but that is reasoning, not a run🤖 Generated with Claude Code
https://claude.ai/code/session_018svudSB34WVN4SKU4hBJoc