fix(accounts): keep the sidebar on the accounts page - #323
Open
eastagiletracker wants to merge 1 commit into
Open
fix(accounts): keep the sidebar on the accounts page#323eastagiletracker wants to merge 1 commit into
eastagiletracker wants to merge 1 commit into
Conversation
The accounts page rendered only its content with no NavBar, so navigating to /accounts dropped the sidebar and left the user with no way to move to another page. Wrap the content in the same flex + NavBar + main layout every other signed-in page uses, so the sidebar persists. Add a regression test asserting the accounts page renders the nav landmark.
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.
This PR proposes restoring the persistent sidebar on the Accounts page (
/accounts) so it no longer vanishes when you navigate there, matching the way every other signed-in page keeps it (Fixes #300). We include this PR work along with a full history of your repo at https://eastagiletracker.com/projects/306. You can sign in with your GitHub ID to claim ownership of the project.What this fixes
Every signed-in app page — Donors, Donations, Expenses, Dashboard, Reports — wraps its content in a
display: flexcontainer with<NavBar />beside a<main>, so the green sidebar stays put as you move around.apps/frontend/src/app/accounts/page.tsxwas the one exception: it returned only its<div className="!p-6">content and never rendered<NavBar />, so opening/accountsdropped the sidebar and left the user with no way to reach any other page. That is exactly what issue #300 reports ("The sidebar disappears, it should persist").The fix wraps the existing accounts content in the same flex +
<NavBar />+<main>layout the sibling pages use. The page's own markup is left untouched inside the<main>, so this is a purely additive structural change — no existing behavior or styling within the page moves.How I verified it
Reproduced on
main(HEAD1955c22) with a test that renders the page and asserts the navigation landmark is present. On the current tree it fails, because there is no sidebar to find:With the fix that test passes, and the full frontend suite stays green with no new failures against the pre-change baseline (baseline was 313 passed; this adds the one regression test).
tsc --noEmitandeslinton the changed files are both clean:The regression test lives beside the existing
AccountsPagetests and fails without this change, so it guards the behavior going forward.Heads-up on overlap
Two open PRs (#302 and #303) also touch
apps/frontend/src/app/accounts/page.tsxfor unrelated reasons (an Add-User modal and real-API wiring). Neither adds the sidebar, so this change is complementary rather than competing — but whichever of us lands first will leave a small merge conflict in that file's outer JSX wrapper for the other. Happy to rebase onto whatever lands first.How this was managed
I imported your GitHub issues and pull requests into a live agile board (249 stories from your issues and PRs, 16 labels) and tracked this fix on the story for issue #300: https://eastagiletracker.com/projects/306/stories/200370 — part of the full board at https://eastagiletracker.com/projects/306.
If you'd rather not receive contributions like this, reply
no-more-prson this pull request and we won't open any further ones on your repositories.Lawrence W. Sinclair
CEO / East Agile
linkedin.com/in/lwsinclair/
eastagile.com