-
Notifications
You must be signed in to change notification settings - Fork 14
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
(Chore) Refactor Service metrics extension slot #7
Conversation
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.
Thanks @ODORA0 , perhaps you should have moved the extension you created from the ServiceMetrics
component to the DashboardComponent
:
<main className={styles.container}>
<ServiceMetrics />
<main className={styles.servicesTableContainer}>
<BillableServices />
</main>
</main>
You are calling the extension within the component thats the extension |
@@ -10,6 +11,7 @@ export default function BillableServicesDashboard() { | |||
<main className={styles.servicesTableContainer}> | |||
<BillableServices /> |
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.
you dont need ServiceMetrics
component since its now rendered throught the extension
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.
Ooh yeah, done!
src/routes.json
Outdated
@@ -73,7 +73,7 @@ | |||
{ | |||
"name": "billing-home-tiles-ext", | |||
"slot": "billing-home-tiles-slot", | |||
"component": "billingServicesTiles" | |||
"component": "ServiceMetrics" |
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.
The case in the index.ts doesn't match this one
export const billableServicesCardLink = getSyncLifecycle(BillableServicesCardLink, options); | ||
export const billableServicesHome = getSyncLifecycle(BillableServiceHome, options); | ||
export const billingCheckInForm = getSyncLifecycle(BillingCheckInForm, options); | ||
export const serviceMetrics = getSyncLifecycle(ServiceMetrics, options); |
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.
Align this with what u have in the routes.json
</section> | ||
); | ||
} | ||
|
||
export default React.memo(ServiceMetrics); |
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.
This doesn't look right. Remove the React.memo from here and you can use it in the logic for calculating the tile cards
Requirements
For changes to apps
If applicable
Summary
There was an issue loading the billing service metrics that would cause so many queries to the endpoint
/billableService?
until it crashes, breaking the metrics dashboard as well. This PR resolves that by refactoring the way the extension slot for the service metrics component is injectedScreenshots
Screen.Recording.2024-02-15.at.12.40.57.mov
Related Issue
Other