Auth Web - Implements Shared Breadcrumb #1874
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1874 +/- ##
==========================================
- Coverage 79.14% 79.14% -0.01%
==========================================
Files 283 283
Lines 9729 9733 +4
Branches 303 304 +1
==========================================
+ Hits 7700 7703 +3
- Misses 2029 2030 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
|
cameron-eyds
requested review from
shabeeb-aot,
saravanpa-aot and
severinbeauvais
| @@ -0,0 +1,69 @@ | |||
| // Gray Palette | |||
There was a problem hiding this comment.
I suppose we need this file since we didn't build shared component. I think we have to prioritize compiled shared components else we will be duplicating codes. Can we add only those colour codes needed for these components in this file and remove everything else. Else it may change the existing style in sbc-auth
There was a problem hiding this comment.
Refactored this to provide an access file for our shared components that imports the theme.file from it's current location.
No changes to original theme files other than add an additional theme colour that has been introduced prior.
app-dk-blue.
There was a problem hiding this comment.
Updated in prior commit
| @@ -0,0 +1,19 @@ | |||
| import { BreadcrumbIF } from '@bcrs-shared-components/interfaces' | |||
There was a problem hiding this comment.
Is this common for all applications? if so we can import from shared components only. We don't have a resources folder yet. Can we use this file inside the util folder for better structure? This will be one file inside folder
There was a problem hiding this comment.
This isn't common for all as of yet.
I believe once the Uber Dashboard or highest level landing/marketing page is complete, we can then then create a common resource. Could look at doing this now though, whatever you prefer.
There was a problem hiding this comment.
To follow up on this, it will be tricky as until each UI has a set configuration object.
Currently this differs between them.
ie the filings UI refers to Auth Web home as the BUSINESS_URL and auth web refers to itself as AUTH_WEB_URL etc.
So in order to make 1 common resource we would either need..
a) all the configuration objects to follow a standard naming convention
b) have local customization of these resourced objects (which we sort of land us in the same spot we are now anywho)
| component: IncorpOrRegisterView, | ||
| meta: { showNavBar: false } | ||
| meta: { | ||
| breadcrumb: [ |
There was a problem hiding this comment.
will it be better to keep breadcrumb in one place? either in resources file (like in line 213 breadcrumb: [DashboardBreadcrumb]) or inline like this?
There was a problem hiding this comment.
It can be done like that when the crumb is common but each of these routes require a unique to defined.
Not sure exactly what you mean.
Do you want to define all of these in a resource and import into the routes?
There was a problem hiding this comment.
I think in these cases it's ok in the routes but if we were to re use these specific crumbs it would make sense to make it a resource. I just can't think of where else we would reuse at this time.
There was a problem hiding this comment.
In someplace, used as breadcrumb: [DashboardBreadcrumb], and other places using only text from resources (text: HomeBreadCrumb.text). So I was thinking to use one pattern. or even if we are using only text, better to add in the language file and use it in both places. (ie in resource text and HomeBreadCrumb.text). I think its fine now as you mentioned above :)
There was a problem hiding this comment.
Ahh i see you what you mean, absolutely that makes sense.
|
Kudos, SonarCloud Quality Gate passed!
|
| @@ -0,0 +1 @@ | |||
| @import "$assets/scss/theme.scss"; | |||
There was a problem hiding this comment.
This is good for now. Once we minify and build @bcrs-shared-components , we can avoid this pattern 👍
There was a problem hiding this comment.
Awesome, yea i look forward to learning more about that approach!
Issue #:
bcgov/entity#10236
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 sbc-auth license (Apache 2.0).