-
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
[Fleet] Implement subcategories in integrations UI #148894
Conversation
@elasticmachine merge upstream |
Pinging @elastic/fleet (Team:Fleet) |
@criamico looks like the broken tests might be related to your changes. |
Thanks, let me check that
Yes those are just mock data, it's fine. I just needed a way to have a longer list of subcategories on the top |
@hop-dev I just pushed another commit where I fix the url reading. Thanks for pointing it out, I had forgot that part! |
💚 Build Succeeded
Metrics [docs]Module Count
Public APIs missing comments
Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: cc @criamico |
Some UX feedback before I do one last code review: I do think from a usability point of view it would be nice to have an indicator of which subcategory is selected. Also, if I have selected a category under the "..." maybe it should be promoted to the front of the list, e.g here subcategory 5 is the selected category but there is no way of telling : |
I agree that we need a visual indication. I pinged @dborodyansky in the original ticket, I'm not sure which way he wants to go about this. Yours would be a good solution, another could be adding the subcategory in the search bar (Infrastructure/kubernetes), since there's enough space there. I think that this requires a bit of discussion though. |
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 left some UX feedback but that shouldn't block this PR. A couple of general questions but nothing serious
> | ||
<ControlsColumn controls={controls} title={title} /> | ||
</EuiFlexItem> | ||
<EuiFlexItem grow={5} data-test-subj="epmList.mainColumn"> |
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.
nit: Should we split some of these UI elements into smaller components and import them? This file is getting quite big
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.
It would be good, yes. I stopped there to avoid doing an even bigger PR.
@@ -0,0 +1,282 @@ | |||
/* |
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.
Is any of this hooks logic worth splitting into smaller functions and unit testing?
@kpollich should we merge this and after follow up with the discussion Cristina mentioned? |
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.
Code LGTM - I do think we need to figure out some kind of "currently selected category" state before this goes live to users, but seems that need is already addressed above. Awesome job here.
Yes let's merge this and follow up ASAP with some kind of selected state when the design is set. |
## Summary Closes elastic#147125 Closes elastic#148654 - Implement subcategories in integrations UI. - I also refactored the `AvailablePackages`, `InstalledPackages` and `PackageListGrid` components to make easier to do changes. I extracted the logic from `AvailablePackages` in a hook `useAvailablePackages` and updated it. - Sneaked a small bugfix into it. It's in commit elastic@7b2946f: removed the js logic that handled the sticky sidemenu, that would cause flickering in some cases and implemented it with some css instead. ## Repro steps ### Display actual subcategories coming from EPR - Enable `showIntegrationsSubcategories` feature flag; - Navigate to Integrations page. `Security` and `infrastructure` categories have sub categories. - When clicking on a subcategory, the page URL gets updated. For example, if clicking on `Infrastructure` > `Kubernetes`, the url will be `/integrations/browse/infrastructure/kubernetes`. It works with search too, if searching `abc` in the subcategory the url will update to `/integrations/browse/infrastructure/kubernetes?q=abc` ### Display mock data Same repro steps as before, but I added some mock subcategories: Added the following subcategories file <details> <p> ``` export const mockSubcategories = [ { id: 'kubernetes', title: 'Kubernetes', count: 18, parent_id: 'infrastructure', parent_title: 'Infrastructure', }, { id: 'message_queue', title: 'Message Broker', count: 7, parent_id: 'infrastructure', parent_title: 'Infrastructure', }, { id: 'monitoring', title: 'Monitoring', count: 16, parent_id: 'observability', parent_title: 'Observability', }, { id: 'threat_intel', title: 'Threat Intelligence', count: 10, parent_id: 'security', parent_title: 'Security', }, { id: 'web', title: 'Web Server', count: 36, parent_id: 'infrastructure', parent_title: 'Infrastructure', }, { id: 'security_mock_1', title: 'Security mock 1', count: 10, parent_id: 'security', parent_title: 'Security', }, { id: 'infrastructure_mock_1', title: 'Subcategory 1', count: 12, parent_id: 'infrastructure', parent_title: 'Infrastructure', }, { id: 'infrastructure_mock_2', title: 'Subcategory 2', count: 12, parent_id: 'infrastructure', parent_title: 'Infrastructure', }, { id: 'infrastructure_mock_3', title: 'Subcategory 3', count: 12, parent_id: 'infrastructure', parent_title: 'Infrastructure', }, { id: 'infrastructure_mock_4', title: 'Subcategory 4', count: 12, parent_id: 'infrastructure', parent_title: 'Infrastructure', }, { id: 'infrastructure_mock_5', title: 'Subcategory 5', count: 12, parent_id: 'infrastructure', parent_title: 'Infrastructure', }, { id: 'infrastructure_mock_6', title: 'Subcategory 6', count: 12, parent_id: 'infrastructure', parent_title: 'Infrastructure', }, ]; ``` </p> </details> I then imported it and replaced the subcategories here: https://github.com/criamico/kibana/blob/7b2946fe3b5067418fffb5fef9812a5080760200/x-pack/plugins/fleet/public/applications/integrations/sections/epm/screens/home/hooks/use_available_packages.tsx#L226. It should provide more subcategories under `infrastructure`. ### Screenshots If subcategories are up to 6, they get rendered on top of the integrations page: <img width="1679" alt="Screenshot 2023-01-17 at 16 11 01" src="https://user-images.githubusercontent.com/16084106/212963949-6e1fdccc-872d-4c40-a594-905e34218494.png"> If more the 6 subcategories are present, the remaining ones get hidden under an option button: <img width="1593" alt="Screenshot 2023-01-17 at 16 07 27" src="https://user-images.githubusercontent.com/16084106/212963863-0be8a97f-75ba-4bfb-a28c-bb9bbeb4c8b3.png"> <img width="1693" alt="Screenshot 2023-01-17 at 16 07 41" src="https://user-images.githubusercontent.com/16084106/212963899-2846b292-45c2-4fc2-91ca-e947653ec961.png"> ### Checklist - [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [ ] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [ ] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) - [ ] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)) Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
## Summary Now that elastic/integrations#5123 and #153216 is complete, this PR enables the integration subcategories feature by default. Users will see a much shorter list of top-level integration categories when they land in Browse Integrations. Once the user clicks into a category, subcategories will be presented if there are any (see #148894 for more details). With the associated changes now merged and enabling this flag, the list now looks like this initially: ![image](https://user-images.githubusercontent.com/1965714/227375323-bd649342-1deb-4106-aaa5-0e7573574d2d.png) And when clicking into a large category with subcategories: ![image](https://user-images.githubusercontent.com/1965714/227375436-7203aec0-48ee-4861-8a5d-af9ce37dedad.png)
## Summary Now that elastic/integrations#5123 and elastic#153216 is complete, this PR enables the integration subcategories feature by default. Users will see a much shorter list of top-level integration categories when they land in Browse Integrations. Once the user clicks into a category, subcategories will be presented if there are any (see elastic#148894 for more details). With the associated changes now merged and enabling this flag, the list now looks like this initially: ![image](https://user-images.githubusercontent.com/1965714/227375323-bd649342-1deb-4106-aaa5-0e7573574d2d.png) And when clicking into a large category with subcategories: ![image](https://user-images.githubusercontent.com/1965714/227375436-7203aec0-48ee-4861-8a5d-af9ce37dedad.png)
Summary
Closes #147125
Closes #148654
Implement subcategories in integrations UI.
I also refactored the
AvailablePackages
,InstalledPackages
andPackageListGrid
components to make easier to do changes. I extracted the logic fromAvailablePackages
in a hookuseAvailablePackages
and updated it.Sneaked a small bugfix into it. It's in commit 7b2946f: removed the js logic that handled the sticky sidemenu, that would cause flickering in some cases and implemented it with some css instead.
Repro steps
Display actual subcategories coming from EPR
showIntegrationsSubcategories
feature flag;Security
andinfrastructure
categories have sub categories.Infrastructure
>Kubernetes
, the url will be/integrations/browse/infrastructure/kubernetes
. It works with search too, if searchingabc
in the subcategory the url will update to/integrations/browse/infrastructure/kubernetes?q=abc
Display mock data
Same repro steps as before, but I added some mock subcategories:
Added the following subcategories file
It should provide more subcategories under
infrastructure
.Screenshots
If subcategories are up to 6, they get rendered on top of the integrations page:
If more the 6 subcategories are present, the remaining ones get hidden under an option button:
Checklist