Skip to content
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

Update navigation landing pages to use appLinks config #132027

Merged
merged 8 commits into from
May 17, 2022

Conversation

machadoum
Copy link
Member

@machadoum machadoum commented May 11, 2022

figma

Summary

Update navigation landing pages to use appLinks config.
It also adds description and image to the Detection Response app link.

  • Wait for design to provide a better screenshot for Detection Response
  • Wait for final page descriptions

User mock
Screenshot 2022-05-11 at 15 17 14

Manage mock
Screenshot 2022-05-17 at 10 32 55

Checklist

Delete any items that are not applicable to this PR.

@machadoum machadoum force-pushed the siem-explore-issue-130565-4 branch 2 times, most recently from db509c8 to 208e065 Compare May 11, 2022 13:12
@machadoum machadoum marked this pull request as ready for review May 11, 2022 13:20
@machadoum machadoum requested review from a team as code owners May 11, 2022 13:20
@machadoum machadoum self-assigned this May 11, 2022
@machadoum machadoum added Team:Threat Hunting Security Solution Threat Hunting Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Threat Hunting:Explore v8.3.0 labels May 11, 2022
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-solution (Team: SecuritySolution)

@elasticmachine
Copy link
Contributor

Pinging @elastic/security-threat-hunting (Team:Threat Hunting)

@machadoum machadoum added the release_note:skip Skip the PR/issue when compiling release notes label May 11, 2022
@machadoum machadoum force-pushed the siem-explore-issue-130565-4 branch from 208e065 to fe2215f Compare May 11, 2022 15:52
@machadoum machadoum marked this pull request as draft May 12, 2022 08:47
@machadoum machadoum force-pushed the siem-explore-issue-130565-4 branch 2 times, most recently from b684e3d to ac690bc Compare May 12, 2022 11:07
@machadoum machadoum force-pushed the siem-explore-issue-130565-4 branch 2 times, most recently from 220ede4 to 5599a96 Compare May 12, 2022 11:57
@machadoum machadoum force-pushed the siem-explore-issue-130565-4 branch from 5599a96 to b8b145e Compare May 12, 2022 13:42
@machadoum machadoum marked this pull request as ready for review May 12, 2022 15:46
@machadoum machadoum requested a review from semd May 12, 2022 15:53
Copy link
Contributor

@semd semd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 🚀 only that NIT about the hook encapsulation.
Thanks Pablo!

@machadoum machadoum force-pushed the siem-explore-issue-130565-4 branch from 3dd4fba to 1a4602f Compare May 16, 2022 09:31
@semd semd force-pushed the siem-explore-issue-130565-4 branch from 1a4602f to 1449681 Compare May 16, 2022 10:19
@machadoum machadoum force-pushed the siem-explore-issue-130565-4 branch from 1449681 to c151576 Compare May 16, 2022 10:26
@machadoum
Copy link
Member Author

machadoum commented May 16, 2022

I am assuming this placeholder text is yet to be updated.

Yes, @ashokaditya. I am still waiting for the design team to provide three two remaining descriptions.

@machadoum machadoum force-pushed the siem-explore-issue-130565-4 branch from 663f433 to af101f4 Compare May 16, 2022 16:29
@machadoum machadoum force-pushed the siem-explore-issue-130565-4 branch from af101f4 to 67ed319 Compare May 16, 2022 16:31
@machadoum machadoum requested a review from ashokaditya May 16, 2022 16:35
Copy link
Contributor

@dasansol92 dasansol92 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, this looks great! Left some more questions, let me know if anything is not clear at all 🙂 Thanks for doing all these changes! 🔥

itemIds: SecurityPageName[];
}

export const MANAGE_NAVIGATION_CATEGORIES: LandingNavGroup[] = [
Copy link
Contributor

@dasansol92 dasansol92 May 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this used somewhere? Does it means rules and expections will be under Management section?

Are the itemIds array sorted? If they are, itemIds for Endpoints should be like this I guess:

[
      SecurityPageName.endpoints,
      SecurityPageName.policies,
      SecurityPageName.trustedApps,
      SecurityPageName.eventFilters,
      SecurityPageName.hostIsolationExceptions,
      SecurityPageName.blocklist,
    ]

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this used somewhere? Does it means rules and expections will be under Management section?

It is used for the menu landing page. Yes, rules and exceptions are located on the Manage landing page.

Screenshot 2022-05-17 at 10 32 55


Are the itemIds array sorted? If they are, itemIds for Endpoints should be like this I guess:

hmm, I see that on your list, hostIsolationExceptions comes before blocklist. But in the design, it is the other way around.

Screenshot 2022-05-17 at 10 09 27

@bfishel Is it ok if we follow the order proposed by @dasansol92 instead the one on Figma?

Copy link
Member

@ashokaditya ashokaditya May 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, it is Blocklist and Event filters that have messages swapped 😅 @bfishel @machadoum

Copy link
Member

@ashokaditya ashokaditya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the chagnes! I have a couple more suggestions. Plus what @dasansol92 pointed out.

x-pack/plugins/security_solution/public/overview/links.ts Outdated Show resolved Hide resolved
Comment on lines +20 to +21
'A comprehensive overview of user data that enables understanding of authentication and user behavior within your environment.',
}),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice!!

@machadoum
Copy link
Member Author

machadoum commented May 17, 2022

Hey @ashokaditya and @dasansol92, Thanks for your review and for catching that wrong description. 👏 👏

I implemented all the suggestions except for hostIsolationExceptions and blocklist because I need to check with Design first.

Copy link
Contributor

@dasansol92 dasansol92 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OLM changes LGTM! Only need to clarify the sorting of management items but it can be done in a subsequent pr if this needs to be merged. Thanks for putting all of this together!

path: BLOCKLIST_PATH,
skipUrlState: true,
},
],
};

export const navigationCategories: NavigationCategories = [
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: is this exactly the same as the one in landing_pages/constants, if so, could we unify them?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, good catch, that was intentionally moved to your team folder, It needs to be removed from landing_pages/constants, will do in my next PR 👍

@semd
Copy link
Contributor

semd commented May 17, 2022

@dasansol92 @ashokaditya This PR is blocking me with this. Since the order of two links is the only issue remaining, would it be possible to merge this one, and if @bfishel decides to change it, we create a small separate PR for it? Thanks

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
securitySolution 2920 2934 +14

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
securitySolution 5.0MB 5.0MB +8.5KB
Unknown metric groups

miscellaneous assets size

id before after diff
securitySolution 1.7MB 1.7MB +35.0KB

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @machadoum

@machadoum machadoum merged commit 37e4628 into elastic:main May 17, 2022
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label May 17, 2022
academo pushed a commit to XavierM/kibana that referenced this pull request May 17, 2022
* Update navigation landing pages to use appLinks config

* Please code review

* align app links changes

* Update links descriptions

* Rollback title changes

* Fix wrong links descriptions

* Fix unit tests

* Fix description

Co-authored-by: semd <sergi.massaneda@elastic.co>
@bfishel
Copy link

bfishel commented May 17, 2022

good on the order that @dasansol92 proposed!

XavierM added a commit that referenced this pull request May 17, 2022
* wip I

* add alert table state in case

* [CI] Auto-commit changed files from 'node scripts/eslint --no-cache --fix'

* add new API to get FeatureID form registrationContext and update UI to use this new API

* rm dead code

* [CI] Auto-commit changed files from 'node scripts/eslint --no-cache --fix'

* remove unnecessary memo

* adds tests for case view helpers

* Move http call to API and add tests for getFeatureIds

* fix type + unit test

* add unit tests + cleanup

* add new api integration test for _feature_ids

* [CI] Auto-commit changed files from 'node scripts/eslint --no-cache --fix'

* Fix small type creating typescript slowness

* remove console log

* use import type for validfeatureId

* force any to improve typescript performance

* Update APM (#132270)

Co-authored-by: Renovate Bot <bot@renovateapp.com>

* [ResponseOps][Docs] Updating ServiceNow docs with OAuth setup instructions (#131344)

* Updating ServiceNow docs. Need screenshots

* Adding screenshots

* Fix nested screenshots and lists

* Tweaks and screenshots

* Updates

* blergh

* Apply suggestions from code review

Co-authored-by: Lisa Cawley <lcawley@elastic.co>

* Apply suggestions from code review

Co-authored-by: Mike Côté <mikecote@users.noreply.github.com>

Co-authored-by: lcawl <lcawley@elastic.co>
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Mike Côté <mikecote@users.noreply.github.com>

* Show polling options when 'Data streams' option is selected in the Console Settings modal. (#132277)

* [Osquery] Make Osquery All with All base privillege (#130523)

* [XY] Add normalizeTable function to correct works with esdocs (#131917)

* Add normalizeTable function to correct works with esdocs

* Fix types

* Fix types

* Fix CI

* Fix CI

* Some fixes

* Remove fallback with min/max value for domain

* Added tests

* Some refactoring

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Yaroslav Kuznietsov <kuznetsov.yaroslav.yk@gmail.com>

* [Osquery] Add default osquery_saved_query objects (#129461)

* [Unified Search] Show error message for invalid date filter value (#131290)

* feat: added show error message for invalid date

* refact: move logic in HOC

* feat: refactoring code and added translation

* refact show error

* refact: show error message

* refact: remove translation

* refactor: changed menu for show FilterEdit

* fix: open/close popover

* feat: field.type => KBN_FIELD_TYPES

* feat: remove extra code with with input check and refactored filter item

* feat: added tests and refactoring code

* refact: getFieldValidityAndErrorMessage

* feat: return isInvalid checking in valur input type for string, ip

* Update navigation landing pages to use appLinks config (#132027)

* Update navigation landing pages to use appLinks config

* Please code review

* align app links changes

* Update links descriptions

* Rollback title changes

* Fix wrong links descriptions

* Fix unit tests

* Fix description

Co-authored-by: semd <sergi.massaneda@elastic.co>

* [Cloud Posture] add resource findings page flyout  (#132243)

* [Discover] Add a tour for Document Explorer (#131125)

* [Discover] Add "Take a tour" button to the Document Explorer callout

* [Discover] Tmp

* [Discover] Add a first Document Explorer tour step

* [Discover] Add other Document Explorer tour steps

* [Discover] Update tour steps positioning

* [Discover] Add gifs to tour steps

* [Discover] Refactor how tour steps are registered

* [Discover] Add new step to the tour. Update tour steps text.

* [Discover] Improve steps positioning

* [Discover] Fix positioning for Add field step

* [Discover] Add icons to tour steps

* [Discover] Reorganize components

* [Discover] Skip Columns step when it's not available

* [Discover] Rename components

* [Discover] Add some tests

* [Discover] Fix positioning

* [Discover] Fix props

* [Discover] Render steps only if the tour is active

* [Discover] Update gifs

* [Discover] Add image alt text for gifs

* [Discover] Tag the Take tour button

* [Discover] Update text and tests

* [Discover] Add more tests

* [Discover] Rename assets directory

* [Discover] Fix tour in mobile view. Improve steps positioning and animation.

* [Discover] Update text in tour steps

* [Discover] Update sort.gif

* [Discover] Update image width

* Update src/plugins/discover/public/components/discover_tour/discover_tour_provider.tsx

Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com>

* Update src/plugins/discover/public/components/discover_tour/discover_tour_provider.tsx

Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com>

* [Discover] Update sort.gif

* [Discover] Fix code style

Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com>
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

* [XY] Add `minTimeBarInterval` arg (#128726)

* Added `xAxisInterval` arg

* Add validation

* Add tests

* Rename xAxisInterval to minTimeBarInterval and add validation

* Fix imports

* Add tests to validation

* Fix conflicts

* [CI] Auto-commit changed files from 'node scripts/precommit_hook.js --ref HEAD~1..HEAD --fix'

* Fix tests

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

* do not use barrel imports

* do not use barrel import

* do not use barrel import

* do not use barrel imports

* do not use barrel import

* import types

* Add tests

* Fix cases bundle size

* Add more tests

* [Fleet] Add new API to get current upgrades (#132276)

* Add support of Data View switching for Agg-Based visualizations (#132184)

* Add support of Data View switching for Agg-Based visualizations

* fix CI

* add use_date_view_updates

* implement sync with state

* cleanup

* cleanup

* cleanup

* Update index.ts

* fix PR comments

* Update use_data_view_updates.ts

* Update use_data_view_updates.ts

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

* [Security Solution] Responsive styling fixes (#131951)

* [Discover] Add Analytics No Data Page (#131965)

* [Discover] Add Analytics No Data Page

* Make showEmptyPrompt parameter optional

* Remove unused import

* Remove unnecessary test

* Fix test

* Update failing test?

* Update failing test

* Changing the order of functional tests

* Fix error handling

* Addressing PR comments

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

* Remove barrel export from public index file

* remove barrel export

* Re-export missing exports

* Turn off feature flag

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Esteban Beltran <esteban.beltran@elastic.co>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Renovate Bot <bot@renovateapp.com>
Co-authored-by: Ying Mao <ying.mao@elastic.co>
Co-authored-by: lcawl <lcawley@elastic.co>
Co-authored-by: Mike Côté <mikecote@users.noreply.github.com>
Co-authored-by: CJ Cenizal <cj.cenizal@elastic.co>
Co-authored-by: Tomasz Ciecierski <ciecierskitomek@gmail.com>
Co-authored-by: Uladzislau Lasitsa <Uladzislau_Lasitsa@epam.com>
Co-authored-by: Yaroslav Kuznietsov <kuznetsov.yaroslav.yk@gmail.com>
Co-authored-by: Nodir Latipov <nodir.latypov@gmail.com>
Co-authored-by: Pablo Machado <pablo.nevesmachado@elastic.co>
Co-authored-by: semd <sergi.massaneda@elastic.co>
Co-authored-by: Or Ouziel <or.ouziel@elastic.co>
Co-authored-by: Julia Rechkunova <julia.rechkunova@elastic.co>
Co-authored-by: gchaps <33642766+gchaps@users.noreply.github.com>
Co-authored-by: Christos Nasikas <christos.nasikas@elastic.co>
Co-authored-by: Nicolas Chaulet <nicolas.chaulet@elastic.co>
Co-authored-by: Alexey Antonov <alexwizp@gmail.com>
Co-authored-by: Steph Milovic <stephanie.milovic@elastic.co>
Co-authored-by: Maja Grubic <maja.grubic@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Threat Hunting:Explore Team:Threat Hunting Security Solution Threat Hunting Team v8.3.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants