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

[Runtime field editor] Preview field against cluster data #101398

Merged
Show file tree
Hide file tree
Changes from 44 commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
17f94dc
Add field preview header and empty prompt
sebelga May 31, 2021
e96e33f
Add preview document navigation
sebelga May 31, 2021
cd80e34
Add basic error handling when fetching documents
sebelga Jun 1, 2021
9648e3e
Add field list and field list item
sebelga Jun 2, 2021
94aaf37
Use virtual list to render fields
sebelga Jun 2, 2021
d265e1b
Expose panel height to consumers
sebelga Jun 2, 2021
60579cd
Make virtual field list height dynamic
sebelga Jun 2, 2021
b6b0eb6
Move filtering of fields to Preview component
sebelga Jun 4, 2021
17c9a22
Render current field being created
sebelga Jun 4, 2021
9f88c22
Add updating preview indicator in header
sebelga Jun 4, 2021
a8d79a0
Update field list rendering
sebelga Jun 7, 2021
6732fdd
Add focus to the field editor flyout
sebelga Jun 7, 2021
c31b315
Fix tests
sebelga Jun 7, 2021
e5395a0
Set fixed width to panel and move preview to the right
sebelga Jun 8, 2021
f9a6fad
[Form lib] Add useFormIsModified() hook
sebelga Jun 15, 2021
5fea64e
Hide flyout close button and show confirm modal when cancel
sebelga Jun 15, 2021
64de3e8
Reduce list item height + bolding field being previewed
sebelga Jun 15, 2021
3e31852
Show _source value when no script is provided
sebelga Jun 15, 2021
5b09833
Fix typo
sebelga Jun 15, 2021
798d4a9
Refactor form lib UseField test
sebelga Jun 16, 2021
e581514
Apply suggestions from code review
sebelga Jun 16, 2021
97c18a2
Use better naming for painless error testBed
sebelga Jun 16, 2021
4c05e3a
Address CR changes
sebelga Jun 16, 2021
a7990ce
Merge branch 'runtime-field-editor/preview-from-cluster-2' of github.…
sebelga Jun 16, 2021
1970973
Revert changes to flyout_service and add missing prop
sebelga Jun 16, 2021
230100b
Create component for confirm modal when saving field with name or typ…
sebelga Jun 16, 2021
c5c5d73
Fix issue with stale state after fetching unknown document
sebelga Jun 17, 2021
5e677e8
Fix "updating..." state not being reset
sebelga Jun 17, 2021
0386a9b
Move change to isModified into dedicated useEffect
sebelga Jun 17, 2021
6febb54
Merge branch 'runtime-field-editor/preview-field-workstream' into run…
sebelga Jun 17, 2021
bdbe126
Fix TS issue
sebelga Jun 17, 2021
639b381
Add missing core public API
sebelga Jun 17, 2021
871b065
Fix TS issue
sebelga Jun 17, 2021
751a0d1
Update a11y tests
sebelga Jun 17, 2021
7b69cc4
Refactor logic to fetch document to fix tests
sebelga Jun 18, 2021
2d3e21e
Discard _meta form field to detect if the form has been modified
sebelga Jun 18, 2021
2072ebe
Address CR changes
sebelga Jun 18, 2021
ba0605a
Fix functional test
sebelga Jun 18, 2021
83007f7
Merge branch 'runtime-field-editor/preview-field-workstream' into run…
sebelga Jun 18, 2021
3be4625
Merge branch 'runtime-field-editor/preview-field-workstream' into run…
sebelga Jun 21, 2021
ca73545
Improve logic to set form isModified for fields removed from the DOM
sebelga Jun 21, 2021
8f33eb0
Display confirm modal when clicking outside the flyout
sebelga Jun 21, 2021
f7cf0ed
Add "onClose" public API
sebelga Jun 21, 2021
6b4b8d1
Refactor overlay onClose option to provide the flyout instance
sebelga Jun 21, 2021
eea8a9a
Add doc for overlay onClose option
sebelga Jun 21, 2021
400550f
Merge branch 'runtime-field-editor/preview-field-workstream' into run…
sebelga Jun 22, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<!-- Do not edit this file. It is automatically generated by API Documenter. -->

[Home](./index.md) &gt; [kibana-plugin-core-public](./kibana-plugin-core-public.md) &gt; [OverlayFlyoutOpenOptions](./kibana-plugin-core-public.overlayflyoutopenoptions.md) &gt; [hideCloseButton](./kibana-plugin-core-public.overlayflyoutopenoptions.hideclosebutton.md)

## OverlayFlyoutOpenOptions.hideCloseButton property

<b>Signature:</b>

```typescript
hideCloseButton?: boolean;
```
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@ export interface OverlayFlyoutOpenOptions
| ["data-test-subj"](./kibana-plugin-core-public.overlayflyoutopenoptions._data-test-subj_.md) | <code>string</code> | |
| [className](./kibana-plugin-core-public.overlayflyoutopenoptions.classname.md) | <code>string</code> | |
| [closeButtonAriaLabel](./kibana-plugin-core-public.overlayflyoutopenoptions.closebuttonarialabel.md) | <code>string</code> | |
| [hideCloseButton](./kibana-plugin-core-public.overlayflyoutopenoptions.hideclosebutton.md) | <code>boolean</code> | |
| [maxWidth](./kibana-plugin-core-public.overlayflyoutopenoptions.maxwidth.md) | <code>boolean &#124; number &#124; string</code> | |
| [onClose](./kibana-plugin-core-public.overlayflyoutopenoptions.onclose.md) | <code>(flyout: OverlayRef) =&gt; void</code> | |
| [ownFocus](./kibana-plugin-core-public.overlayflyoutopenoptions.ownfocus.md) | <code>boolean</code> | |
| [size](./kibana-plugin-core-public.overlayflyoutopenoptions.size.md) | <code>EuiFlyoutSize</code> | |

Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<!-- Do not edit this file. It is automatically generated by API Documenter. -->

[Home](./index.md) &gt; [kibana-plugin-core-public](./kibana-plugin-core-public.md) &gt; [OverlayFlyoutOpenOptions](./kibana-plugin-core-public.overlayflyoutopenoptions.md) &gt; [onClose](./kibana-plugin-core-public.overlayflyoutopenoptions.onclose.md)

## OverlayFlyoutOpenOptions.onClose property

<b>Signature:</b>

```typescript
onClose?: (flyout: OverlayRef) => void;
```
12 changes: 11 additions & 1 deletion src/core/public/overlays/flyout/flyout_service.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,8 @@ export interface OverlayFlyoutOpenOptions {
'data-test-subj'?: string;
size?: EuiFlyoutSize;
maxWidth?: boolean | number | string;
hideCloseButton?: boolean;
onClose?: (flyout: OverlayRef) => void;
Copy link
Contributor

Choose a reason for hiding this comment

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

Would you mind adding a comment that flyout.close must be called by an external 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.

Sure 👍

}

interface StartDeps {
Expand Down Expand Up @@ -118,9 +120,17 @@ export class FlyoutService {

this.activeFlyout = flyout;

const onCloseFlyout = () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mshustov I've made another changes to the file in this commit. I needed to be able to have a custom onClose handler that could prevent the flyout from closing.

Copy link
Contributor

Choose a reason for hiding this comment

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

It doesn't look like onClose. onClose means a reaction to an action. Maybe it should be named canClose as well?

Copy link
Contributor

Choose a reason for hiding this comment

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

Or onClose can accept flyout as an argument. Then a caller must call flyout.close:

onClose = {(flyout) => if (condition) { flyout.close() } }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes I'd prefer to stick to EUI onClose prop naming as it correspond to the "closing" event when clicking outside of the flyout. I will update with your second suggestion 👍

if (options.onClose) {
options.onClose(flyout);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mshustov I've made the change as you suggested

} else {
flyout.close();
}
};

render(
<i18n.Context>
<EuiFlyout {...options} onClose={() => flyout.close()}>
<EuiFlyout {...options} onClose={onCloseFlyout}>
<MountWrapper mount={mount} className="kbnOverlayMountWrapper" />
</EuiFlyout>
</i18n.Context>,
Expand Down
4 changes: 4 additions & 0 deletions src/core/public/public.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -970,8 +970,12 @@ export interface OverlayFlyoutOpenOptions {
// (undocumented)
closeButtonAriaLabel?: string;
// (undocumented)
hideCloseButton?: boolean;
// (undocumented)
maxWidth?: boolean | number | string;
// (undocumented)
onClose?: (flyout: OverlayRef) => void;
// (undocumented)
ownFocus?: boolean;
// (undocumented)
size?: EuiFlyoutSize;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
---
id: formLibCoreUseFormIsModified
slug: /form-lib/core/use-form-is-modified
title: useFormIsModified()
summary: Know when your form has been modified by the user
tags: ['forms', 'kibana', 'dev']
date: 2021-06-15
---

**Returns:** `boolean`

There might be cases where you need to know if the form has been modified by the user. For example: the user is about to leave the form after making some changes, you might want to show a modal indicating that the changes will be lost.

For that you can use the `useFormIsModified` hook which will update each time any of the field value changes. If the user makes a change and then undoes the change and puts the initial value back, the form **won't be marked** as modified.

**Important:** If you form dynamically adds and removes fields, the `isModified` state will be set to `true` when a field is removed from the DOM **only** if it was declared in the form initial `defaultValue` object.

## Options

### form

**Type:** `FormHook`

The form hook object. It is only required to provide the form hook object in your **root form component**.

```js
const RootFormComponent = () => {
// root form component, where the form object is declared
const { form } = useForm();
const isModified = useFormIsModified({ form });

return (
<Form form={form}>
<ChildComponent />
</Form>
);
};

const ChildComponent = () => {
const isModified = useFormIsModified(); // no need to provide the form object
return (
<div>...</div>
);
};
```

### discard

**Type:** `string[]`

If there are certain fields that you want to discard when checking if the form has been modified you can provide an array of field paths to the `discard` option.
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import React, { useEffect, FunctionComponent } from 'react';
import { act } from 'react-dom/test-utils';

import { registerTestBed, TestBed } from '../shared_imports';
import { FormHook, OnUpdateHandler, FieldConfig } from '../types';
import { FormHook, OnUpdateHandler, FieldConfig, FieldHook } from '../types';
import { useForm } from '../hooks/use_form';
import { Form } from './form';
import { UseField } from './use_field';
Expand Down Expand Up @@ -54,6 +54,145 @@ describe('<UseField />', () => {
});
});

describe('state', () => {
describe('isPristine, isDirty, isModified', () => {
// Dummy component to handle object type data
const ObjectField: React.FC<{ field: FieldHook }> = ({ field: { setValue } }) => {
const onFieldChange = (e: React.ChangeEvent<HTMLInputElement>) => {
// Make sure to set the field value to an **object**
setValue(JSON.parse(e.target.value));
};

return <input onChange={onFieldChange} data-test-subj="testField" />;
};

interface FieldState {
isModified: boolean;
isDirty: boolean;
isPristine: boolean;
value: unknown;
}

const getChildrenFunc = (
onStateChange: (state: FieldState) => void,
Component?: React.ComponentType<{ field: FieldHook }>
) => {
// This is the children passed down to the <UseField path="name" /> of our form
const childrenFunc = (field: FieldHook) => {
const { onChange, isModified, isPristine, isDirty, value } = field;

// Forward the field state to our jest.fn() spy
onStateChange({ isModified, isPristine, isDirty, value });

// Render the child component if any (useful to test the Object field type)
return Component ? (
<Component field={field} />
) : (
<input onChange={onChange} data-test-subj="testField" />
);
};

return childrenFunc;
};

interface Props {
fieldProps: Record<string, any>;
}

const TestComp = ({ fieldProps }: Props) => {
const { form } = useForm();
return (
<Form form={form}>
<UseField path="name" {...fieldProps} />
</Form>
);
};

const onStateChangeSpy = jest.fn<void, [FieldState]>();
const lastFieldState = (): FieldState =>
onStateChangeSpy.mock.calls[onStateChangeSpy.mock.calls.length - 1][0];
const toString = (value: unknown): string =>
typeof value === 'string' ? value : JSON.stringify(value);

const setup = registerTestBed(TestComp, {
defaultProps: { onStateChangeSpy },
memoryRouter: { wrapComponent: false },
});

[
{
description: 'should update the state for field without default values',
initialValue: '',
changedValue: 'changed',
fieldProps: { children: getChildrenFunc(onStateChangeSpy) },
},
{
description: 'should update the state for field with default value in their config',
initialValue: 'initialValue',
changedValue: 'changed',
fieldProps: {
children: getChildrenFunc(onStateChangeSpy),
config: { defaultValue: 'initialValue' },
},
},
{
description: 'should update the state for field with default value passed through props',
initialValue: 'initialValue',
changedValue: 'changed',
fieldProps: {
children: getChildrenFunc(onStateChangeSpy),
defaultValue: 'initialValue',
},
},
// "Object" field type must be JSON.serialized to compare old and new value
// this test makes sure this is done and "isModified" is indeed "false" when
// putting back the original object
{
description: 'should update the state for field with object field type',
initialValue: { initial: 'value' },
changedValue: { foo: 'bar' },
fieldProps: {
children: getChildrenFunc(onStateChangeSpy, ObjectField),
defaultValue: { initial: 'value' },
},
},
].forEach(({ description, fieldProps, initialValue, changedValue }) => {
test(description, async () => {
const { form } = await setup({ fieldProps });

expect(lastFieldState()).toEqual({
isPristine: true,
isDirty: false,
isModified: false,
value: initialValue,
});

await act(async () => {
form.setInputValue('testField', toString(changedValue));
});

expect(lastFieldState()).toEqual({
isPristine: false,
isDirty: true,
isModified: true,
value: changedValue,
});

// Put back to the initial value --> isModified should be false
await act(async () => {
form.setInputValue('testField', toString(initialValue));
});
expect(lastFieldState()).toEqual({
isPristine: false,
isDirty: true,
isModified: false,
value: initialValue,
});
});
});
});
});

describe('validation', () => {
let formHook: FormHook | null = null;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,15 @@ export const FormProvider = ({ children, form }: Props) => (
<FormContext.Provider value={form}>{children}</FormContext.Provider>
);

export const useFormContext = function <T extends FormData = FormData>() {
interface Options {
throwIfNotFound?: boolean;
}

export const useFormContext = function <T extends FormData = FormData>({
throwIfNotFound = true,
}: Options = {}) {
const context = useContext(FormContext) as FormHook<T>;
if (context === undefined) {
if (throwIfNotFound && context === undefined) {
throw new Error('useFormContext must be used within a <FormProvider />');
}
return context;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,4 @@
export { useField, InternalFieldConfig } from './use_field';
export { useForm } from './use_form';
export { useFormData } from './use_form_data';
export { useFormIsModified } from './use_form_is_modified';
Loading