Use react-router for legal page tabs. #2622
Merged
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.
Fixes #2621
The primary purpose is to control the routing of the Legal page declaratively (with routes) rather than imperatively (
history.push
). Thereact-tabs
package is fine on its own but doesn't work elegantly alongsidereact-router
.Changes:
RouterTab
Tab
component fromDashboardTabSwitcher.jsx
into a reusableRouterTab
component.isSelected
prop and use thereact-router
NavLink
component which automatically detects if a link is for the current page._dashboard-header.scss
NavLink
means that thedashboard-header__tab--selected
class is now applied to the child<a>
rather than the parent<li>
, so I had to move around some styles in the.scss
file to keep things correct.RouterTab
instead of thereact-tabs
package.Legal
component into a container that renders the correct content based on props, rather than switching children.useEffect
and theHelmet
up into the parent since it is the same for all tabs.Loader
while loading. // TODO: this is not visible and should probably be a child of the PolicyContainer.PrivacyPolicy
etc. are now specific instances ofLegal
.routes.jsx
Legal
component.DashboardTabSwitcher
div
andul
tags so that theli
elements are direct children of theul
.h4
in each tab. Just<li><a/></li>
. I don't think this matters? But now that I'm writing this I think I might put it back?a
instead of aspan
.I have verified that this pull request:
npm run lint
)npm run test
)develop
branch.Fixes #123