Repository navigation
Route Aid Tracker's scholarship, and stop prev/next filling history - #91
Merged
Merged
Conversation
Two follow-ups from the review of #90. Aid Tracker's open scholarship was closed by a counter the module bumped on every sidebar click, because the Engine's detail next to it is a scored object out of a paginated in-memory list that an id could not rebuild. That reasoning never applied to Aid Tracker: it holds a bare id and resolves it against the tracked list, so it can be routed like the articles. It now is. That gives it the exits it was missing — a breadcrumb that names it and goes back, the browser's Back, and a refresh that keeps it — and leaves the counter needed in one place rather than two. It also removes the bespoke mechanism rather than testing it. Aid Tracker now leaves on exactly the behaviour hashRoute already covers: picking a tab clears the open page, including the tab already active. Paging through articles with prev/next wrote a history entry each, so reading Scholarships 101 end to end buried the way out under eighteen of them and Back walked back article by article instead of leaving the module. writeRoute takes a `replace`, useRoutedTab's item setter takes `{ replace: true }`, and the readers pass it for prev/next only — a step inside a page rather than a move to a new one. Opening and closing still push, because those are places you can reasonably come back to. replaceState fires no hashchange, and that event is how a module hears about a route it did not write itself, so writeRoute dispatches one.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Deploying timeline-prototype with
|
| Latest commit: |
a7a03f9
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://edebf53a.timeline-prototype.pages.dev |
| Branch Preview URL: | https://feature-reader-history-and-t.timeline-prototype.pages.dev |
The replace intent was a ref the write effect cleared, and React can skip the set that arms it — asking to replace with the value already held is a no-op, so the effect never ran and the intent survived. The next write by anyone, a tab change included, then replaced instead of pushing: the Back exit deleted by the thing meant to protect it. It now travels with the open page in one state value, so there is no set for React to skip, and nothing mutates a ref during render, which both modules were doing in their open transition. Widening the content wrapper's key to the open page reset the scroll by throwing the tab away. Aid Tracker lost its sort, its status filter and any half-filled add form every time a scholarship was opened or left, and its own scroll-to-top effect became unreachable, since each remount re-armed the first-render guard. ModuleShell already solves this for the other modules with a scrollTop effect and no remount; Financial Aid's hand-rolled shell now does the same and keys on the tab again. Removing a tracked scholarship closed its page with a push, so Back landed on the removed row's URL and did nothing visible — the list rendered, the URL still named a page, and it took two presses to leave. A dismissal is not a place to come back to, so it replaces. The breadcrumb's way back was gated on a resolved title. Articles read theirs from a static map, but a scholarship's name comes from the tracker fetch, so a refresh or a shared link had no exit until it landed and none at all when it failed. The exit now follows whether a page is open; the title only decides what it is called. Dropped the synthetic hashchange on a replace. The only two listeners are this hook's own, recomputing the state it just wrote, and the dashboard's, which reads page and module — neither of which a replace changes. It woke both for nothing, and now that the page state is a fresh object each set, waking this one costs a render. Known and left: an id the tracker cannot resolve leaves the URL naming a page while the list renders. Validating it against the tracked list would fight the async load and blank a deep link before it arrives; with the breadcrumb exit no longer tied to the title, the disagreement is cosmetic and recoverable.
This branch was successfully deployed
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.
The two follow-ups named in the review of #90.
1. Aid Tracker's scholarship is routed, not closed by a counter
#90 closed Aid Tracker's open scholarship with a counter the module bumped on
every sidebar click, justified by the Engine's detail sitting next to it being
a scored object out of a paginated in-memory list that an id could not rebuild.
That reasoning never applied to Aid Tracker. It holds a bare id and
resolves it against the tracked list, so it routes like the articles. It now
does, which gives it the exits it was missing:
The counter stays for the Scholarship Engine alone, where the id genuinely
cannot rebuild the object.
This also answers the review's "no automated test for those two detail views"
by removing the mechanism rather than testing it: Aid Tracker now leaves on
exactly the behaviour
hashRoutealready covers — picking a tab clears theopen page, including the tab already active.
2. prev/next no longer fills the history stack
Paging through articles wrote a history entry each. Reading Scholarships 101
end to end buried the way out under eighteen of them, and Back walked back
article by article instead of leaving the module.
writeRoutetakes areplace,useRoutedTab's item setter takes{ replace: true }, and the readers pass it for prev/next only — a stepinside a page rather than a move to a new one. Opening and closing still push,
since those are places you can reasonably come back to.
One subtlety worth knowing:
replaceStatefires nohashchange, and thatevent is how a module hears about a route it did not write itself, so
writeRoutedispatches one.Tests
hashRoute— replace keepshistory.lengthwhile a normal write grows it,and the item and URL still land correctly.
moduleReaderNav— paging on with next through the real reader adds noentry and still moves the URL. Verified to fail without the change.
Local:
tsc -bclean,eslintclean, 740 tests, build clean.Not covered
No render test for the Scholarship Engine's detail view — it is the one place
still using the counter, and rendering it needs a
TrackerApiand themodule's provider stack. Unchanged from #90. I also have not exercised either
change against a signed-in session; both are covered by tests and types only.