Skip to content

Comments

Painless help text addition#9180

Closed
LeeDr wants to merge 2 commits intoelastic:masterfrom
LeeDr:painlessHelpText
Closed

Painless help text addition#9180
LeeDr wants to merge 2 commits intoelastic:masterfrom
LeeDr:painlessHelpText

Conversation

@LeeDr
Copy link

@LeeDr LeeDr commented Nov 22, 2016

Added 2 h5 headers "Painless:" and "Expression:". But I'm not thrilled with how they look. Maybe should be bigger?

Added note that says;
Sorting, aggregations, and access to field values in scripts require doc values which are supported (and on by default) on almost all field types, with the notable exception of analyzed string fields. You can check the Fields tab to make sure the fields you want to use in your scripts are aggregatable and not analyzed before using them in a script.

Before:
2016-11-22 10_02_28-new scripted field - logstash-_ - kibana

After:
2016-11-22 10_22_10-new scripted field - logstash-_ - kibana

@LeeDr
Copy link
Author

LeeDr commented Nov 22, 2016

Maybe we should add another example that shows some Painless scripting feature?

@tbragin
Copy link
Contributor

tbragin commented Nov 22, 2016

@Bargs @epixa Since we now have a PR in flight to fix the filtering issue, do we still need the note introduced in this PR? #9090

@tbragin tbragin added the Feature:Scripted Fields Scripted fields features label Nov 22, 2016
@cjcenizal
Copy link
Contributor

You're right @LeeDr ... the current styles for h5 don't elevate that text high enough in the visual hierarchy. What do you think of this:

image

Here's how can you can update your PR to the above screenshot:

  1. Create a class in base.less:
.heading5 {
  font-size: 16px;
  font-weight: 700;
}
  1. Apply this class to the headings you have, e.g. <h5 class="heading5">Painless</h5>.
  2. Remove the colons from these headings.

@Bargs
Copy link
Contributor

Bargs commented Nov 22, 2016

@tbragin unless I'm missing something, I think this is orthogonal to the filter issue.

The note is useful, but the current wording and structure might be a bit confusing. The note is really applicable to both painless and expressions, but the current format makes it look like it just applies to painless. Also, it's not strictly true that access to field values in scripts require doc values. If they've read the ES docs, users might have seen _source being accessed so this wording could confuse them. doc_values are required because of the way we use those scripts.

For now it might be sufficient to say "Field access in Kibana scripted fields should be done via doc values. Doc values are supported (and on by default) by almost all field types...(and so on)"

@LeeDr
Copy link
Author

LeeDr commented Nov 23, 2016

@Bargs I'm fine with other wording, so long as I can easily relate it to something I can see on the Fields tab. I don't want it to describe doc values and the user doesn't have a clue to figure out what has doc values.

And if this applies to both expression and Painless scripts we should definitely move the text above the script-specific sections.

@Bargs Bargs removed their assignment Dec 20, 2016
@tbragin
Copy link
Contributor

tbragin commented Jan 5, 2017

@LeeDr are you still working on this?

Might be good to get @debadair eyes on it.

@LeeDr
Copy link
Author

LeeDr commented Jan 5, 2017

@tbragin yes, I'd like to get back to this soon. If @debadair has any comments now I'll incorporate those as soon as I can. Or she can check it after I've rearranged things a bit.

@epixa epixa removed the P3 label Apr 25, 2017
@tsullivan
Copy link
Member

Hi @LeeDr, do we still need this PR? There's no updates in over a year.

@elasticmachine
Copy link
Contributor

💔 Build Failed

@LeeDr
Copy link
Author

LeeDr commented May 1, 2018

Apparently I'm not treating this with much priority. I'll close it and maybe I'll come back around to it someday...

@LeeDr LeeDr closed this May 1, 2018
@LeeDr LeeDr deleted the painlessHelpText branch August 20, 2020 17:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature:Scripted Fields Scripted fields features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants