Skip to content

removing extra legend div from pie chart#8175

Merged
ppisljar merged 2 commits intoelastic:masterfrom
ppisljar:fix/8165
Sep 13, 2016
Merged

removing extra legend div from pie chart#8175
ppisljar merged 2 commits intoelastic:masterfrom
ppisljar:fix/8165

Conversation

@ppisljar
Copy link
Contributor

@ppisljar ppisljar commented Sep 7, 2016

fixes #8165

in pie chart layout there was an extra legend div. seems like leftover from old times as now the legend is separated from the chart.

(seems like a left over from old times)
@ppisljar ppisljar added review Feature:Vislib Vislib chart implementation labels Sep 7, 2016
@BigFunger
Copy link
Contributor

This fixes the issue that I raised.
There's a new small issue though. On a dashboard when a pie chart has the legend on the bottom or top, there's a scroll bar visible.

temp

@BigFunger
Copy link
Contributor

Also, on a related note, I don't think this was introduced by your changes, but should the collapse/expand button for the legend be visible when the panel does not have focus? Looks like we were trying to hide all widgets of a panel when it doesn't have focus.

@thomasneirynck
Copy link
Contributor

I can reproduce the scrollbar issue on:

  • Chrome
  • IE
  • Edge

but NOT on Firefox

(firefox left, Edge right)
image

.vis-container--legend-bottom & {
width: auto;
overflow-y: hidden;

Copy link
Contributor

Choose a reason for hiding this comment

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

works good, also with long legends.

image

@thomasneirynck
Copy link
Contributor

LGTM. As for hiding the expando-button when there's no focus, I agree that that's a separate issue.

@BigFunger
Copy link
Contributor

LGTM

@BigFunger BigFunger removed their assignment Sep 13, 2016
@ppisljar ppisljar merged commit 0f489c1 into elastic:master Sep 13, 2016
elastic-jasper added a commit that referenced this pull request Sep 13, 2016
---------

**Commit 1:**
removing extra legend div from pie chart
(seems like a left over from old times)

* Original sha: ef97965
* Authored by ppisljar <peter.pisljar@gmail.com> on 2016-09-07T07:47:40Z

**Commit 2:**
fixing issue with scrollbars

* Original sha: 91a8c4b
* Authored by ppisljar <peter.pisljar@gmail.com> on 2016-09-07T17:47:30Z
elastic-jasper added a commit that referenced this pull request Sep 13, 2016
---------

**Commit 1:**
removing extra legend div from pie chart
(seems like a left over from old times)

* Original sha: ef97965
* Authored by ppisljar <peter.pisljar@gmail.com> on 2016-09-07T07:47:40Z

**Commit 2:**
fixing issue with scrollbars

* Original sha: 91a8c4b
* Authored by ppisljar <peter.pisljar@gmail.com> on 2016-09-07T17:47:30Z
ppisljar added a commit that referenced this pull request Sep 13, 2016
ppisljar added a commit that referenced this pull request Sep 13, 2016
airow pushed a commit to airow/kibana that referenced this pull request Feb 16, 2017
---------

**Commit 1:**
removing extra legend div from pie chart
(seems like a left over from old times)

* Original sha: 202006606049998a9fc98a86c5602dc18f63648c [formerly ef97965]
* Authored by ppisljar <peter.pisljar@gmail.com> on 2016-09-07T07:47:40Z

**Commit 2:**
fixing issue with scrollbars

* Original sha: 5cb43d3a824b64fb5d723ef4bc74d7b30719a64a [formerly 91a8c4b]
* Authored by ppisljar <peter.pisljar@gmail.com> on 2016-09-07T17:47:30Z


Former-commit-id: 3205134
airow pushed a commit to airow/kibana that referenced this pull request Feb 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature:Vislib Vislib chart implementation review v5.0.0-beta1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Legend Positioning resulting in 'container too small' messages on Pie Chart

4 participants