fix/audit-batch-1

vas2000-work commented

2026-05-07 03:58:51 +02:00

Owner

No description provided.

vas2000-work added 19 commits

2026-05-07 03:58:51 +02:00

fix: stop /portal/tickets/new from leaking form data via GET 400ee5bd55

P0 from the 2026-05-06 UI/UX audit. The portal ticket-submission form had no `onsubmit` handler, so the browser default-submitted as a GET and pushed subject/description/priority into the URL, then blanked the SPA. Add `e.prevent_default()` so the form is inert until a real POST endpoint exists (gated on server F6 portal work).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

fix: highlight the current sidebar item 4e3313c37a

P1-08 from the audit. Sidebar items always rendered in the un-active style, leaving users with no visual indication of where they were. Wire `use_route::<Route>()` and apply the existing active class (which adds bg-gray-800 + a left blue border) when the item's `to` matches the current route.

Detail-page parent highlighting (e.g. TicketDetail -> Tickets nav) is intentionally NOT done here; it needs a path-prefix check that's larger than the audit fix asked for. Top-level navigation now visually tracks the page.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

fix: drop dishonest TableRow clickable: true from rows with no onclick c8b4c0d1e3

P1 from the audit (was P1-03 / `TableRow{clickable:true}` without onclick). Affected ~20 rows across dashboard, tickets, time, contacts, contracts, billing, assets, KB, admin, portal pages: rows showed `cursor: pointer` on hover but had no `onclick` wired, so users clicked expecting navigation and nothing happened.

Caller-side fix per the audit's option (alternative was a component-level prop API change which would touch 20+ call sites and risk regression on the rows that ARE legitimately clickable). Surgically removes `clickable: true` only from rows with no onclick within the next 8 lines; rows that have a real onclick are untouched.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

fix: stop emitting the HTML title from both index.html and Dioxus.toml f33f4c1f0d

P1-09 from the audit. Browser tab title rendered as "Mokosh PlatformMokosh Platform" because both `index.html` (`<title>Mokosh Platform</title>`) and `Dioxus.toml` (`[web.app] title = "Mokosh Platform"`) injected the title into the built HTML. Drop the Dioxus.toml side; index.html stays the single source of truth.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

fix: stop portal pages from rendering their title twice f41cf07536

P1-10 from the audit. `My Tickets`, `Submit Ticket`, `Invoices`, `Knowledge Base` rendered the page title both via the `PortalLayout { title: "..." }` prop AND a page-level `<h1>` immediately below it. Drop the layout-prop side on these 4 pages and keep the page-level h1, which is what users actually look at (and which often appears in a flex row with action buttons like "New Ticket").

`PortalHomePage`, `PortalTicketDetailPage`, `PortalInvoiceDetailPage` were not duplicating (their layout title and page h1 carried different content) and are unchanged.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

fix: drop href=\"#\" from anchors with no destination c609d13ce9

P2-18 from the audit. 16 instances of `<a href="#">` rendered as link-styled, cursor-pointer text but did nothing on click — Privacy/Terms/Contact in the home footer, Contact in login, KB related-articles, contract documents, asset RMM link, notification email-template list in settings, portal article items. Removing the href turns them into inert spans (browser drops the link cursor and underline) without changing the visible text or surrounding layout.

When the underlying destinations exist (real Routes for KB articles, mailto: for Contact, etc.) they should be added back as proper Links; tracked separately. This PR removes the fake-link affordance only.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

fix: bring SLA pill text contrast above WCAG AA d595260216

P2-11 + P2-12 from the audit. The dashboard's SLA-warning pills had:
- danger:  text-red-400    on bg-red-900/20    (dark-red on dark-red — fails AA)
- warning: text-yellow-400 on bg-yellow-900/20 (yellow-on-yellow borderline)

Shift the dark-mode foreground to *-200 (and bump the light-mode foreground a half-shade) so the contrast ratio passes WCAG AA (4.5:1) for normal text. Light mode was already passing but tightened slightly for consistency.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

fix: wire onclick navigation on dashboard rows b76e9b0a47

Follow-up to the TableRow honest-cursor pass. PR-D removed the false `cursor-pointer` from rows that had no onclick wired, but the dashboard's Recent Tickets, SLA Warnings, and Recent Time Entries rows *should* be clickable - they reference real tickets / time entries the user expects to drill into. Add the missing handlers:

- RecentTicketRow -> Route::TicketDetail { id: <number> }
- SlaWarningItem  -> Route::TicketDetail { id: <ticket> } (also a div, so its own cursor-pointer + hover:opacity-80)
- TimeEntryRow    -> Route::TicketDetail when the description leads with "TKT-...:", else Route::TimeEntryList

The `clickable: true` flag is restored on the two TableRow callers since they now have real onclicks.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

fix: wire onclick on tickets/companies/contacts/contracts/invoices/assets list rows eb62c38962

Restores the row-level click affordance that PR-D removed, this time with real navigation. For each row component, pull `use_navigator()` and a clone of `props.id`, then on the TableRow set `clickable: true` and `onclick: move |_| { navigator.push(Route::XDetail { id }); }`.

Coverage:
- TicketRow      -> Route::TicketDetail
- CompanyRow     -> Route::CompanyDetail
- ContactRow     -> Route::ContactDetail
- ContractRow    -> Route::ContractDetail
- InvoiceRow     -> Route::InvoiceDetail
- AssetRow       -> Route::AssetDetail

KB articles, admin tenants, and portal tickets are inline TableRow blocks with hardcoded mock IDs (no extracted *Row component); deferred until they're loop-driven from real data.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

fix: unfreeze stub-submit forms (projects, time, companies, contacts) 8deee10254

P1-04 from the audit. The four New-X forms set `is_submitting=true` on submit and never reset, leaving the spinner button frozen forever. Mirror the existing pattern in tickets/new: spawn a 1s mock TimeoutFuture, flip is_submitting back to false, then `navigator.push(<list route>)`. Real POST/redirect lands when the corresponding server module / endpoint is wired.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

fix: call navigator() inside async-block stub-submit handlers da1eb4bbdb

The previous commit's `let navigator = use_navigator();` binding worked for the simple `move |_|` onclick closures but the outer `move |e| { spawn(async move {...}) }` shape can't reliably propagate the captured Navigator into the inner async block — rustc resolves `navigator` to the prelude `fn item` instead. Switch the four stub-submit handlers to `dioxus::prelude::navigator().push(...)` which calls the prelude function inside the async future, sidestepping the capture-chain entirely.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

fix: client polish batch (P2-14/16/17/19, P3-22/26) 9bde6832bf

- P2-17 StatCard delta indicator: text-sm -> text-base, semibold; was easy to miss next to a 32px stat number.
- P2-16 required-field marker: add aria_label="required" + role="img" on the red `*` so screen readers announce it.
- P3-22 Pagination prev/next: explicit class shift on boundary so the disabled state is visibly distinct from the active state (audit: "looks identical to active >"). Was relying on Tailwind's `disabled:` modifier alone.
- P3-26 UserMenu: avatar top-bar button was a plain non-interactive icon; replace with a Profile / Settings / Logout dropdown. Logout clears the auth signal and routes to /login.
- P2-19 KB CategoryCard: wrap in Link to /kb/articles so the cursor-pointer affordance actually navigates.
- P2-14 reports Date Range / Group By: Dioxus 0.7 doesn't render `selected:` on bare `<option>`; drive selection via `value:` on the `<select>` element with explicit option values.

Skipped P3-25 (decorative top-bar search caret): SearchInput component has no caret element to remove; the audit's observation may have been about a different rendering path.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

fix(reports): replace chart placeholder text with styled cards (P2-15) c2a70061c7

Audit P2-15 flagged that the report detail page shipped literal "[Chart placeholder - ...]" debug strings. Replace with a reusable ChartComingSoon component that shows a dashed-border card with the chart icon, "Charts coming soon" heading, and the specific caption - so the page communicates "this is intentionally pending" instead of looking like leftover dev stubs.

fix(layout): collapse double scrollbar on AppLayout (P3-20) 09c1b97e96

Audit P3-20 flagged "two scrollbars at narrow viewport heights" on /dashboard - the body scrolled because the inner column used min-h-screen, and the desktop sidebar had its own overflow-y-auto, so they stacked. Pin the outer container to h-screen overflow-hidden and move the scroll surface into <main>, giving exactly one main-content scrollbar plus the sidebar's internal scrollbar only when the nav itself overflows. Add overscroll-contain on both surfaces so scroll chains stay isolated.

fix(detail-pages): hide decorative action buttons (P1-07 partial) 6e1e195835

Audit P1-07 called out detail-page header buttons that look interactive but had no onclick and no backend to call. Hide the clearly-fake set so the UI no longer mimes capability it doesn't have:

- billing.rs InvoiceDetail: Download PDF / Send / Record Payment
- portal.rs invoice row: Pay Now (no portal payments flow yet)
- contracts.rs ContractDetail: Edit / Renew
- assets.rs AssetDetail: Edit / Remote Connect (no RMM integration)
- reports.rs ReportDetail: Export PDF / Export CSV / Schedule and the no-op Apply Filters button
- time.rs Timesheets: Submit Timesheet (no submission workflow)

Each removal carries a one-line "audit P1-07" comment naming the future flow that should restore the button when it ships, so the trail is obvious to a future reader. Edit/list-page-level buttons that DO have real targets are left alone - this PR only pulls the genuinely decorative ones.

chore: clear all cargo check warnings 04b8173bc6

Three categories of cleanup, no behavior change:

1) Declare the `server` feature in Cargo.toml (without enabling it). Several modules under src/modules/* are shape-shared with the server side and gate server-only code with #[cfg(feature = "server")]. Declaring the feature name silences the 22 "unexpected cfg condition value: server" warnings without pulling the server-only code into the wasm build.

2) Drop unused imports across components/table, hooks/auth, hooks/fetch, utils/error, and the page modules (Spinner, ChevronDownIcon, AuthState, DeserializeOwned at file scope, BadgeVariant/Badge/Button/ButtonVariant/DataTable/PageHeader/EmptyState/SearchInput/crate::Route where unused, std::fmt). Drop the dead `let fetch_fn_clone = fetch_fn.clone();` in the paginated fetch hook (the use_effect closure clones again on its own).

3) Add #[allow(unused_variables)] on the detail-page #[component] functions where `props` is on the signature for the macro's Props-struct generation but isn't read in the rsx body yet (assets, auth/reset, billing, contacts x2, contracts, knowledge_base, portal x2, projects, tickets). Also on AlertItem in assets.rs where rsx! interpolation of `icon_class` doesn't register as a use to rustc. Drop the dead `let mut selected_week = use_signal(...)` in TimesheetsPage (the chevron buttons have no onclick yet).

Result: cargo check --target wasm32-unknown-unknown finishes with zero warnings.

fix(card): align header padding with body when padding is disabled 8cf7487232

The Card component put `p-6` only on the outer wrapper, so cards using `padding: false` (Recent Tickets and Recent Time Entries on the dashboard, where the body is a full-bleed table) lost the header inset too: the title sat flush against the card edge while table cells below kept their own internal padding, and "View all" hugged the bare right edge. The same cards with `padding: true` (Today's Schedule, SLA Warnings) read normally.

Move padding off the outer wrapper. CardHeader now self-pads (`px-6 pt-6 pb-4`) so its title and actions stay inset regardless of the body. The body gets a separate wrapper that adds `px-6 pb-6 pt-4` only when `padding: true`, so a full-bleed table sits flush edge-to-edge while the header still aligns with the table's column padding above it.

fix(headings): stop descender clipping on page titles 4111661a67

The page-title h2 in PageHeader used `leading-7` (28px line-height) plus `sm:text-3xl` (30px font) plus `sm:truncate` (overflow:hidden). At sm and up the line-height was smaller than the font, so g/j/p/q/y descenders sat below the line box and got chopped by overflow:hidden. Visible on every AppLayout page that renders through PageHeader (Projects, Dispatch, Settings, Knowledge Base, etc).

Two fixes:

1) Targeted: add `sm:leading-9` (36px) to PageHeader's h2 so the line box keeps up with the larger font at sm+.

2) Defensive: a base-layer rule in input.css that catches any heading h1..h6 whose class attribute contains `truncate` (matches `truncate`, `sm:truncate`, `md:truncate`, etc) and floors its line-height at 1.25 plus a 0.125em bottom pad. New code that uses `truncate` on a heading can't reintroduce this clip without explicitly overriding it.

assets/styles.css is gitignored and will pick up the new rule on the next css-build / dev start.

feat(layout): full-width top bar, collapsible sidebar sections

Create release / Create release from merged PR (pull_request) Has been skipped

Details

e1801ebb22

Restructure AppLayout so the top bar spans the full viewport width with the brand on the left (above the sidebar column), page title in the middle, and search + notifications + user menu on the right. The sidebar starts beneath the top bar and contains nav only - eliminates the previous "two boxes of different heights" mismatch where the brand block in the sidebar had different padding than the topbar holding the page title.

Other layout changes:

- Sidebar gets `border-r border-gray-700` so it has a clean vertical separator instead of bleeding into the main column.
- Sidebar's overflow-y-auto now uses the new `.scrollbar-hide` utility (added to input.css). Long nav lists still scroll, but the scrollbar trough is gone - the sidebar reads as a single column.
- NavSection becomes a collapsible accordion: header is now a clickable button with a chevron (down = expanded, right = collapsed) that toggles its child NavItems. Each section's open/closed state is keyed by title in a SidebarState signal owned by App-root context, so collapse choices survive SPA navigations (each page re-mounts AppLayout, but the signal lives one level up).
- New hooks: use_sidebar_provider() at App root + use_sidebar_state() for NavSection consumers + is_section_collapsed() helper.
- Header → TopBar rename (no external users of Header). The brand inside the top bar Link's to Dashboard, mirroring most app conventions.
- UserMenu dropdown items get inset rounding (`p-1` on the menu, `rounded-md` on each button) so hover backgrounds stay rounded inside the rounded shell - corners no longer go square on first/last item hover. Keeps the menu's shadow intact (no overflow-hidden).

assets/styles.css is gitignored and will pick up the new `.scrollbar-hide` rule on the next css-build / dev start.