Repository navigation
Fix the first paint, and route with TanStack Router - #15
Merged
Merged
Conversation
ThemeToggle applied the stored theme from an effect, so a reader who had picked light while the OS prefers dark saw the dark palette for a frame. An inline script in the head now sets data-theme before anything renders. The storage key is duplicated there because the script cannot be a module. CSS already covers the unpinned case through prefers-color-scheme, so "system" deliberately stamps nothing and falls through to it.
A deep link such as #/trust-auth showed the landing page until its capture arrived. App derived the screen from three separate pieces of state and fell back to the landing page while a fetch was still in flight. Each capture route now has a loader, so a screen does not render until its capture is in hand. App.tsx is gone, replaced by the route tree. Hash history, because GitHub Pages cannot rewrite an arbitrary path back to index.html. Routes gain a leading slash as a result, so a capture is now at #/trust-auth rather than #trust-auth. Linking to a single message is removed rather than ported over. It meant carrying a session id and a packet id through the router, the loaded capture and the explorer, and it was the only reason the packet list centred a row it revealed. The message index is reference data now, so its Example column goes with it. The packet list aligns a revealed row to the nearest edge instead, which is what hexWindow.ts already did, so a keyboard step moves the list by one row rather than half a viewport. That scroll is animated, and the row's focus() call passes preventScroll so it cannot cancel the animation. The message summaries also gain backticks around TLS, GSSAPI, SQL, COPY and the rest, so renderInline marks them up like every other literal.
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
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.
Two things a reader saw on first load, and a simplification that came out of fixing the second.
The theme flash
ThemeToggleapplied the stored theme from an effect, so a reader who had picked light while the OS prefers dark saw the dark palette for a frame. An inline script in the head now stampsdata-themebefore anything renders.CSS already covers the unpinned case through
prefers-color-scheme, sosystemdeliberately stamps nothing and falls through to it.color-schemewas already handled in three layers, so nothing changed there.The deep-link flash
#/trust-authshowed the landing page until its capture arrived.Appderived the screen from three separate pieces of state and fell back to the landing page while a fetch was in flight.Each capture route now has a loader, so a screen does not render until its capture is in hand. This is structural rather than a guard flag.
App.tsxis gone, replaced by the route tree.Hash history, because GitHub Pages cannot rewrite an arbitrary path back to
index.html, and it is the mode the router's own docs recommend for that case.Routes gain a leading slash. A capture is now at
#/trust-authrather than#trust-auth. Nothing in the repo pointed at the old form and the sitemap lists only the root, so there was nothing internal to migrate.Dropping the message deep link
Linking to one message is removed rather than ported. It meant carrying a session id and a packet id through the router, the loaded capture and the explorer, and it was the only reason the packet list centred a row it revealed. The message index is reference data now, so its
Examplecolumn goes with it.That also fixed a real bug: arrow-keying off the bottom of the packet list jumped the next row to the middle of the viewport. Centering was deliberate, but only ever intended for the deep link. It never did anything for
Home/Endeither, since the centred position always exceededmaxScrollTopand clamped to flush anyway.The list now aligns a revealed row to the nearest edge, matching what
hexWindow.tsalready did, so a keyboard step moves one row. Two tests were added for exactly that, since the old ones only covered large jumps and so never caught it.Smoother stepping
scroll-behavior: smoothon the list, with aprefers-reduced-motionoverride. Only programmatic scrolls are affected, so the wheel and scrollbar stay native.This required
focus({ preventScroll: true })on the row. At the point the focus effect runs the row is still off-screen, sofocus()would reveal it instantly and cancel the animation.Notes for review
scrollTopToRevealderives an absolute target from the row's offset rather than a delta, so a keypress mid-animation just retargets.Home/Endnow animate across the whole list. Capped by the browser at a few hundred ms, but if it reads badly the fix is to animate only small deltas in JS instead of via CSS.MESSAGE_EXAMPLESis kept but is now imported only by its test, which guards that each entry names a packet that really is that message type. That leaves test-only data in a module whose header says "display data only", so it may belong in the test file.