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

[Mappings editor] Clean up dynamic_templates code #54950

Conversation

alisonelizabeth
Copy link
Contributor

Addresses this comment: #54523 (comment)

How to review

  • Confirm comment above is addressed as expected
  • Confirm dynamic_templates functionality still works as expected

@alisonelizabeth alisonelizabeth added v8.0.0 Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more release_note:skip Skip the PR/issue when compiling release notes v7.7.0 Feature:Mappings Editor Index mappings editor UI labels Jan 15, 2020
@elasticmachine
Copy link
Contributor

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

Copy link
Contributor

@sebelga sebelga left a comment

Choose a reason for hiding this comment

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

Thanks for making these changes @alisonelizabeth ! Tested locally and works as expected.

Testing this PR, I notice we have a small regression bug on the UI but it is not critical.
After the change to start using "tabs", the isStateValid() (in utils.ts) logic needs to be updated. Now, it always returns undefined, so when a tab is not valid (for example the dynamic templates), the Steps and the "Next" button are not disabled anymore.

@alisonelizabeth
Copy link
Contributor Author

alisonelizabeth commented Jan 16, 2020

Testing this PR, I notice we have a small regression bug on the UI but it is not critical.
After the change to start using "tabs", the isStateValid() (in utils.ts) logic needs to be updated. Now, it always returns undefined, so when a tab is not valid (for example the dynamic templates), the Steps and the "Next" button are not disabled anymore.

Great catch! I'm opened #55051 since this bug wasn't introduced via this PR.

@alisonelizabeth
Copy link
Contributor Author

@elasticmachine merge upstream

@elasticmachine
Copy link
Contributor

user doesn't have permission to update head repository

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

@alisonelizabeth alisonelizabeth merged commit 46568c2 into elastic:master Jan 16, 2020
@alisonelizabeth alisonelizabeth deleted the mappings-editor/dynamic-templates-fix branch January 16, 2020 16:53
alisonelizabeth added a commit to alisonelizabeth/kibana that referenced this pull request Jan 16, 2020
jkelastic pushed a commit to jkelastic/kibana that referenced this pull request Jan 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Mappings Editor Index mappings editor UI 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.7.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants