You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Four dialogs wrapped their fields in a <form> with no onsubmit handler and
placed the submit button outside the form. Since our Button defaults to
type="button", Enter fell through to a native GET submit and tore the dialog
down. Worst case was the share-code redeem dialog, a single text input whose
whole purpose is type-a-code-and-press-Enter.
Also removes 21 leftover Skeleton UI classes (border-surface-*, modal-form,
rounded-container-token) across 12 files. Nothing defines them under Tailwind
v4, so they emitted no CSS: the sites that wanted a border already get the
token color from the base layer, and the four forms never had one at all.
Reboot and the Wi-Fi hotspot toggles fired straight at the hub on a single
dropdown click while Pair, Regenerate Token and Delete in the same menu all
sat behind dialogs. A mis-click restarted hardware attached to a person.
Both now route through dialog.confirm with copy that states the consequence.
Emergency Stop is deliberately left instant. It is a safety control, so the
cost of a delayed real stop outweighs the cost of an accidental one.
The mobile branch had no {:else} on its {#each}, so a user with no hubs got a
blank screen with no explanation and no call to action. Desktop only managed a
bare "No hubs found." table cell. Both branches now share one EmptyState with
an Add Hub action, hoisted above the responsive split.
h-screen is 100vh, the viewport with a phone's URL bar hidden, so the shell
outgrew the visible page and left it scrollable by exactly that bar's height.
Locking and unlocking body scroll around the mobile sidebar sheet's transition
then shifted the whole page for a frame. h-svh matches the sidebar wrapper's
min-h-svh. w-full for the same reason on the other axis, since 100vw counts
the scrollbar.
This PR contains 118 files, which is 18 over the limit of 100.
To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch.
Upgrade to a paid plan to raise the limit.
This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry.
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Advanced
Run ID: 171feec7-c003-46e3-b581-a51a16d61839
📥 Commits
Reviewing files that changed from the base of the PR and between 279ba8f and d6a4154.
UserSelector renders its own <form>, so this new outer submit handler creates nested forms. Pressing Enter in the username field can bubble the selector's submit into onFormSubmit and create/send the share before the user intended; nested forms also produce invalid, hydration-sensitive HTML. Keep the selector outside the share-creation form (or remove its internal form) so each submit action remains independent.
Picks up the status tokens (--success/--warning/--info) that the palette sweep
depends on, PageLoading and PageError, and ConfirmDeleteDialog's pending state
and Cancel button.
Sweeps about 230 raw Tailwind palette classes out of the app in favour of the
theme's tokens, now that --success, --warning and --info exist alongside
--destructive in svelte-core.
Mappings: green -> success, yellow/amber/orange -> warning, blue -> info,
red -> destructive, and the neutral ramps -> foreground/muted-foreground/
border/ring/background. Where a class was half of a light/dark pair the dark:
override goes away, since the token already switches: 14 copies of
"text-gray-600 dark:text-gray-300" on spinners collapse to
text-muted-foreground, and CircleSlider loses five such pairs.
Fixes several sites that were broken in one theme or the other:
- report/api-tokens was light-only (bg-red-50, bg-gray-50, bg-green-200) and
rendered as a white slab in dark mode
- Footer used text-green-800/text-red-800 for connection state, near-invisible
on a dark background
- ring-slate-800 on three share items, likewise
- admin/blacklists had a bg-gray-800 pill, unreadable in light mode
Also drops hardcoded hex from Turnstile, Footer, SerialPortSelector,
FirmwareChannelSelector and FirmwareFlasher.
Deliberately left alone: WelcomeScreen and the marketing page (intentionally
always-dark brand surfaces), CircleSlider's blue accent arc, the ActionButtons
party animation, the LiveButton gradient, and the online-hubs chart palette,
which is already theme-aware via its own light/dark pairs.
The app shipped three competing page templates: PageHeader (10 pages), a
Card.Header + Card.Title class="text-3xl" pastiche standing in for a page title
(10 pages), and hand-rolled h1 blocks at four different sizes (text-xl on
settings/account, text-2xl on shockers/own, text-3xl on profile, text-8xl on
hangfire). Clicking down the sidebar changed the title size and the action-row
layout at almost every stop.
Everything in (app) now uses Container + PageHeader. Knock-on fixes:
- 22 pages had no h1 at all, because Card.Title renders a div. The (auth)
pages, the public share page, the terminal and the token report keep their
centered-card design and get a plain <h1> inside Card.Title rather than a
patch to the shadcn component, which is regenerated wholesale on update.
The marketing page's logo img moves into an h1. Heading coverage is now
complete: the only pages without their own h1 are the three shares/user tabs,
which inherit one from their layout.
- hubs/[hubId]/update and shares/public/[shareId]/edit rendered outside
Container, so content sat flush against the viewport edge. Both are wrapped
now, and the latter's fixed grid-cols-3 header bar (which had no responsive
fallback and broke on a phone) becomes a PageHeader with the share link on
its own row.
- All eight page-body wrappers converge on one class string. The four that
carried py-6 double-spaced the title now that PageHeader supplies mb-4.
- admin/blacklists had no page title whatsoever; profile and hangfire replace
ad hoc placeholder boxes with EmptyState, and hangfire stops telling users
"This is not supposed to be visible in prod" at text-8xl.
- Refresh demoted to outline where it was competing with a real primary action,
and the stray text-xl on two Refresh buttons dropped.
Loading was a spinner in an h-64 box, an inline spinner with a label,
animate-pulse divs, or a bare "Loading..." string, picked at random per page.
Errors were an ad hoc destructive <p> with an optional retry button and
per-page wording. Both now come from PageLoading and PageError, which join
EmptyState as the states a data view needs: 10 loading sites and 4 error sites
collapse onto them.
Delete dialogs each carried their own `.finally(() => (open = false))` and no
busy state, so every delete in the app was double-clickable. Closing and
pending now live in ConfirmDeleteDialog, which also gained a Cancel button and
a real Dialog.Footer, so its 8 consumers just return the request promise. That
also means those confirmations finally offer a symmetric way out.
Adds Table/RowActions.svelte and puts five action menus on it, replacing the
copy-pasted Label/Group/Item/Separator markup with a list of action objects.
This also lines up the odd ones out: admin/config's menu had no icons and no
cursor-pointer, and admin/webhooks had an Edit item with no onclick at all,
which silently did nothing - it is now rendered disabled instead.
The hubs row menu is left alone; its items are interleaved with four dialogs
and a pair-code flow, so a data-driven list would not be an improvement.
Button hierarchy: every page header now has exactly one default-variant
action. settings/account gave Change Username, Change Email and Change
Password all the filled variant, so three peer row actions competed as
primaries; they follow admin/users/[userId] and use outline. Refresh on the
hubs list and Redeem Code on user shares likewise demote to outline behind
their real create action.
Dialog footers: 20 dialogs ended in a bare button floating under a paragraph,
with no Cancel and so no symmetric way out. They now use Dialog.Footer with
[Cancel outline][Action] in that order. On the account change dialogs the
footer sits inside the <form> so type="submit" still works, and Cancel is
explicitly type="button". oauth-disconnect had a hand-rolled action row and a
secondary Cancel; it joins the same pattern. Left bare on purpose: the four
informational dismiss-only dialogs, DangerZone, the signup page, and
RiskAcknowledgementModal, whose bespoke layout is deliberate.
Copy: the marketing page rendered devicesOnline as "people online right now",
which is simply wrong. The Shared-with-Me tab reported "Failed to load
outgoing invites". The home page called hubs "your devices" under a tile
titled Hubs, though no other screen uses that word for them - "device" now
appears only in the firmware terminal, where it means a board over USB that
is not yet a registered hub.
Casing: page titles were already converging on Title Case and action buttons
on Title Case, but dialog titles were split almost evenly and "Change
Username" and "Change username" both existed for the same action. Dialog
titles settle on sentence case. Five page subtitles get consistent
punctuation, and three that read awkwardly are reworded.
- terminal used `items-top`, which is not a Tailwind class and emitted no CSS;
the row was never top-aligned. Now items-start, with gap instead of space-x.
- The public share card was pinned to w-[400px] and overflowed any phone
narrower than that. Now w-full max-w-sm.
- Four dialogs sized themselves in vh, which ignores mobile browser chrome and
clipped their content; they use dvh, and the two that pinned 80vh and 85vh
for the same job agree on one value.
- HelpDialog set both a width and an equal max-width, and both a height and an
equal max-height.
The earlier fix wired each form to an inline arrow that called
preventDefault and then `void handler()`. Two problems with that:
`void` was doing nothing. no-floating-promises is not configured, so it
satisfied no rule, and it is not how this codebase passes async handlers -
onclick={addUsernameEntry} on an async function is the established shape. The
existing `void` uses are a different idiom: `void goto(...)` for one floating
navigation and `void refreshNonce` to register a reactive read.
The arrow was hiding a two-path design. In three of these dialogs the submit
button sits outside the <form>, so the handler was reachable both from the
form's onsubmit and from the button's onclick, and the arrow existed only to
service the first. Those buttons now carry type="submit" form="<id>", which is
the standard way to submit a form from outside it, so there is one path. The
handler takes the SubmitEvent and calls preventDefault itself.
The redeem dialog's button is already inside its form, so it just gets
type="submit" with no id/form pair.
Bumps svelte-core for the new *-foreground status tokens and the darker light
--warning.
The footer heart went to text-destructive, which read as an error state. It is
decoration and goes back to its own #e25555.
On the leaked-token report page, red and green text on its own /10 and /15 tint
fell to about 4:1 and 3.7:1 in light mode, below AA. Body text is now foreground
and the tint and a left border carry the state.
CircleSlider's handle (stroke-border) and track (stroke-muted) were barely
visible on a white card. Both now use muted-foreground at reduced opacity. The
focus ring was a box-shadow on an SVG circle, which never renders; focus now
thickens and recolours the stroke.
The permission tooltip and terminal drew white on bg-success and bg-destructive,
under 3:1 on the lighter dark-mode fills. They use the matching foreground
token.
Smaller fixes:
- outline-ring was used as a decorative outline on ControlView; it is the focus
ring colour, so it becomes outline-muted-foreground/50.
- DiscordLogo and LiveControlIcon use fill-current so they follow their
parent's text colour instead of matching it by coincidence.
- The connections skeleton was bg-muted on a white card, nearly invisible.
- The serial terminal log area became lighter than its surroundings in dark
mode; it is recessed again.
Emergency Stop still dropped its send promise, so a send on a disconnected
SignalR connection was an unhandled rejection with no feedback. All hub
commands now go through sendHubCommand, which refuses with a toast when the
connection is not Connected and reports send errors. Previously the serializers
returned silently on a null connection, so a confirmed Reboot could do nothing.
The hubs list showed "No hubs yet" while the first fetch was in flight, and kept
showing it if the fetch failed. hubs-state now tracks whether ownHubs has been
loaded, and the page shows PageLoading or PageError before the empty state.
The delete dialogs caught their own errors, so ConfirmDeleteDialog always saw a
resolved promise and closed even when the request failed. They now rethrow
after reporting, and the dialog stays open for a retry. Factory reset returns
its send instead of closing the dialog itself.
The share invite dialog's Cancel set open = false directly, which skips
onOpenChange, so the looked-up user and shocker selection survived a reopen
and the next submit invited the previous user. Cancel and success now go
through onOpenChange. The add-shocker dialog had the same leak. The invite
dialog also blocks repeat and empty submits, which Enter-to-submit made easy.
The redeem dialog sent empty codes, left the input disabled after an error, and
showed the error twice. It now validates, resets on failure, and relies on the
toast.
The admin config, webhook and user dialogs closed in finally, losing input on
failure, and had no pending guard. Public share create had no error handling.
All now close only on success and disable while submitting. The config edit
dialog said "Add configuration item", "Create" and "Created"; it now says
Edit, Save and Updated.
The token edit dialog's footer bleeds with negative margins, which the
overflow-y-auto wrapper clipped. The footer now sits outside the scroll area.
…tion
The share editor body sat directly in Container, which is items-start, so its
sections shrank to content width and the device dividers collapsed. It gets
the full-width body wrapper the other migrated pages use, and its author chip
drops outline-ring (the focus ring colour) for outline-muted-foreground/50.
Edit Shocker still had a hand-rolled h1 and back button; it now uses
PageHeader, PageLoading and PageError.
Smaller fixes:
- edit-share's drawer still used max-h-screen; it moves to max-h-dvh like
manage-share.
- The blacklists subtitle said email addresses; the second list blocks email
providers.
- The landing page read "1 hubs online".
- Shorten the app shell comment to one sentence per line.
UserSelector rendered its own <form> inside the share invite dialog's form.
The HTML parser drops a nested form tag, so Enter in the username field could
submit the dialog and create the share before the lookup resolved. The
selector is now a div with a type="button" search; Enter runs the lookup while
the name is unconfirmed and falls through to the dialog form once it is.
Closing the redeem dialog while a redeem was in flight let the late response
repopulate the result, so reopening showed the old success with the input
disabled. A generation counter bumped on close discards those responses.
The delete dialogs caught, reported and rethrew so ConfirmDeleteDialog would
stay open for a retry. Each handler is now a plain async function with its own
try/catch around handleApiError, and svelte-core's dialog only logs a rejection
that still reaches it. A failed delete closes the dialog after the error toast,
as it did before this branch.
The success toast now comes before onDeleted/onRevoked in all eight, so a
failing callback cannot hide that the delete went through. The public share
toast read "Deleted publicShare successfully"; it now says "Deleted public
share".
Addresses the review findings, and the markup drift they sat on top of — these
specs were written in August, before #254 rewrote the pages they drive.
- account-settings: the username is text with a 'Change Username' dialog now,
so the update test opens the dialog, submits, reloads and asserts the new
name survived; the short-name test does the same. The email assertion no
longer looks for an input placeholder that the dialog owns. Both .catch()
calls that discarded their assertion are gone.
- api-tokens: the nested 'if (await x.count())' guards skipped every assertion
when an element was missing, so a token that was never deleted still passed.
Each step now asserts. TableActionMenu's sr-only 'Open menu' label confirmed
still present.
- auth: logout is a menuitem labelled 'Logout' inside the header avatar
dropdown, not a button or link, so the old locators could never match and
the test passed without logging out.
- login-logout: assert the error toast or inline validation rather than
sleeping 3s and re-checking a URL that was already /login.
- signup-verify: poll for 'dialog appeared or redirected to /login' instead of
reading page.url() before the wait and relying on isVisible's ignored timeout.
- test-fixtures: take the cookie domain from the frontend origin the browser
actually talks to, not TEST_BACKEND_URL's host.
Not executed: this sandbox cannot reach container host ports, so the stack
cannot run here. Verified statically — check, lint, format and playwright
--list all clean, selectors checked against current component source.
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
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.
No description provided.