fix(navbar): make the dashboard links work on every page - #110
Merged
udaycodespace merged 1 commit intoAug 27, 2026
Merged
Conversation
NavBar drove the dashboard by calling a setSelectedComponent prop that only Dashboard.jsx passed. CourseContent.jsx renders the same component bare, so on the course player — the page a student spends the whole course on — Add Course, Courses and Enrolled Courses all threw "setSelectedComponent is not a function" on click. React Router runs the caller's onClick before its own click handling, so the throw stopped the navigation too and the link was simply dead. The same three links carried no `to`. React Router resolves undefined against the current location, so they rendered as anchors pointing at the page you were already on: no middle click, no open-in-new-tab, no keyboard route that did not go through the broken handler. Home was a raw anchor, which inside a BrowserRouter is a full document load that tears down every provider. The panel is part of the URL now. The navbar navigates like any other link and takes no props, the links are real anchors, and each panel has an address that survives a reload. The panel list moves to lib/dashboardPanels, so the navbar renders from the same data the dashboard validates against instead of two lists of string literals that had to agree by hand. The old misspelled names are accepted as aliases. Also removes an unreachable `case 'cousreSection'` from the switch: nothing ever set that name, and CourseContent reads route params /dashboard does not have.
udaycodespace
self-requested a review
August 27, 2026 06:52
Owner
|
@MOHITKOURAV01 LGTM! Dashboard navigation is working properly across pages, and the tests are passing. Approved. |
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.
Summary
NavBardrove the dashboard by calling a prop onlyDashboard.jsxpassed.CourseContent.jsxrenders the same component bare, so on the course player every dashboard link threw aTypeErrorand did nothing. This moves the panel into the URL, which fixes that and three related problems at once.Related Issue
Closes #105
What changed?
frontend/src/lib/dashboardPanels.js(new) — the panel names, who may see each, and how a panel maps to and from a URL. Held as data so the navbar renders from the same source the dashboard validates against.frontend/src/components/common/NavBar.jsx— takes no props. The three role links become real<Link>anchors;Homebecomes a<Link>instead of an<a href>.frontend/src/components/common/Dashboard.jsx— reads the panel fromuseSearchParams()throughresolvePanel.frontend/src/lib/dashboardPanels.test.js(new) — 26 tests.docs/issue-105-navbar-dashboard-links.md(new) — write-up.CourseContent.jsxis deliberately untouched — two open PRs (#81, #98) are already editing that file, and it should not have to pass a prop for the navbar to work anyway.Four problems, one cause
The panel lived in one component's
useState:setSelectedComponentwasundefinedanywhere but the dashboard. React Router runs the caller'sonClickbefore its own click handling, so the throw also stopped the navigation — the link was dead, not just broken.<NavLink>elements had noto. React Router resolvesundefinedagainst the current location, so they rendered as anchors pointing at the page you were already on: no middle click, no open-in-new-tab, no keyboard route that did not go through the broken handler.Homereloaded the document. A raw<a href="/dashboard">inside aBrowserRoutertears downAuthProvider,BookmarksProviderandThemeProviderand re-fetches everything./dashboardwhile on Add Course dropped the user back to the catalogue, and there was no URL to link or share.One piece of dead code removed
Nothing could reach it —
grepfinds only thecase, no caller ever set that name — andCourseContentreads:courseIdand:courseTitlefrom the route, which/dashboarddoes not have, so it would have rendered a broken page if anything had. Removed rather than ported.Type
Areas touched
Testing
cd frontend && npm test→ 138 pass, 0 fail (112 before, 26 added).cd frontend && npm run build→ clean.npm run lintdoes not pass onmaineither — 69 pre-existing problems, almost allno-unused-varson React imports and missingreact/prop-types. This PR adds none, and removes two now-unused imports.Test steps
/courseSection/<id>/<title>, open the console, click Add Course.→ You arrive at
/dashboard?panel=addcourse. Before:Uncaught TypeError: setSelectedComponent is not a function, and nothing moved./dashboard?panel=addcourseby hand → the student catalogue, not the educator's form.Screenshots
No visual change — the same links in the same place, now working.
Edge cases checked
Other edge case details
normalizePanelreturns''for anything unrecognised, andresolvePanelfalls back tohome. Tested with?panel=../../etc/passwdand?panel=<script>.?panel=addcoursegets the catalogue. The role guards moved intocanSeePaneland are checked on render, not only on the link.canSeePanelgoes throughlib/roles, so an account still storing"Teacher"(pre-[Security]: Registration lets a client self-assign the admin role, and raw card details are persisted #55) matches, per [Bug]: The dashboard is blank for every account — the UI compares roles as "Teacher" while the API stores "teacher" #84.cousresandenrolledcoureseare accepted as aliases, so a bookmarked or in-flight value keeps working and nothing needs coordinating across a deploy.visiblePanelLinksreturns an empty list rather than throwing, and the dashboard renders<UserHome />, which already explains itself for that case.panelPathproduces is read back to the same panel byreadPanelFromSearch.Checklist
CONTRIBUTING.mdNotes
lib/dashboardPanels.jsimports./roles.jswith the extension: Vite resolves either form, butnode --testruns these modules as plain ESM and will not guess one.The navbar and the dashboard previously kept two separate lists of panel-name string literals that had to agree by hand — and three of the four were misspelled. They now read the same
PANEL_LINKS, so a panel cannot be advertised that the dashboard would refuse to open. There is a test asserting exactly that.