Add pluggable panel action tests #20163
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,115 @@ | ||
| /* | ||
| * Licensed to Elasticsearch B.V. under one or more contributor | ||
| * license agreements. See the NOTICE file distributed with | ||
| * this work for additional information regarding copyright | ||
| * ownership. Elasticsearch B.V. licenses this file to you under | ||
| * the Apache License, Version 2.0 (the "License"); you may | ||
| * not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, | ||
| * software distributed under the License is distributed on an | ||
| * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
| * KIND, either express or implied. See the License for the | ||
| * specific language governing permissions and limitations | ||
| * under the License. | ||
| */ | ||
|
|
||
| import React from 'react'; | ||
|
|
||
| import { EuiFlyout } from '@elastic/eui'; | ||
| import { EventEmitter } from 'events'; | ||
| import ReactDOM from 'react-dom'; | ||
|
|
||
| let activeSession: FlyoutSession | null = null; | ||
|
|
||
| const CONTAINER_ID = 'flyout-container'; | ||
|
|
||
| function getOrCreateContainerElement() { | ||
| let container = document.getElementById(CONTAINER_ID); | ||
| if (!container) { | ||
| container = document.createElement('div'); | ||
| container.id = CONTAINER_ID; | ||
| document.body.appendChild(container); | ||
| } | ||
| return container; | ||
| } | ||
|
|
||
| /** | ||
| * An FlyoutSession describes the session of one opened flyout panel. It offers | ||
| * methods to close the flyout panel again. If you open a flyout panel you should make | ||
| * sure you call {@link FlyoutSession#close} when it should be closed. | ||
| * Since a flyout could also be closed without calling this method (e.g. because | ||
| * the user closes it), you must listen to the "closed" event on this instance. | ||
| * It will be emitted whenever the flyout will be closed and you should throw | ||
| * away your reference to this instance whenever you receive that event. | ||
| * @extends EventEmitter | ||
| */ | ||
| class FlyoutSession extends EventEmitter { | ||
| /** | ||
| * Binds the current flyout session to an Angular scope, meaning this flyout | ||
| * session will be closed as soon as the Angular scope gets destroyed. | ||
| * @param {object} scope - And angular scope object to bind to. | ||
|
There was a problem hiding this comment. Typo: And -> An (or The) |
||
| */ | ||
| public bindToAngularScope(scope: ng.IScope): void { | ||
| const removeWatch = scope.$on('$destroy', () => this.close()); | ||
| this.on('closed', () => removeWatch()); | ||
| } | ||
|
|
||
| /** | ||
| * Closes the opened flyout as long as it's stil the open one. | ||
|
|
||
| * If this is not the active session anymore, this method won't do anything. | ||
| * If this session was still active and a flyout was closed, the 'closed' | ||
| * event will be emitted on this FlyoutSession instance. | ||
| */ | ||
| public close(): void { | ||
| if (activeSession === this) { | ||
| const container = document.getElementById(CONTAINER_ID); | ||
| if (container) { | ||
| ReactDOM.unmountComponentAtNode(container); | ||
| this.emit('closed'); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Opens a flyout panel with the given component inside. You can use | ||
| * {@link FlyoutSession#close} on the return value to close the flyout. | ||
| * | ||
| * @param flyoutChildren - Mounts the children inside a fly out panel | ||
| * @return {FlyoutSession} The session instance for the opened flyout panel. | ||
| */ | ||
| export function openFlyout( | ||
| flyoutChildren: React.ReactNode, | ||
| flyoutProps: { | ||
|
There was a problem hiding this comment. If possible, let's switch the order to first pass in the children. |
||
| onClose?: () => void; | ||
| 'data-test-subj'?: string; | ||
| } = {} | ||
| ): FlyoutSession { | ||
| // If there is an active inspector session close it before opening a new one. | ||
| if (activeSession) { | ||
| activeSession.close(); | ||
| } | ||
| const container = getOrCreateContainerElement(); | ||
|
There was a problem hiding this comment. I have a few thoughts about this. Should EuiFlyout itself just coordinate this internally, essentially acting like a singleton and ensuring only one flyout is ever visible at a time? It almost seems like that would be a sensible design. Alternatively, we could do that ourselves, and keep this inside the React lifecycle by just making it a plain React component. Something like this, maybe: // Something like this
let currentFlyout = null;
class SingletonFlyout {
componentWillMount() {
if (currentFlyout) {
currentFlyout.close();
}
currentFlyout = {
flyout: this,
close: this.props.onClose,
};
}
componentWillUnmount() {
if (currentFlyout && currentFlyout.flyout === this) {
currentFlyout = null;
}
}
render() {
return (
<EuiFlyout>
{this.props.children}
<EuiFlyout>
);
}
}The advantage there is that consumers can use it like any other React component. It'd be styled to have a fixed position, etc, but could show/ hide based on props / state changes without special cases or whatever. It'd be consumed like this: // in some stateful component's render...
render() {
return (
<div>
My usual component.
<button onClick={() => this.setState({ isFlying: true })}>
Fly, little bird!
</button>
{!this.state.isFlying ? null :
<SingletonFlyout onClose={() => this.setState({ isFlying: false })}>
<h1>Hi!</h1>
</SingletonFlyout>
}
</div>
);
}Or something. You could pass it an Angular scope as an optional param, too, if that's a case that needs handling. The advantage to either of those ^^^ approaches is that the lifecycle of the flyout is normal React stuff, no special DOM manipulation or possible leaking edge cases. If we keep the current solution (which is fine by me), it seems like it might be nice to optionally pass the container in, so that callers could pass a container that is managed by React / destroyed by React and not have to worry about cleanup or extraneous references. There was a problem hiding this comment. I prefer your first solutions - having flyout be a singleton. I can't think of any good situation where we should have two flyouts. I would prefer to leave it as is though in this PR, just for the sake of unblocking some things (Tim wants to add inspector tests, I want to extend the panel action tests, and we both want to generalize this approach to all plugins not just panels). I think we'll have to address the flyout changes soon, though, as what should happen if you open a panel action with a flyout while an inspector is open? |
||
| const session = (activeSession = new FlyoutSession()); | ||
| const onClose = () => { | ||
| if (flyoutProps.onClose) { | ||
| flyoutProps.onClose(); | ||
| } | ||
| session.close(); | ||
| }; | ||
|
|
||
| ReactDOM.render( | ||
| <EuiFlyout {...flyoutProps} onClose={onClose}> | ||
| {flyoutChildren} | ||
| </EuiFlyout>, | ||
| container | ||
| ); | ||
|
|
||
| return session; | ||
| } | ||
|
|
||
| export { FlyoutSession }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| /* | ||
| * Licensed to Elasticsearch B.V. under one or more contributor | ||
| * license agreements. See the NOTICE file distributed with | ||
| * this work for additional information regarding copyright | ||
| * ownership. Elasticsearch B.V. licenses this file to you under | ||
| * the Apache License, Version 2.0 (the "License"); you may | ||
| * not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, | ||
| * software distributed under the License is distributed on an | ||
| * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
| * KIND, either express or implied. See the License for the | ||
| * specific language governing permissions and limitations | ||
| * under the License. | ||
| */ | ||
|
|
||
| export * from './flyout_session'; |
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.
Typo: An -> A