[To Main] Feature - DESENG-627 use createBrowserRouter() in-app #2534
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2534 +/- ##
==========================================
- Coverage 76.00% 75.93% -0.07%
==========================================
Files 604 603 -1
Lines 21785 21755 -30
Branches 1663 1667 +4
==========================================
- Hits 16557 16519 -38
- Misses 4965 4973 +8
Partials 263 263
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
| @@ -0,0 +1,27 @@ | |||
| import React from 'react'; | |||
| import '@bcgov/design-tokens/css-prefixed/variables.css'; // Will be available to use in all component | |||
There was a problem hiding this comment.
I think I know what this comment is trying to say, but want to confirm. Is it:
| import '@bcgov/design-tokens/css-prefixed/variables.css'; // Will be available to use in all component | |
| import '@bcgov/design-tokens/css-prefixed/variables.css'; // Variables will be within scope within PublicLayout and all its child components. |
| * @param smallScreenOnly If true, only displays the breadcrumbs on small screens. | ||
| * @returns A list of breadcrumbs. | ||
| */ | ||
| export const AutoBreadcrumbs: React.FC<{ smallScreenOnly?: boolean }> = ({ smallScreenOnly }) => { |
There was a problem hiding this comment.
Thanks for this great component!
| key={match.pathname} | ||
| fallback={ | ||
| <BodyText bold size="small"> | ||
| Loading... |
There was a problem hiding this comment.
You know how this functions the best, but I'm just curious why the breadcrumbs have a loading indicator
|
|
||
| <Header1>Tenant Admin</Header1> | ||
| <Grid container spacing={0} direction="row" mb="0.5em"> | ||
| <Grid item xs={12} sm={7} lg={9}> | ||
| <Header2 decorated>Tenant Instances {!loading && `(${tenants.length})`}</Header2> | ||
| <Header2 decorated> |
There was a problem hiding this comment.
Although this isn't as much a concern now, if this platform is offered to folks with varying accessibility needs, we should bear in mind that headings are typically used to provide summaries of sections.
If the goal is just to display the number of tenant instances very prominently, I'd recommend just using a span and styling it to be large. A person using a screen reader will construe this as a description of the section that follows and may be confused as a result.
I also recognize that you didn't add this originally but it'd be something small to change if you get a sec!
| onClick={() => { | ||
| navigate(`/tenantadmin/${tenant.short_name}/detail`); | ||
| }} | ||
| onKeyDown={(event) => { |
There was a problem hiding this comment.
Thanks for thinking of keyboard users!
Issue #: https://github.com/bcgov/met-public/issues/
Description of changes:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the met-public license (Apache 2.0).