Skip to content

[serverless] Advanced Settings - Form component#166460

Merged
ElenaStoeva merged 32 commits intoelastic:mainfrom
ElenaStoeva:advanced_settings/form
Sep 26, 2023
Merged

[serverless] Advanced Settings - Form component#166460
ElenaStoeva merged 32 commits intoelastic:mainfrom
ElenaStoeva:advanced_settings/form

Conversation

@ElenaStoeva
Copy link
Copy Markdown
Contributor

@ElenaStoeva ElenaStoeva commented Sep 14, 2023

Addresses #160411

Summary

This PR adds a package that contains a form component for the Advanced Settings UI in serverless.
This implementation was extracted from the the Form component in the advancedSettings plugin, excluding some functionalities:

  • The form doesn't support search queries.
  • The form doesn't divide the settings into categories.

Testing

The form can be tested in the Storybook Preview from the CI build. Some things to be tested:

  • Making changes to any of the fields displays the bottom bar.
  • Clicking the Cancel button clears the changes.
  • Clicking the Save button triggers a saveChanges action with the correct changes.
  • The bottom bar correctly shows the number of unsaved settings.
  • Toggling the isSavingEnabled control to false disables all fields.
  • Toggling the requirePageReload control to true causes saving of changes to any of the fields to trigger a showReloadPagePrompt action.

Checklist

@ElenaStoeva ElenaStoeva added Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more t// release_note:skip Skip the PR/issue when compiling release notes labels Sep 14, 2023
@ElenaStoeva ElenaStoeva self-assigned this Sep 14, 2023
Copy link
Copy Markdown
Contributor

@clintandrewhall clintandrewhall left a comment

Choose a reason for hiding this comment

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

This is great work...! Quick turnaround, too. Just a few nits/improvements. While we wait for the prerequisite to ship, I would take some time to document and add a few tests.

Comment thread packages/kbn-management/settings/components/form/bottom_bar/index.tsx Outdated
Comment thread packages/kbn-management/settings/components/form/bottom_bar/index.tsx Outdated
Comment thread packages/kbn-management/settings/components/form/form.tsx Outdated
Comment thread packages/kbn-management/settings/components/form/services.tsx Outdated
@ElenaStoeva ElenaStoeva force-pushed the advanced_settings/form branch from 0787f7a to 012e68b Compare September 19, 2023 17:31
@ElenaStoeva ElenaStoeva force-pushed the advanced_settings/form branch from b9ea610 to bdce1a5 Compare September 19, 2023 18:17
Comment thread packages/kbn-management/settings/components/form/reload_page_toast.tsx Outdated
Comment thread packages/kbn-management/settings/components/form/storybook/get_form_story.tsx Outdated
Comment thread packages/kbn-management/settings/components/form/storybook/get_form_story.tsx Outdated
Comment thread packages/kbn-management/settings/components/form/storybook/get_form_story.tsx Outdated
Comment thread packages/kbn-management/settings/components/form/storybook/get_form_story.tsx Outdated
Comment thread packages/kbn-management/settings/components/form/types.ts Outdated
Comment thread packages/kbn-management/settings/components/form/types.ts
@ElenaStoeva ElenaStoeva force-pushed the advanced_settings/form branch from 0865caa to cb9e357 Compare September 20, 2023 07:23
@ElenaStoeva ElenaStoeva marked this pull request as ready for review September 20, 2023 18:14
@ElenaStoeva ElenaStoeva requested a review from a team as a code owner September 20, 2023 18:14
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/platform-deployment-management (Team:Deployment Management)

@ElenaStoeva ElenaStoeva changed the title [WIP] [serverless] Advanced Settings - Form component [serverless] Advanced Settings - Form component Sep 20, 2023
@ElenaStoeva
Copy link
Copy Markdown
Contributor Author

@elasticmachine merge upstream

@ElenaStoeva
Copy link
Copy Markdown
Contributor Author

@elasticmachine merge upstream

Copy link
Copy Markdown
Contributor

@yuliacech yuliacech left a comment

Choose a reason for hiding this comment

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

Amazing work, @ElenaStoeva! Code changes LGTM, also verified the form locally with storybook.

Copy link
Copy Markdown
Contributor

@clintandrewhall clintandrewhall left a comment

Choose a reason for hiding this comment

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

Awesome work...! I'm approving as well, but please address my comments before committing. Thanks for your work here!

Comment thread packages/kbn-management/settings/components/form/bottom_bar/index.tsx Outdated
Comment thread packages/kbn-management/settings/components/form/bottom_bar/unsaved_count.tsx Outdated
Comment thread packages/kbn-management/settings/components/form/form.styles.ts Outdated
Comment thread packages/kbn-management/settings/components/form/form.tsx Outdated
Comment thread packages/kbn-management/settings/components/form/form.tsx Outdated
Comment thread packages/kbn-management/settings/components/form/reload_page_toast.tsx Outdated
Comment thread packages/kbn-management/settings/components/form/storybook/get_form_story.tsx Outdated
@ElenaStoeva
Copy link
Copy Markdown
Contributor Author

@elasticmachine merge upstream

@ElenaStoeva
Copy link
Copy Markdown
Contributor Author

@elasticmachine merge upstream

@ElenaStoeva
Copy link
Copy Markdown
Contributor Author

@elasticmachine merge upstream

@kibana-ci
Copy link
Copy Markdown

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #17 / serverless examples UI Partial Results Example "before all" hook for "should trace mouse events"

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
@kbn/management-settings-components-form - 2 +2

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id before after diff
@kbn/management-settings-components-form - 3 +3
Unknown metric groups

API count

id before after diff
@kbn/management-settings-components-form - 8 +8

History

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

cc @ElenaStoeva

@ElenaStoeva ElenaStoeva merged commit fd1a1f9 into elastic:main Sep 26, 2023
@kibanamachine kibanamachine added v8.11.0 backport:skip This PR does not require backporting labels Sep 26, 2023
@ElenaStoeva ElenaStoeva deleted the advanced_settings/form branch September 27, 2023 11:43
clintandrewhall added a commit that referenced this pull request Sep 28, 2023
…less (#167447)

## Summary

This PR follows #166460 by adding Category panels to the Form.

<img width="1807" alt="Screenshot 2023-09-27 at 3 36 16 PM"
src="https://github.com/elastic/kibana/assets/297604/2abe8cf5-5822-473f-affd-148fb7949316">

## Notes
This PR is divided into several commits, the first few being
prerequisite codemods. I recommend reviewing each commit separately, as
the codemods might obscure the actual component work.

- [e78586f - Make SettingType pre-defined to clean up
references](e78586f)
- This makes the `SettingType` optional, to clean up areas where the
generic need not be specific.
- [80a9988 - [codemod] Make onFieldChange and onInputChange more
distinct](80a9988)
- The `onChange` handlers weren't very clear as you work your way up the
component tree. This makes the implementation and usage easier to
understand, (and easier to [replace with state
management](#166579)).
- [5d0beff - [fix] Fix logged errors in form
tests](5d0beff)
- This fixes some logged errors in the Form from `Monaco` and from some
missing `act` and `waitFor` calls.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting Feature:uiSettings 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 t// v8.11.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants