-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Tech debt] Reorder Overview page #152802
[Tech debt] Reorder Overview page #152802
Conversation
…just-folder-structure-of-overview-page
@elasticmachine merge upstream |
…of github.com:CoenWarmer/kibana into chore/152787-adjust-folder-structure-of-overview-page
…just-folder-structure-of-overview-page
…just-folder-structure-of-overview-page
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.
LGTM, just one nit and a question regarding hooks defined for specific pages.
@@ -13,7 +13,7 @@ import { MetricsSection } from '../../../components/app/section/metrics'; | |||
import { UptimeSection } from '../../../components/app/section/uptime'; | |||
import { UXSection } from '../../../components/app/section/ux'; | |||
import { HasDataMap } from '../../../context/has_data_context'; | |||
import { BucketSize } from '../containers'; | |||
import { BucketSize } from '../helpers/calculate_bucket_size'; |
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.
Just to be consistent with the other identical import of this type
import { BucketSize } from '../helpers/calculate_bucket_size'; | |
import type { BucketSize } from '../helpers/calculate_bucket_size'; |
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.
Nice. Agree
import { useUiTracker } from '../../../hooks/use_track_metric'; | ||
import { ObservabilityAppServices } from '../../../application/types'; | ||
|
||
const CAPABILITIES_KEYS = ['logs', 'infrastructure', 'apm', 'uptime']; | ||
|
||
export const useOverviewMetrics = ({ hasAnyData }: { hasAnyData: boolean | undefined }) => { |
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.
Should we place hooks used only in one page, into pages/<page_name>/hooks
?
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.
Yes, agree. This particular hook however will be refactored (most likely it will go away), which is why I left it for now. Will be revisited!
…just-folder-structure-of-overview-page
💚 Build Succeeded
Metrics [docs]Module Count
Async chunks
Page load bundle
Unknown metric groupsESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: |
Resolves #152787
Summary
This PR updates the Overview Page to follow the newly agreed upon structure for the Observability app.
This PR is part of a larger effort to have all pages in the Observability app follow the same structure so they are more consistent with each other. More details in the epic: #152783
It also cleans up dead and unused components where found.
All design and functionality should be exactly the same as before.
Checklist
<to_name_of_page.tsx>
/container
subfolder if availabletranslations.ts
/app_root/components/feature
/page/<name_of_page>/components/<component_name>.tsx