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.

Sign up for GitHub

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

Jump to bottom

Axis Labels Filter - better geometry assessment #16130

Merged
merged 1 commit into from
May 24, 2018
Merged

Axis Labels Filter - better geometry assessment #16130

merged 1 commit into from
May 24, 2018

Conversation

daniele-pini
Copy link

Fixes #13590

Scales in line/area/bar charts are often of type 'rangeBands', that
applies padding on the sides and leaves space for the bands.

During axis label filtering, this padding was not taken in
consideration.

@elasticmachine
Copy link
Contributor

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@daniele-pini
Copy link
Author

I have signed the CLA and received the confirmation by mail ("Individual Contributor License Agreement between elasticsearch BV and Daniele Pini is Signed and Filed!").

Do I have to do something more to pass the check?

@daniele-pini
Copy link
Author

Uh. As soon as I commented, the check passed... 😑

@timroes
Copy link
Contributor

timroes commented Feb 9, 2018

Jenkins, test this

@timroes
Copy link
Contributor

timroes commented Feb 9, 2018

Actually @daniele-pini you needed to do exactly that, leaving another comment to trigger the check :D

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@Bargs Bargs added review Feature:Visualizations Generic visualization features (in case no more specific feature label is available) labels Feb 14, 2018
@tsullivan tsullivan requested a review from bhavyarm March 28, 2018 19:16
@tsullivan
Copy link
Member

Hi @bhavyarm can you pull this in and see if it resolves the issue you filed?

@elasticmachine
Copy link
Contributor

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@elasticmachine
Copy link
Contributor

💔 Build Failed

@bhavyarm
Copy link
Contributor

bhavyarm commented May 1, 2018 •
edited
Loading

@tsullivan yes I can. I have a 5.6.9 in my local right now. Testing it. Sorry for the delay.

@tsullivan
Copy link
Member

jenkins test this

@elasticmachine
Copy link
Contributor

💔 Build Failed

@tsullivan
Copy link
Member

Hi @daniele-pini, tests might be failing because the baseline of the kibana code might be pretty old.

Would you mind updating your local master branch and merging that in to this one?

Axis Labels Filter - better geometry assessment
566af83 
Fixes elastic#13590

Scales in line/area/bar charts are often of type 'rangeBands', that
applies padding on the sides and leaves space for the bands.

During axis label filtering, this padding was not taken in
consideration.
@daniele-pini
Copy link
Author

Sorry for the delay, I've rebased it on current master.

@timroes timroes requested review from timroes and ppisljar May 4, 2018 16:59
@tsullivan tsullivan requested review from timroes and removed request for timroes May 4, 2018 16:59
bhavyarm
bhavyarm reviewed May 8, 2018
View reviewed changes
Copy link
Contributor

@bhavyarm bhavyarm left a comment

Choose a reason for hiding this comment

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

So I tested this in my local on 5.6.9 and t looks good. But I don't know about the code. I would rather if @timroes or @ppisljar do LGTM in the code.

ppisljar
ppisljar approved these changes May 23, 2018
View reviewed changes
Copy link
Member

@ppisljar ppisljar left a comment

Choose a reason for hiding this comment

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

LGTM

@timroes timroes merged commit 294bc30 into elastic:master May 24, 2018
@timroes timroes removed the review label May 24, 2018
@timroes timroes mentioned this pull request May 24, 2018
Merged
timroes pushed a commit to timroes/kibana that referenced this pull request May 24, 2018
@timroes
Axis Labels Filter - better geometry assessment (elastic#16130)
756aca0 
Fixes elastic#13590

Scales in line/area/bar charts are often of type 'rangeBands', that
applies padding on the sides and leaves space for the bands.

During axis label filtering, this padding was not taken in
consideration.
@daniele-pini daniele-pini deleted the axis_labels_filter_fix branch May 24, 2018 11:04
timroes added a commit that referenced this pull request May 24, 2018
@timroes
Axis Labels Filter - better geometry assessment (#16130) (#19389)
d473668 
Fixes #13590

Scales in line/area/bar charts are often of type 'rangeBands', that
applies padding on the sides and leaves space for the bands.

During axis label filtering, this padding was not taken in
consideration.
@markov00 markov00 mentioned this pull request Jul 31, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bhavyarm bhavyarm bhavyarm left review comments

@ppisljar ppisljar ppisljar approved these changes

@timroes timroes Awaiting requested review from timroes

Assignees
No one assigned
Labels
Feature:Visualizations Generic visualization features (in case no more specific feature label is available) v6.4.0 v7.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@daniele-pini @elasticmachine @timroes @tsullivan @bhavyarm @ppisljar @Bargs