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

Custom legends can be specified for filter aggregates #3621

Merged
merged 6 commits into from Jun 19, 2015

Conversation

@K-Phoen
Copy link
Contributor

@K-Phoen K-Phoen commented Apr 17, 2015

Closes #2245

This PR allows the user to define custom labels in filter aggregates, as shown:

this screenshot.

It's still WIP and this should also be implemented for range aggregates but it already works.

Let me know what you think :)

@@ -29,7 +29,7 @@ define(function (require) {

decorateQuery(query);

var label = _.deepGet(query, 'query_string.query') || angular.toJson(query);
var label = filter.label ? filter.label : (_.deepGet(query, 'query_string.query') || angular.toJson(query));

This comment has been minimized.

@rashidkpc

rashidkpc Apr 17, 2015
Contributor

Can you not use a ternary here? Make this hard to read.

This comment has been minimized.

@K-Phoen

K-Phoen Apr 20, 2015
Author Contributor

Done.

@rashidkpc
Copy link
Contributor

@rashidkpc rashidkpc commented Apr 20, 2015

This is a good start, something seems off about the label input box, spacing or something. It seems like the legend input should have a <label> for the input, something like Filter 1 label. Also, when you have a long label it interferes with the buttons:

image

@rashidkpc rashidkpc self-assigned this Apr 20, 2015
@K-Phoen
Copy link
Contributor Author

@K-Phoen K-Phoen commented Apr 22, 2015

I added a <label> for the legend field and fixed the "long label interfering with the buttons" issue but I didn't understand what you meant by "something seems off about the label input box, spacing or something".

@rashidkpc
Copy link
Contributor

@rashidkpc rashidkpc commented Apr 24, 2015

The spacing of the label on the input box is a bit more than it is for the other labels, likely due to the use of vis-editor-agg-header-title which isn't really meant for input box labels.

screen shot 2015-04-24 at 10 23 18 am

@K-Phoen
Copy link
Contributor Author

@K-Phoen K-Phoen commented Apr 25, 2015

Should be better now.

@@ -153,6 +153,14 @@
font-weight: bold;
}

&-growing {

This comment has been minimized.

@rashidkpc

rashidkpc Apr 28, 2015
Contributor

Not sure on this class naming? Also, this really isn't any kind of vis-editor-agg-header

@K-Phoen
Copy link
Contributor Author

@K-Phoen K-Phoen commented May 5, 2015

I updated the CSS definition.

@rashidkpc rashidkpc added review v4.2.0 and removed updates_needed labels May 7, 2015
@rashidkpc
Copy link
Contributor

@rashidkpc rashidkpc commented May 14, 2015

Looks good, I expect to merge this right after we cut 4.1

@K-Phoen
Copy link
Contributor Author

@K-Phoen K-Phoen commented May 15, 2015

Great, thanks for your patience :)

@elasticsearch-release
Copy link

@elasticsearch-release elasticsearch-release commented Jun 16, 2015

Jenkins standing by to test this. If you aren't a maintainer, you can ignore this comment. Someone with commit access, please review this and clear it for Jenkins to run; then say 'jenkins, test it'.

rashidkpc added a commit that referenced this pull request Jun 19, 2015
Custom legends can be specified for filter aggregates
@rashidkpc rashidkpc merged commit 4d7037c into elastic:master Jun 19, 2015
2 checks passed
2 checks passed
CLA Commit author has signed the CLA
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@peterdm
Copy link

@peterdm peterdm commented Jun 23, 2015

Saw this merge. Even tested it out on master. Thanks! Extremely helpful!

However, as of today, I'm seeing commits going to master requiring Elasticsearch 2.0.0 to run Kibana.
132740d

Any plan to splice out a 4.2 Kibana release that still supports 1.4.x?

@pmoust
Copy link
Member

@pmoust pmoust commented Aug 6, 2015

Has this been in a release?

@evansendra
Copy link

@evansendra evansendra commented Aug 7, 2015

^ +1. is this released? Using kibana 4.1.1 build 7489. This is really useful for simplifying dashboards for management types.

@megglos
Copy link

@megglos megglos commented Aug 26, 2015

@K-Phoen hope you don't mind me creating this backport PR for 4.1, #4767

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

7 participants
You can’t perform that action at this time.