-
Notifications
You must be signed in to change notification settings - Fork 99
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix lint and modularize dashboard #583
Fix lint and modularize dashboard #583
Conversation
Signed-off-by: Eugene Lee <[email protected]>
…props Signed-off-by: Eugene Lee <[email protected]>
Codecov Report
@@ Coverage Diff @@
## main #583 +/- ##
=============================================
+ Coverage 53.24% 72.33% +19.08%
Complexity 271 271
=============================================
Files 236 38 -198
Lines 7779 2136 -5643
Branches 1611 236 -1375
=============================================
- Hits 4142 1545 -2597
+ Misses 3439 429 -3010
+ Partials 198 162 -36
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this change be tested with Cypress? Also, the frontend Observability workflow is failing.
Signed-off-by: Eugene Lee <[email protected]>
Signed-off-by: Eugene Lee <[email protected]>
Signed-off-by: Eugene Lee <[email protected]>
Signed-off-by: Eugene Lee <[email protected]>
Signed-off-by: Eugene Lee <[email protected]>
Signed-off-by: Eugene Lee <[email protected]>
Signed-off-by: Eugene Lee <[email protected]>
Signed-off-by: Eugene Lee <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -60,7 +60,7 @@ export const App = ({ | |||
chrome={chrome} | |||
http={http} | |||
notifications={notifications} | |||
parentBreadcrumb={parentBreadcrumb} | |||
parentBreadcrumbs={[parentBreadcrumb]} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not related to the PR but i never understood why parentBreadcrumb
needs to be passed through props previously, it's just a constant
observability/dashboards-observability/public/components/app.tsx
Lines 38 to 41 in 6d3c351
const parentBreadcrumb = { | |
text: observabilityTitle, | |
href: `${observabilityID}#/`, | |
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's to be able to keep the previous breadcrumbs and extend with the new ones for each component.
Signed-off-by: Eugene Lee <[email protected]>
Description
Issues Resolved
Breadcrumb link was broken in app analytics
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.