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

[Form lib] Add useFormData() hook to listen to fields value changes #76107

Merged
merged 17 commits into from
Sep 1, 2020

Conversation

sebelga
Copy link
Contributor

@sebelga sebelga commented Aug 27, 2020

This PR adds a new hook on the form lib to listen to field value changes. With this hook we will be able to deprecate the <FormDataProvider />.

This hook also deprecates the use of form.subscribe(), so the form lib is more aligned with the React API with the use of useEffect().

Note: None of the above will be removed just yet, we will remove them in separate PR.

// Listen to form data change in **root** component (where the form object is created)

const { form } = useForm();

// Listen to any field change
const [data] = useFormData({ form }); // Provide the "form" object to the hook

// To only listen to **some** field change, provide one or multiple string) in the "watch" option
const [{ 'some.field': someField }] = useFormData({ form, watch: 'some.field' /* string | string[] */ });
// Forward state and getFormData handler to a parent component (replaces form.subscribe())

const { form } = useForm();
const { isValid, validate, getFormData } = form;

useEffect(() => {
  // Do anything when the data or validity changes (e.g. forward the state to a parent component)
  // onFormChange({ isValid, getFormData, validate });
}, [isValid, getFormData, validate]);
// In any **child** component of the <Form /> you don't need to provide the form object
const [{ field1, field2 }] = useFormData({ watch: ['field1', 'field2'] });

// In the JSX
return (
  { field1 === 'someValue' && (<div>Hello</div>) }
);

// If you need to format the form data (build the final object),
// the second parameter you receive is a handler to do just that.
// Be aware that it could have some performance hit if you have a complex object (e.g. loads of nested fields with expensive `serializers` like JSON.parse)
const [formData, format] = useFormData();

// In the JSX

return (
  <code>{JSON.stringify(format(), null, 2)}</code>
);

@sebelga sebelga requested a review from a team as a code owner August 27, 2020 15:58
@sebelga sebelga added release_note:skip Skip the PR/issue when compiling release notes Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more v7.10.0 v8.0.0 labels Aug 27, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/es-ui (Team:Elasticsearch UI)

Copy link
Contributor

@alisonelizabeth alisonelizabeth left a comment

Choose a reason for hiding this comment

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

Great work @sebelga! I really like the addition of the new hook. I tested the meta toggle in the index templates form and it still worked as expected.

What is the timeline for removing the use of FormDataProvider and form.subscribe() across our forms? Is it worth adding a deprecation warning in form_data_provider?


I found a couple bugs while testing, although I don't think they are related to your changes. I can open up separate issues if you'd like.

  1. Enable the meta toggle, add some data and save. Then go back to edit the template and toggle off the meta field. If I save it, the meta data is still saved (and not cleared).
  2. The validation for the combobox doesn't appear to get cleared.
    indices_error

@@ -0,0 +1,234 @@
/*
Copy link
Contributor

Choose a reason for hiding this comment

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

Great work with the tests 🎉

@sebelga
Copy link
Contributor Author

sebelga commented Sep 1, 2020

Thanks for the review @alisonelizabeth !

Great catch for the 2 issues, there are not related to my changes bug I fixed them in this PR. For the ComboBox issue, I added an optional isBlocking param to the ValidationConfig. We might want to add it to all of our Combobox VALIDATION_TYPES.ARRAY_ITEM validations if we think it is necessary.

What is the timeline for removing the use of FormDataProvider and form.subscribe() across our forms?

I don't have a timeline but I'd love to be able to do it in one go after I finish writing the docs. For now the important is not to add any new ones to future forms 😊

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

@kbn/optimizer bundle module count

id value diff baseline
esUiShared 231 +2 229

async chunks size

id value diff baseline
indexManagement 1.6MB +1.4KB 1.6MB

page load bundle size

id value diff baseline
esUiShared 993.6KB +2.6KB 991.0KB
indexManagement 248.0KB -5.0B 248.1KB
total +2.6KB

oss distributable file count

id value diff baseline
total 27307 +2 27305

distributable file count

id value diff baseline
total 45458 +2 45456

History

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

@sebelga sebelga merged commit ef7246f into elastic:master Sep 1, 2020
@sebelga sebelga deleted the form-lib/use-form-data-2 branch September 1, 2020 12:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:skip Skip the PR/issue when compiling release notes Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more v7.10.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants