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

[Lens] Pie/Treemap chart external labels are hard to read when in dark mode #116777

Closed
dej611 opened this issue Oct 29, 2021 · 14 comments · Fixed by #120271
Closed

[Lens] Pie/Treemap chart external labels are hard to read when in dark mode #116777

dej611 opened this issue Oct 29, 2021 · 14 comments · Fixed by #120271
Labels
blocked bug Fixes for quality problems that affect the customer experience Feature:Lens impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:large Large Level of Effort Team:Visualizations Visualization editors, elastic-charts and infrastructure

Comments

@dej611
Copy link
Contributor

dej611 commented Oct 29, 2021

Describe the bug:

Labels on the outside are of the slice (pie) or box (treemap) are pretty hard to read when kibana is in dark mode:

Screenshot 2021-10-29 at 16 48 06

The color should automatically adjust based on the theme.

@dej611 dej611 added bug Fixes for quality problems that affect the customer experience Team:Visualizations Visualization editors, elastic-charts and infrastructure Feature:Lens labels Oct 29, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-vis-editors (Team:VisEditors)

@dej611
Copy link
Contributor Author

dej611 commented Oct 29, 2021

Apparently the EC library already supports this, so it requires some configuration tweak in the current Lens implementation.

Pie chart: https://elastic.github.io/elastic-charts/?path=/story/sunburst--with-linked-text-labels&globals=theme:eui-dark

Treemap: https://elastic.github.io/elastic-charts/?path=/story/treemap--mid-two-layers&globals=theme:eui-dark;background:black

@flash1293 flash1293 added the impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. label Oct 29, 2021
@dej611 dej611 self-assigned this Nov 8, 2021
@dej611
Copy link
Contributor Author

dej611 commented Nov 8, 2021

After some investigations, found a few blocking issue on Canvas and Elastic Charts sides:

@flash1293
Copy link
Contributor

Not blocked anymore - reducing impact because it works correctly in most places now

@flash1293 flash1293 added impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. and removed blocked impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. labels Jan 12, 2022
@flash1293 flash1293 removed the Team:Visualizations Visualization editors, elastic-charts and infrastructure label Feb 10, 2022
@botelastic botelastic bot added the needs-team Issues missing a team label label Feb 10, 2022
@flash1293 flash1293 added Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas Team:Visualizations Visualization editors, elastic-charts and infrastructure and removed needs-team Issues missing a team label labels Feb 10, 2022
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-presentation (Team:Presentation)

@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-vis-editors @elastic/kibana-vis-editors-external (Team:VisEditors)

@flash1293
Copy link
Contributor

flash1293 commented Feb 10, 2022

@elastic/kibana-presentation How do you think this should be fixed for the Canvas case? Should there be a "on dark background" flag on the the lensEmbeddable function which is passed down to the chart? Should it try to infer it from the background color somehow?

Blocked on presentation team decision how Canvas wants to handle this case (manually by user selection or somehow automatically)

@thekofimensah
Copy link

I also ran into this issue and would be great if I could use lens in dark mode!

@flash1293
Copy link
Contributor

Bumping this - @ThomThomson do you think it's worth putting this on the near term roadmap from the presentation team side of things?

@ThomThomson
Copy link
Contributor

Hey @flash1293! @teresaalvarezsoler can correct me if I'm wrong, but we aren't focused on any enhancement requests in Canvas for the near-term. We are only doing maintenance work and major bug fixes.

That said, I would say the best course of action is to just have these labels infer their color solely based on theme.darkMode, and not try to infer anything based on the user-configured background color. It's definitely not ideal, but if the user has a workpad with a dark background that makes it hard to see the external labels, they can turn on dark mode.

The most canvas thing to do here would be to allow the color of those external labels to be user-configurable by exposing linkLabels.textColor through the expression. That said, I'm not sure if it's worth the work.

@flash1293
Copy link
Contributor

Makes sense, happy with parking this for now

@teresaalvarezsoler
Copy link

Thanks Joe, we will consider this for the future. As Devon said, we are not prioritizing Canvas enhancements for now.

@ThomThomson ThomThomson added the loe:large Large Level of Effort label Jul 12, 2022
@ThomThomson
Copy link
Contributor

@elastic/kibana-visualizations, I'm removing the Presentation team label from this because I don't believe we'll be able to prioritize this change in Canvas. Going to leave it open though, because it seems like it might be a problem with Lens more generally as well? Feel free to close if it's been resolved on that side.

@ThomThomson ThomThomson removed the Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas label Apr 12, 2023
@nickofthyme
Copy link
Contributor

I verified this is no longer an issue for Lens and Aggs-based pie charts after elastic/elastic-charts#1402 was introduced. The issue would still remain if charts consumers elsewhere in kibana do to not apply a background to the chart. The background defaults to transparent however an opaque background is required for contrast logic to work.

There is a separate issue elastic/elastic-charts#718, that could be the solution for fixing this across all chart usage in kibana but this is not a high priority at the moment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked bug Fixes for quality problems that affect the customer experience Feature:Lens impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:large Large Level of Effort Team:Visualizations Visualization editors, elastic-charts and infrastructure
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants