[Endpoint] Common PageView and LinkToApp components by paul-tavares · Pull Request #60819 · elastic/kibana

paul-tavares · 2020-03-20T21:11:55Z

Summary

This PR contains the following:

A common PageView layout component.
Ensure that all endpoint views are using a consistent layout. Currently, only the Policy List and Policy Details are using it.
A common LinkToApp component
Its a EuiLink that utilizes services.application.navigateTo() to send a user to another app's URL without triggering a full page refresh. Policy List refactored to use it.
A common useNavigateToAppEventHandler() hook. Returns back an event handler callback that can be used with onClick events when wanting to create a Link or button that navigates to another app. Currently in use by the LinkToApp component.
Removes use of NP appBasePath (deprecated) in favor or using history to initialize react router (see Add ScopedHistory to AppMountParams #56705 for details)
Fixes bug in HeaderNav where section sub-routes would not show nav item as selected (seen currently when navigating to the Policy details)
Removed hello-world API that was initially part of the first NP plugin commit
Add the Endpoint Security app icon to the plugin registration so that it is used on Kibana's side navigation.

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Unit or functional tests were updated or added to match the most common scenarios

- For details - see elastic#56705

elasticmachine · 2020-03-20T21:11:58Z

Pinging @elastic/endpoint-management (Team:Endpoint Management)

elasticmachine · 2020-03-20T21:12:00Z

Pinging @elastic/endpoint-app-team (Feature:Endpoint)

paul-tavares · 2020-03-20T21:20:59Z

Once I get some feedback on this PR, I will add test cases on to this PR :)

kevinlog · 2020-03-24T13:29:49Z

x-pack/plugins/endpoint/public/applications/endpoint/view/policy/policy_list.tsx

+        defaultMessage: 'Policies',
+      })}
+      bodyHeader={
+        <EuiText color="subdued" data-test-subj="policyTotalCount">


I think you need to adjust tests for checking for another data-test-subj, it checks for policyViewTitle now. We need to readjust

Yes, agreed. They are actually currently failing, but I wanted to hold off on fixing them until I got feedback that these changes look good to go in. :)

the PageView component certainly lgtm, might have questions about the router and hook changes still

kevinlog · 2020-03-24T13:32:36Z

x-pack/plugins/endpoint/public/applications/endpoint/index.tsx

          <KibanaContextProvider services={{ http, notifications, application, data }}>
            <EuiThemeProvider darkMode={isDarkMode}>
-              <BrowserRouter basename={basename}>
+              <Router history={history}>


why did we stop using <BrowserRouter> ?

It was a change request pushed down from the Kibana Platform - see #56705

from the ticket, looks like they still use <BrowserRouter> in the after:

core.application.register({ id: 'myApp', mount({ element, history }) { ReactDOM.render( <BrowserRouter history={history}> <App /> </BrowserRouter>, element ); return () => ReactDOM.unmountComponentAtNode(element); } });

@kevinlog Thats a mistake. BrowserRouter does not accept the history prop 😃

got it, sounds good

…n-components

aisantos · 2020-03-24T23:25:50Z

@paul-tavares - Is there a link to the main issue this PR is related to or is this the issue? It looks like this is the issue and the PR. If so, could we add the AC? So I can help with adding test cases for our team :)

paul-tavares · 2020-03-25T12:45:54Z

Hi @aisantos . I did not open an issue specifically for this, but I think I did have one on our backlog for misc. items. I'll find it and let you know.

kevinlog

LGTM based on past conversations surrounding unifying some of these navigation actions in the links. Would want another 👍 from another dev on another team - @kqualters-elastic @oatkiller etc

kqualters-elastic

component stuff lgtm, and can't think of any problems off the top of my head with the nav hook. 👍 lgtm

…n-components # Conflicts: # x-pack/plugins/endpoint/public/applications/endpoint/view/policy/policy_details.tsx

…n-components

kibanamachine · 2020-03-26T02:33:27Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 197ccd5

History

💔 Build #36375 failed 803adc8
💚 Build #36036 succeeded 525dee6
💔 Build #35640 failed ba61dc0
💔 Build #35092 failed 6fced30

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

* a common PageView component * Policy List cleanup + improvements to PageView * Policy details refactored to use PageView layout * Fix bug: header nav - ensure section sub-routes set nav to `isSelected` * Added useNavigateToAppEventHandler hook * Remove use of `appBasePath` and use `history` for initializing router - For details - see #56705 * Removed `hello-world` API * Added `LinkToApp` component + refactor policy list to use it * Add icon to plugin registration

paul-tavares added 9 commits March 20, 2020 16:46

a PageView component

b7c8e87

Policy List cleanup + improvements to PageView

61afb38

Policy details refactored to use PageView layout

0fe3432

Fix bug: header nav - ensure section sub-routes set nav to isSelected

0c36bad

Added useNavigateToAppEventHandler hook

6feed5a

Remove use of appBasePath and use history for initializing router

f90253c

- For details - see elastic#56705

Removed hello-world API

3e7d459

Added LinkToApp component + refactor policy list to use it

039ad18

add onClick support to hook + improvements to clicking

6fced30

paul-tavares added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.7.0 Team:Endpoint Management Feature:Endpoint Elastic Endpoint feature labels Mar 20, 2020

paul-tavares requested a review from a team as a code owner March 20, 2020 21:11

paul-tavares self-assigned this Mar 20, 2020

paul-tavares requested review from bkimmel, dplumlee, kevinlog, kqualters-elastic, oatkiller, parkiino and peluja1012 March 20, 2020 21:19

Fix typings + remove fake href

ba61dc0

kevinlog reviewed Mar 24, 2020

View reviewed changes

Add icon to plugin registration

6636d9d

paul-tavares added 6 commits March 24, 2020 09:59

added exported LinkToAppProps type

5c293a2

Fix functional tests

559deb9

tests for page_view

5c7049d

Updated PageView tests to use wrappingComponent

a356c20

Tests for LinkToApp component

368baf1

Merge remote-tracking branch 'upstream/master' into task/EMT-FE-commo…

525dee6

…n-components

kevinlog approved these changes Mar 25, 2020

View reviewed changes

kqualters-elastic approved these changes Mar 25, 2020

View reviewed changes

paul-tavares added 4 commits March 25, 2020 18:36

Merge remote-tracking branch 'upstream/master' into task/EMT-FE-commo…

0b2bf62

…n-components # Conflicts: # x-pack/plugins/endpoint/public/applications/endpoint/view/policy/policy_details.tsx

Fix Policy Details after merge

803adc8

Fix failing UT

a555484

Merge remote-tracking branch 'upstream/master' into task/EMT-FE-commo…

197ccd5

…n-components

paul-tavares merged commit af316c5 into elastic:master Mar 26, 2020

paul-tavares deleted the task/EMT-FE-common-components branch March 26, 2020 12:01

paul-tavares mentioned this pull request Mar 26, 2020

[7.x] [Endpoint] Common PageView and LinkToApp components (#60819) #61555

Merged

paul-tavares added v7.8.0 and removed v7.7.0 labels Mar 27, 2020

Conversation

paul-tavares commented Mar 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

Uh oh!

elasticmachine commented Mar 20, 2020

Uh oh!

elasticmachine commented Mar 20, 2020

Uh oh!

paul-tavares commented Mar 20, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aisantos commented Mar 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

paul-tavares commented Mar 25, 2020

Uh oh!

kevinlog left a comment

Choose a reason for hiding this comment

Uh oh!

kqualters-elastic left a comment

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Mar 26, 2020

💚 Build Succeeded

History

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

paul-tavares commented Mar 20, 2020 •

edited

Loading

aisantos commented Mar 24, 2020 •

edited

Loading