17488 legend color picker bugfix

[varunsharma27]

#17488 legend color picker bugfix

[elasticmachine]

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?

[timroes]

Jenkins, test this

[elasticmachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request

[thomasneirynck]

jenkins test this

[thomasneirynck]

hi @varundbest

thanks for this. Seems to fix it

[elasticmachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request

[ppisljar]

there are a couple of issues:

• left/right positioned legend: opening the color picker covers all the other legend entries below the current one
  (this might not be bad, but its a change from current behaviour)

• top legend: chart still shifts down a bit, since "filter in" and "filter out" buttons appear on a new line

• bottom legend: color picker is not visible at all

[timroes]

In regards to building a custom chart component in EUI (elastic/eui#536) and replace our existing chart implementations by that, I wonder if we should touch this topic at all for the existing legend, if it's not that simple of a fix, and can cause multiple edge cases.

[varunsharma27]

• Added a Special case for Top. Right and Left remains unchanged.

@ppisljar
How do you suggest to tackle Bottom?
I can make it similar to Top and set css 'bottom: 5% or 10%' but that might break a little (Position might be a little far or close) if the height of vis panel is too small or too large, what do you suggest?

[ppisljar]

what about fixed bottom offset ? bottom: 0 or similar ?

[varunsharma27]

@ppisljar Then the color picker will hide the legend toggle and user will be unable to get rid of it.
How to you feel about:-

position:  absolute;
bottom:  5%;
margin-bottom: 15px;
background-color:  white;

For bottom case?

[ppisljar]

can you try applying all of the above to this PR so i can test it out easily ? thanks!

[varunsharma27]

@ppisljar Changes Pushed.

[timroes]

Could you please rebase that on master, so we can get all the tests running with x-pack together and better check if there isn't an issue with regards to reporting?

[timroes]

Jenkins, test this

[elasticmachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request

[varunsharma27]

@ppisljar What do you feel?

[ppisljar]

sorry for late response ...

there are still some issues:

clicking on a legend entry will move the chart for a pixel or two:

if legend spreads multiple rows chart is not positioned correctly:

Same happens for both the top and bottom position. Do you think those could be addressed ?

Thanks!

[varunsharma27]

Sure, will look into those issues.

[rashmivkulkarni]

can you pull master into this branch and re run the tests and address the feedback. Thanks.

[elasticmachine]

💔 Build Failed

• continuous-integration/kibana-ci/pull-request

[flash1293]

Since so much has changed since this PR was opened (less to sass, new class names, new markup structure...) I will close this PR. The bug will be fixed by #30960

Thank you very much @varunsharma27 for working on this - your changes were the starting point for the new PR.