278 corps castle link not redirecting correctly#281
Conversation
… Remove any logic from breadcrumb items, just use the href provided.
…usace logo in the header, just pass that href down, or if not provided bail with a `#`
|
Please set a versioning label of either |
krowvin
left a comment
krowvin
left a comment
There was a problem hiding this comment.
Seems we both took a crack at this!
I should have assigned I only set it as a child issue
Also good we both did because I did not go the route of adding the home link param. Agreed on that.
But consider the default. If you believe it should be # ?
| msgBanner = undefined, | ||
| msgBannerPosition = "top", | ||
| title = "US Army Corps of Engineers", | ||
| homeUrl = "#", |
There was a problem hiding this comment.
Should this default to #?
I would think / or nothing by default not an empty anchor tag. I realize the hash router might require it though.
| return ( | ||
| <Container fluid> | ||
| <Breadcrumbs baseUrl={BASE_URL}> | ||
| <Breadcrumbs baseUrl={`${BASE_URL}#/`}> |
There was a problem hiding this comment.
Good catch, I thought the base had the # in it for the hash router stuff
| mode === "production" | ||
| ? "https://usace.github.io/groundwork/" | ||
| : "http://localhost:5173/"; | ||
| : "http://localhost:5173/groundwork/"; |
There was a problem hiding this comment.
Makes sense. Avoids the awkward chance of missing path related issues on localhost
2 participants
Summary
Fix a couple of the link issues. This should solve the breadcrumbs not working #273 and the corps castle link issue #278 .
Checklist
patch-bump,minor-bump, ormajor-bumpRelease impact
Should not have any breaking changes, but adds to the api of SiteWrapper