-
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
Features integrations + mobile, copy, design tweaks #115495
Conversation
Pinging @elastic/fleet (Team:Fleet) |
...gins/fleet/public/applications/integrations/sections/epm/screens/home/available_packages.tsx
Outdated
Show resolved
Hide resolved
@@ -91,7 +91,7 @@ export const EmptyIndexListPrompt = ({ | |||
<EuiCard | |||
className="inpEmptyState__card" | |||
onClick={() => { | |||
navigateToApp('home', { path: '/app/integrations/browse' }); | |||
navigateToApp('integrations', { path: '/browse' }); |
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.
Bug from #114911
This fixes the link from the index pattern redirect.
💔 Build Failed
Failed CI StepsTest FailuresKibana Pipeline / general / X-Pack Saved Object Tagging Functional Tests.x-pack/test/saved_object_tagging/functional/tests/dashboard_integration·ts.saved objects tagging - functional tests dashboard integration editing allows to select tags for an existing dashboardStandard Out
Stack Trace
Metrics [docs]Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: |
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.
Fleet changes LGTM, and I have no qualms about hardcoding the featured integrations + their links for now.
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.
App services code change lgtm, didn't test
What should we show when Kibana doesn't have access to EPR, such as when they are running in an air gapped environment? In this case, Endpoint Security and APM won't be accessible and will result in broken links. Can we hide those cards when EPR is not accessible? Also, windows is currently our most popular observability integration. System and Fleet server are added automatically so we don't need to recommend those to users at the moment (that may change later #108456). The advantage of Windows is that its easier to set up than APM since you only need to install a single agent, and no code changes are needed to your apps. This makes it easier for an ops persona to get started with Elastic observability. What are your thoughts on swapping this for Windows @akshay-saraswat @mukeshelastic @alex-fedotyev? |
Let's handle that separately as part of #115564 ? |
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 is looking good. Left a few comments/questions for your review below.
src/plugins/kibana_react/public/page_template/no_data_page/no_data_card/elastic_agent_card.tsx
Show resolved
Hide resolved
src/plugins/kibana_react/public/page_template/no_data_page/no_data_card/no_data_card.tsx
Show resolved
Hide resolved
...gins/fleet/public/applications/integrations/sections/epm/screens/home/available_packages.tsx
Show resolved
Hide resolved
@include euiBreakpoint('xl', 'l') { | ||
.kbnStickyMenu { | ||
position: sticky; | ||
max-height: calc(100vh - #{$headerHeight + $euiSize}); |
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.
Any thoughts on applying this max-height
style regardless if the sticky condition is true? Doing so would avoid the visual "jumpiness" of the menu when transitioning between the non-sticky and sticky modes.
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've made a follow up item to track this. I think I have some better ways to do this, but given the FF timing / difficulty around getting stuff merged with tests I'm gonna punt this to a follow PR.
@elasticmachine merge upstream |
@elasticmachine merge upstream |
@elasticmachine merge upstream |
@elasticmachine merge upstream |
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 for responding to my comments/questions. Approving.
@elasticmachine merge upstream |
💛 Build succeeded, but was flaky
Test Failures
Metrics [docs]Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: |
* Features integrations + mobile, copy, design tweaks * i18n * ts fix * center button in no data card * tracking ids Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
💔 Backport failed
Successful backport PRs will be merged automatically after passing CI. To backport manually run: |
* Features integrations + mobile, copy, design tweaks * i18n * ts fix * center button in no data card * tracking ids Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Dave Snider <dave.snider@gmail.com>
Adds hardcoded featured integrations
Fixes bottom padding on stickied sidebar
Aligns empty state cards' text to left-aligned
Changed button titling to Add integrations
Fixed mobile view of integration manager