Charts - update for dark theme support #7188

Activity

1. evwilkin

   added

   needs triage

   on Apr 7, 2022
2. evwilkin

   added this to PatternFly Issueson Apr 7, 2022
3. mcoker

   added

   bug 🐛

   and removed

   needs triage

   on Apr 14, 2022
4. mcoker

   self-assigned this

   on Apr 14, 2022
5. nicolethoen

   moved this to In Progress in PatternFly Issueson Apr 14, 2022
6. mcarrano

   added this to the 2022.06 milestone on Apr 20, 2022
7. dlabrecq commented on Apr 21, 2022

   

   dlabrecq

   on Apr 21, 2022

   Last edited by dlabrecq

   Member

   More actions

   The charts do pull PatternFly styles from react-tokens, but we use the variable's value. We don't output the variable's name (var) in the SVG.

   The reason we must do this is related to font size, padding, chart height / width, etc. For example, in order to position legend items horizontally, we must calculate the total width of the legend text, icon, etc. Then, we subtract that from the SVG width, padding, etc. If we used a variable name, the text width and padding is unknown until the SVG is rendered in the page.

   There may be cases where we can output the variable, instead? For example, we may do this for the color pallet. However, probably not for font size, padding, etc. -- nothing used to calculate widths.

   Note that for the tooltip, the chart theme defines specific properties for fill (background), border, stroke (border), strokeWidth, etc. For example,

   tooltip: {
     cornerRadius: chart_tooltip_corner_radius.value,
     flyoutPadding: chart_tooltip_Padding.value,
     flyoutStyle: {
       cornerRadius: chart_tooltip_flyoutStyle_corner_radius.value,
       fill: chart_tooltip_flyoutStyle_Fill.value, // background
       pointerEvents: chart_tooltip_flyoutStyle_PointerEvents.value,
       stroke: chart_tooltip_flyoutStyle_stroke_Color.value, // border
       strokeWidth: chart_tooltip_flyoutStyle_stroke_Width.value
     },
     pointerLength: chart_tooltip_pointer_length.value,
     pointerWidth: chart_tooltip_pointer_Width.value,
     style: {
       fill: chart_tooltip_Fill.value, // text
       pointerEvents: chart_tooltip_PointerEvents.value
     }
   },
   

   It would be good to meet and discuss how this is expected to work. For example, is PatternFly redefining the --pf-chart-tooltip--flyoutStyle--Fill variable above for light and dark charts?

   That said, the charts already have a light and dark theme available as a theme variant -- it's been there from day one. Although, the default has always been to show the light theme.

   The intention was to specify a light or dark variant at the component level. And the light and dark chart themes would define the expected properties for tooltips, legends, etc.
8. dlabrecq commented on Apr 21, 2022

   

   dlabrecq

   on Apr 21, 2022

   Last edited by dlabrecq

   Member

   More actions

   Please note that we did have different designs for light and dark chart themes below. At least, the multi-color pallets were ordered differently here.

   https://drive.google.com/file/d/1jCxBH0niB78V0PJvzHDBr8Hm5wKmDlgH/view

   This was one reason for creating light and dark variants at the time. That is, to change the order of how some color pallets are applied to data.

   For example:

   
9. evwilkin commented on Apr 21, 2022

   

   evwilkin

   on Apr 21, 2022

   MemberAuthor

   More actions

   Notes from further discussion between @mcoker and @dlabrecq :

   • Consider the ordered theme and unordered theme colors which ensure adequate contrast between adjacent data points in a chart (ordered example, unordered example)

   • Consider the individual color themes to ensure contrast between shades of one color

   • Dark theme note from @mceledonia

     In dark theme I'm finding the darker colors don't work at all and need to be removed or altered which makes the whole palette for that color need to shift

   • Testing:

     • Review the themed charts
     • Review the legends such as the embedded legend to confirm dark theme compatibility in custom tooltip implementations
10. nicolethoen

    moved this from In Progress to PR Review in PatternFly Issueson May 3, 2022
11. tlabaj

    closed this as completedin #7310on May 9, 2022
12. Repository owner moved this from PR Review to Done in PatternFly Issueson May 9, 2022
13. evwilkin

    added

    dark theme

    on May 18, 2022