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

[Endpoint] add react router to endpoint app #53808

Merged
merged 12 commits into from
Jan 2, 2020
43 changes: 38 additions & 5 deletions x-pack/plugins/endpoint/public/applications/endpoint/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,24 +8,57 @@ import * as React from 'react';
import ReactDOM from 'react-dom';
import { CoreStart, AppMountParameters } from 'kibana/public';
import { I18nProvider, FormattedMessage } from '@kbn/i18n/react';
import { History, createHashHistory } from 'history';
import { Route, Router, Switch } from 'react-router-dom';

/**
* This module will be loaded asynchronously to reduce the bundle size of your plugin's main bundle.
*/
export function renderApp(coreStart: CoreStart, { element }: AppMountParameters) {
coreStart.http.get('/api/endpoint/hello-world');

ReactDOM.render(<AppRoot />, element);
const history = createHashHistory();
Copy link
Contributor

Choose a reason for hiding this comment

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

See my comment further below - this might not be needed if we use React-router's HashRouter component

ReactDOM.render(<AppRoot history={history} />, element);

return () => {
ReactDOM.unmountComponentAtNode(element);
};
}

const AppRoot = React.memo(() => (
interface RouterProps {
history: History;
}

const AppRoot: React.FC<RouterProps> = React.memo(({ history }) => (
Copy link
Contributor

Choose a reason for hiding this comment

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

FYI - just noticed in the TS typing definition that React.FC is marked as @deprecated and we should use FunctionComponent instead (at least in the version of React that is being used in the feature-ingest branch 🙂 )

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch, fixed 👍

<I18nProvider>
<h1 data-test-subj="welcomeTitle">
<FormattedMessage id="xpack.endpoint.welcomeTitle" defaultMessage="Hello World" />
</h1>
<Router history={history}>
Copy link
Contributor

@paul-tavares paul-tavares Dec 27, 2019

Choose a reason for hiding this comment

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

Consider using React Router's HashRouter instead --- until we can figure out how to use the Functional test framework with "real" URLs and then switch over to react-router's BrowserRotuer component.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could also probably write a helper function to just navigate us to a real url. Other plugins in Kibana all seem to use hashes and I just wanna understand why.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the "old" platform client side framework was hash based so that might be the reason why all (most) current functional test use it. 🤷‍♂

Copy link
Contributor

Choose a reason for hiding this comment

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

Now that we figured out support for "real" URLs, consider using react-router's <BrowserRouter /> component instead, which avoid having to create/manage a history instance ourselves

<Switch>
<Route
exact
path="/"
render={() => (
<h1 data-test-subj="welcomeTitle">
<FormattedMessage id="xpack.endpoint.welcomeTitle" defaultMessage="Hello World" />
</h1>
)}
/>
<Route
path="/management"
render={() => (
<h1 data-test-subj="endpointManagement">
<FormattedMessage
id="xpack.endpoint.endpointManagement"
defaultMessage="Manage Endpoints"
/>
</h1>
)}
/>
<Route
render={() => (
<FormattedMessage id="xpack.endpoint.notFound" defaultMessage="Page Not Found" />
)}
/>
</Switch>
</Router>
</I18nProvider>
));
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,15 @@ export default function({ getPageObjects, getService }: FtrProviderContext) {
});
await testSubjects.existOrFail('welcomeTitle');
});

it(`endpoint management shows 'Manage Endpoints'`, async () => {
await pageObjects.common.navigateToActualUrl('endpoint', '/management', {
basePath: '/s/custom_space',
ensureCurrentUrl: false,
shouldLoginIfPrompted: false,
});
await testSubjects.existOrFail('endpointManagement');
});
});

describe('space with endpoint disabled', () => {
Expand Down