Skip to content

[Advanced settings] Reset to default for empty strings#85137

Merged
stratoula merged 2 commits intoelastic:masterfrom
stratoula:display-reset-for-empty-strings
Dec 10, 2020
Merged

[Advanced settings] Reset to default for empty strings#85137
stratoula merged 2 commits intoelastic:masterfrom
stratoula:display-reset-for-empty-strings

Conversation

@stratoula
Copy link
Copy Markdown
Contributor

@stratoula stratoula commented Dec 7, 2020

Summary

Fixes #84876.

I can see that we check if the value is empty in order to not display the Reset to default button link. I agree with Marco that this is a bug rather than functionality that we want.

With removing this check the reset to default is displayed now when the user empties an advanced setting.

Screenshot 2020-12-07 at 4 10 14 PM

I can't think of any scenario that we don't want this. I checked on fields that they are empty by default and this change doesn't affect the way that they behave as the empty string is the default value and the is_default_value.ts works as expected. For example for the Index pattern placeholder setting.

Screenshot 2020-12-07 at 6 20 14 PM

Checklist

Delete any items that are not applicable to this PR.

@stratoula stratoula added Feature:Kibana Management Feature label for Data Views, Advanced Setting, Saved Object management pages release_note:fix v7.11.0 v8.0.0 labels Dec 7, 2020
@stratoula stratoula requested a review from markov00 December 7, 2020 16:23
@stratoula stratoula marked this pull request as ready for review December 8, 2020 11:10
@stratoula stratoula requested a review from a team December 8, 2020 11:10
@stratoula stratoula added the Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// label Dec 8, 2020
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/kibana-app (Team:KibanaApp)

@stratoula
Copy link
Copy Markdown
Contributor Author

@elasticmachine merge upstream

Copy link
Copy Markdown
Contributor

@markov00 markov00 left a comment

Choose a reason for hiding this comment

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

LGTM

@kibanamachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
advancedSettings 919.6KB 919.6KB -20.0B

Distributable file count

id before after diff
default 46981 47741 +760

History

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

Copy link
Copy Markdown
Contributor

@mbondyra mbondyra left a comment

Choose a reason for hiding this comment

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

tested on FF, code LGTM 🆗

@stratoula stratoula merged commit b46655d into elastic:master Dec 10, 2020
stratoula added a commit to stratoula/kibana that referenced this pull request Dec 10, 2020
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
stratoula added a commit that referenced this pull request Dec 10, 2020
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
gmmorris added a commit to gmmorris/kibana that referenced this pull request Dec 10, 2020
* master: (53 commits)
  Fixing recovered instance reference bug (elastic#85412)
  Switch to new elasticsearch client for Visualizations (elastic#85245)
  Switch to new elasticsearch client for TSVB (elastic#85275)
  Switch to new elasticsearch client for Vega (elastic#85280)
  [ILM] Add shrink field to hot phase (elastic#84087)
  Add rolling-file appender to core logging (elastic#84735)
  [APM] Service overview: Dependencies table (elastic#83416)
  [Uptime ]Update empty message for certs list (elastic#78575)
  [Graph] Fix graph saved object references (elastic#85295)
  [APM] Create new API's to return Latency and Throughput charts (elastic#85242)
  [Advanced settings] Reset to default for empty strings (elastic#85137)
  [SECURITY SOLUTION] Bundles _source -> Fields + able to sort on multiple fields in Timeline (elastic#83761)
  [Fleet] Update agent listing for better status reporting (elastic#84798)
  [APM] enable 'sanitize_field_names' for Go (elastic#85373)
  Update dependency @elastic/charts to v24.4.0 (elastic#85452)
  Introduce external url service (elastic#81234)
  Deprecate disabling the security plugin (elastic#85159)
  [FLEET] New Integration Policy Details page for use in Integrations section (elastic#85355)
  [Security Solutions][Detection Engine] Fixes one liner access control with find_rules REST API
  chore: 🤖 remove extraPublicDirs (elastic#85454)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature:Kibana Management Feature label for Data Views, Advanced Setting, Saved Object management pages release_note:fix Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// v7.11.0 v8.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Advanced Settings] Reset to default button doesn't appear with empty strings

5 participants