uiSettings - use validation field for image field maxSize #54522

mattkime · 2020-01-11T01:28:05Z

Summary

In uiSettings, the image upload field required specifying maxSize via the options field. This was in conflict with the stated use and type of options - which is a string[] use to populate select fields. Use validation field for image field maxSize

Checklist

Use ~~strikethroughs~~ to remove checklist items you don't feel are applicable to this PR.

This was checked for cross-browser compatibility, including a check against IE11
~~[ ] Any text added follows EUI's writing guidelines, uses sentence case text and includes [i18n support]~~(https://github.com/elastic/kibana/blob/master/packages/kbn-i18n/README.md)
~~[ ] Documentation was added for features that require explanation or tutorials~~
Unit or functional tests were updated or added to match the most common scenarios
~~[ ] This was checked for keyboard-only and screenreader accessibility~~

For maintainers

This was checked for breaking API changes and was labeled appropriately
This includes a feature addition or change that requires a release note and was labeled appropriately

Dev Docs

`uiSettings` image upload field config

In uiSettings, the image upload field required specifying maxSize via the options field. This was in conflict with the stated use and type of options - which is a string[] use to populate select fields. uiOptions has been provided instead, accepting Record<string, any> values.

mattkime · 2020-01-11T05:15:18Z

@elasticmachine merge upstream

src/legacy/core_plugins/kibana/public/management/sections/settings/components/field/field.js

…tkime/kibana into reporting_custom_logo_fix_uiSettings

elasticmachine · 2020-01-11T19:48:03Z

Pinging @elastic/kibana-app-arch (Team:AppArch)

pgayvallet · 2020-01-12T16:42:33Z

docs/development/core/server/kibana-plugin-server.uisettingsparams.md

-|  [requiresPageReload](./kibana-plugin-server.uisettingsparams.requirespagereload.md) | <code>boolean</code> | a flag indicating whether new value applying requires page reloading |
-|  [type](./kibana-plugin-server.uisettingsparams.type.md) | <code>UiSettingsType</code> | defines a type of UI element [UiSettingsType](./kibana-plugin-server.uisettingstype.md) |
-|  [value](./kibana-plugin-server.uisettingsparams.value.md) | <code>SavedObjectAttribute</code> | default value to fall back to if a user doesn't provide any |
-
+<!-- Do not edit this file. It is automatically generated by API Documenter. -->
+
+[Home](./index.md) &gt; [kibana-plugin-server](./kibana-plugin-server.md) &gt; [UiSettingsParams](./kibana-plugin-server.uisettingsparams.md)
+
+## UiSettingsParams interface
+
+UiSettings parameters defined by the plugins.


The generated doc files are using windows line feed (this is how the tool work so we decided to keep it like this), and you pushed them with LF (autocrlf probably). Please regenerate the doc and commit them with preserved line feeds.

I don't think there is a way to configure it currently #53761 (comment)

I don't think either, that's why we need to stick to CRLF for now. Issue is, a lot of developers uses core.autocrlf input git config to convert to LF (which is a good practice most of the time) and may push without even knowing they altered the linefeeds.

we can enforce EOL format for all *.md files on the github repo level https://github.com/microsoft/rushstack/blob/2c941e21777393c67979b25dc087433485b12575/.gitattributes#L44

Obviously I know very little about this tool, but I'd be tempted to wrap it in a script that converts the line endings. Having markdown files that must use different line endings is going to cause some headaches.

mshustov · 2020-01-13T08:55:57Z

src/core/server/ui_settings/types.ts

@@ -102,6 +102,8 @@ export interface UiSettingsParams {
  readonly?: boolean;
  /** defines a type of UI element {@link UiSettingsType} */
  type?: UiSettingsType;
+  /** field ui options */
+  uiOptions?: Record<string, any>;


uiOptions sounds like anything used in UI, although reporting uses it for validation. There is the validation field for the same purpose added in #44678 although it's missed in TypeScript definition. Would you mind adding one?
We have the separate issue to introduce a proper validation for UI settings #46717
Thus, we should mark this validation field as deprecated.

type StringValidation = { regexString: string, message: string }; type ImageValidation = { maxSize: ... } interface UiSettingsParams { ... /* * Allows defining a custom validation applicable to value change on the client. * @deprecated */ validation: StringValidation | ImageValidation; }

mattkime · 2020-01-13T15:47:11Z

@elasticmachine merge upstream

…tkime/kibana into reporting_custom_logo_fix_uiSettings

kibanamachine · 2020-01-13T17:36:15Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 8c81b7c

History

💔 Build #19787 failed d3a6654
💔 Build #19785 failed f8bf4f6
💔 Build #19781 failed 4aa2b7f
💚 Build #19608 succeeded 136aa44
💚 Build #19560 succeeded fe1ea65

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

streamich

Code changes LGTM.

) * uiSettings - use validation field for image field maxSize

…54641) * uiSettings - use validation field for image field maxSize

* upstream/master: (26 commits) Take page offset into account too (elastic#54567) [APM] Support error.{log,exception}.stacktrace.classname (elastic#54577) Np migration tsvb route validation (elastic#51850) [ML] MML calculator enhancements for multi-metric job wizard (elastic#54573) [SIEM] Fix Inspect query 'request timestamp' value changes when curso… (elastic#54223) Fix chromeless NP apps not using full page width (elastic#54550) Remove extraneous public import to prevent failing Kibana startup (elastic#54676) [Uptime] Temporarily skip flakey tests (elastic#54675) Skip failing uptime tests Create UI for alerting and actions plugin (elastic#48959) [dev/build/sass] build stylesheets for disabled plugins too (elastic#54654) [SIEM] Use bulk actions API when updating or deleting rules (elastic#54521) Support "Deprecated" label in advanced settings (elastic#54539) [Maps] add text halo color and width style properties (elastic#53827) Service Map Data API at Runtime (elastic#54027) [SIEM] Detection Engine Create Rule Design Review #1 (elastic#54442) Skip flaky test [Canvas] Enable Embeddable maps (elastic#53971) [SIEM][Detection Engine] Increases the number or rules you can view on a single page (elastic#54628) uiSettings - use validation field for image field maxSize (elastic#54522) ...

) * uiSettings - use validation field for image field maxSize

mattkime added 3 commits January 10, 2020 19:25

partial progress

b25bd39

update api docs

fd66ff3

update field test

fbd9854

Merge branch 'master' into reporting_custom_logo_fix_uiSettings

3ca14d7

mattkime commented Jan 11, 2020

View reviewed changes

src/legacy/core_plugins/kibana/public/management/sections/settings/components/field/field.js Outdated Show resolved Hide resolved

mattkime added 2 commits January 11, 2020 10:15

use record<string,any> - pr seems to be working

4828ff0

Merge branch 'reporting_custom_logo_fix_uiSettings' of github.com:mat…

90e7722

…tkime/kibana into reporting_custom_logo_fix_uiSettings

mattkime added v7.6.0 v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Team:AppArch labels Jan 11, 2020

mattkime marked this pull request as ready for review January 11, 2020 19:48

mattkime requested review from a team as code owners January 11, 2020 19:48

mattkime added 2 commits January 11, 2020 18:10

update snapshot

d26b7cb

speeling counts!

fe1ea65

mattkime added release_note:plugin_api_changes Contains a Plugin API changes section for the breaking plugin API changes section. and removed release_note:skip Skip the PR/issue when compiling release notes labels Jan 12, 2020

pgayvallet reviewed Jan 12, 2020

View reviewed changes

fix line feeds

136aa44

mshustov reviewed Jan 13, 2020

View reviewed changes

mattkime added 4 commits January 13, 2020 09:16

use validation instead of uiOptions

7049ce8

fix snapshot, add server side ts

1fe4fc2

update docs

4aa2b7f

Merge branch 'master' into reporting_custom_logo_fix_uiSettings

f8bf4f6

Merge branch 'master' into reporting_custom_logo_fix_uiSettings

d3a6654

mattkime added 5 commits January 13, 2020 10:07

ts fix

11b0a49

Merge branch 'reporting_custom_logo_fix_uiSettings' of github.com:mat…

8a52e06

…tkime/kibana into reporting_custom_logo_fix_uiSettings

update api docs

99cb684

export types

fa4cbbc

fix docs

8c81b7c

mshustov approved these changes Jan 13, 2020

View reviewed changes

streamich approved these changes Jan 13, 2020

View reviewed changes

mattkime changed the title ~~uiSettings - method of supplying arbitrary params to ui (reporting was overloading options)~~ uiSettings - use validation field for image field maxSize Jan 13, 2020

mattkime merged commit 2178ee3 into elastic:master Jan 13, 2020

mattkime added a commit to mattkime/kibana that referenced this pull request Jan 13, 2020

uiSettings - use validation field for image field maxSize (elastic#54522

b486b25

) * uiSettings - use validation field for image field maxSize

mattkime mentioned this pull request Jan 13, 2020

[7.x] uiSettings - use validation field for image field maxSize (#54522) #54641

Merged

mattkime added a commit that referenced this pull request Jan 13, 2020

uiSettings - use validation field for image field maxSize (#54522) (#…

6ca9018

…54641) * uiSettings - use validation field for image field maxSize

jkelastic pushed a commit to jkelastic/kibana that referenced this pull request Jan 17, 2020

uiSettings - use validation field for image field maxSize (elastic#54522

0ef0f23

) * uiSettings - use validation field for image field maxSize

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

uiSettings - use validation field for image field maxSize #54522

uiSettings - use validation field for image field maxSize #54522

mattkime commented Jan 11, 2020 •

edited

Loading

mattkime commented Jan 11, 2020

elasticmachine commented Jan 11, 2020

pgayvallet Jan 12, 2020

mshustov Jan 13, 2020

pgayvallet Jan 13, 2020

mshustov Jan 13, 2020

mattkime Jan 13, 2020

mshustov Jan 13, 2020

mattkime commented Jan 13, 2020

kibanamachine commented Jan 13, 2020

streamich left a comment

uiSettings - use validation field for image field maxSize #54522

uiSettings - use validation field for image field maxSize #54522

Conversation

mattkime commented Jan 11, 2020 • edited Loading

Summary

Checklist

For maintainers

Dev Docs

uiSettings image upload field config

mattkime commented Jan 11, 2020

elasticmachine commented Jan 11, 2020

pgayvallet Jan 12, 2020

Choose a reason for hiding this comment

mshustov Jan 13, 2020

Choose a reason for hiding this comment

pgayvallet Jan 13, 2020

Choose a reason for hiding this comment

mshustov Jan 13, 2020

Choose a reason for hiding this comment

mattkime Jan 13, 2020

Choose a reason for hiding this comment

mshustov Jan 13, 2020

Choose a reason for hiding this comment

mattkime commented Jan 13, 2020

kibanamachine commented Jan 13, 2020

💚 Build Succeeded

History

streamich left a comment

Choose a reason for hiding this comment

mattkime commented Jan 11, 2020 •

edited

Loading

`uiSettings` image upload field config