Skip to content

Conversation

@logonoff
Copy link
Member

@logonoff logonoff commented Oct 1, 2025

Closes #11803

Enhanced the @patternfly/react-tokens package to include dark theme values in semantic token definitions, making it easier for developers to access both light and dark theme values programmatically.

Changes:

  • Added getDarkThemeDeclarations() to extract dark theme CSS rules
  • Added getDarkLocalVarsMap() to build dark theme variable mappings
  • Updated token generation to include darkValue and darkValues properties
  • Enhanced variable resolution to support dark theme context
  • Updated legacy token support to include dark values

Result:

  • 1,998 tokens now include dark theme values
  • Tokens with dark overrides expose darkValue property
  • Backward compatible with existing code
  • Enables programmatic theme switching and consistency

Implements: #11803

🤖 Generated with Claude Code

@rebeccaalpert
Copy link
Member

Seeing some build errors here:

Error: packages/react-charts/src/victory/components/ChartTheme/themes/base-theme.ts(164,3): error TS2322: Type '{ style: { data: { fill: "var(--pf-v6-chart-area--data--Fill, --pf-t--chart--global--fill--color--900)"; fillOpacity: 0.3; strokeWidth: "--pf-t--chart--global--stroke--width--sm"; }; labels: { ...; }; }; padding: "--pf-t--chart--global--layout--padding"; height: "--pf-t--chart--global--layout--height"; width: "--pf-...' is not assignable to type '{ style?: { data?: VictoryStyleObject; labels?: VictoryLabelStyleObject | VictoryLabelStyleObject[]; }; } & VictoryCommonThemeProps & VictoryDatableProps'.
  Type '{ style: { data: { fill: "var(--pf-v6-chart-area--data--Fill, --pf-t--chart--global--fill--color--900)"; fillOpacity: 0.3; strokeWidth: "--pf-t--chart--global--stroke--width--sm"; }; labels: { ...; }; }; padding: "--pf-t--chart--global--layout--padding"; height: "--pf-t--chart--global--layout--height"; width: "--pf-...' is not assignable to type 'VictoryCommonThemeProps'.
    Types of property 'height' are incompatible.
      Type 'string' is not assignable to type 'number'.
Error: packages/react-charts/src/victory/components/ChartTheme/themes/base-theme.ts(177,3): error TS2322: Type '{ style: { axis: { fill: "var(--pf-v6-chart-axis--axis--Fill, transparent)"; strokeWidth: "--pf-t--chart--global--stroke--width--xs"; stroke: "var(--pf-v6-chart-axis--axis--stroke--Color, --pf-t--chart--global--fill--color--300)"; strokeLinecap: "--pf-t--chart--global--stroke-line-cap"; strokeLinejoin: "--pf-t--char...' is not assignable to type '{ style?: { axis?: VictoryStyleObject; axisLabel?: VictoryLabelStyleObject | VictoryLabelStyleObject[]; grid?: VictoryStyleObject; ticks?: VictoryTickStyleObject; tickLabels?: VictoryLabelStyleObject | VictoryLabelStyleObject[]; }; offsetX?: number; offsetY?: number; } & VictoryCommonThemeProps'.
  Type '{ style: { axis: { fill: "var(--pf-v6-chart-axis--axis--Fill, transparent)"; strokeWidth: "--pf-t--chart--global--stroke--width--xs"; stroke: "var(--pf-v6-chart-axis--axis--stroke--Color, --pf-t--chart--global--fill--color--300)"; strokeLinecap: "--pf-t--chart--global--stroke-line-cap"; strokeLinejoin: "--pf-t--char...' is not assignable to type 'VictoryCommonThemeProps'.
    Types of property 'height' are incompatible.
      Type 'string' is not assignable to type 'number'.
Error: packages/react-charts/src/victory/components/ChartTheme/themes/base-theme.ts(226,3): error TS2322: Type '{ style: { max: { padding: 8; stroke: "var(--pf-v6-chart-boxplot--max--stroke--Color, --pf-t--chart--global--fill--color--900)"; strokeWidth: "--pf-t--chart--global--stroke--width--xs"; }; ... 8 more ...; q3Labels: { ...; }; }; boxWidth: 20; padding: "--pf-t--chart--global--layout--padding"; height: "--pf-t--chart--...' is not assignable to type '{ style?: { max?: VictoryStyleObject; maxLabels?: VictoryLabelStyleObject | VictoryLabelStyleObject[]; median?: VictoryStyleObject; ... 6 more ...; q3Labels?: VictoryLabelStyleObject | VictoryLabelStyleObject[]; }; boxWidth?: number; } & VictoryCommonThemeProps'.
  Type '{ style: { max: { padding: 8; stroke: "var(--pf-v6-chart-boxplot--max--stroke--Color, --pf-t--chart--global--fill--color--900)"; strokeWidth: "--pf-t--chart--global--stroke--width--xs"; }; ... 8 more ...; q3Labels: { ...; }; }; boxWidth: 20; padding: "--pf-t--chart--global--layout--padding"; height: "--pf-t--chart--...' is not assignable to type 'VictoryCommonThemeProps'.
    Types of property 'height' are incompatible.
      Type 'string' is not assignable to type 'number'.
Error: packages/react-charts/src/victory/components/ChartTheme/themes/base-theme.ts(260,3): error TS2322: Type '{ candleColors: { positive: "var(--pf-v6-chart-candelstick--candle--positive--Color, --pf-t--chart--global--fill--color--white)"; negative: "var(--pf-v6-chart-candelstick--candle--negative--Color, --pf-t--chart--global--fill--color--900)"; }; style: { ...; }; padding: "--pf-t--chart--global--layout--padding"; height...' is not assignable to type '{ style?: { data?: VictoryStyleObject; labels?: VictoryLabelStyleObject | VictoryLabelStyleObject[]; }; candleColors?: { ...; }; } & VictoryCommonThemeProps'.
  Type '{ candleColors: { positive: "var(--pf-v6-chart-candelstick--candle--positive--Color, --pf-t--chart--global--fill--color--white)"; negative: "var(--pf-v6-chart-candelstick--candle--negative--Color, --pf-t--chart--global--fill--color--900)"; }; style: { ...; }; padding: "--pf-t--chart--global--layout--padding"; height...' is not assignable to type 'VictoryCommonThemeProps'.
    Types of property 'height' are incompatible.
      Type 'string' is not assignable to type 'number'.
Error: packages/react-charts/src/victory/components/ChartTheme/themes/base-theme.ts(274,3): error TS2322: Type '{ padding: "--pf-t--chart--global--layout--padding"; height: "--pf-t--chart--global--layout--height"; width: "--pf-t--chart--global--layout--width"; }' is not assignable to type 'VictoryCommonThemeProps'.
  Types of property 'height' are incompatible.
    Type 'string' is not assignable to type 'number'.
Error: packages/react-charts/src/victory/components/ChartTheme/themes/base-theme.ts(279,5): error TS2322: Type 'string' is not assignable to type 'number'.
Error: packages/react-charts/src/victory/components/ChartTheme/themes/base-theme.ts(290,3): error TS2322: Type '{ padding: "--pf-t--chart--global--layout--padding"; height: "--pf-t--chart--global--layout--height"; width: "--pf-t--chart--global--layout--width"; }' is not assignable to type '{ style?: { data?: VictoryStyleObject; labels?: VictoryLabelStyleObject | VictoryLabelStyleObject[]; }; } & VictoryCommonThemeProps'.
  Type '{ padding: "--pf-t--chart--global--layout--padding"; height: "--pf-t--chart--global--layout--height"; width: "--pf-t--chart--global--layout--width"; }' is not assignable to type 'VictoryCommonThemeProps'.
    Types of property 'height' are incompatible.
      Type 'string' is not assignable to type 'number'.
Error: packages/react-charts/src/victory/components/ChartTheme/themes/base-theme.ts(309,3): error TS2322: Type '{ style: { data: { fill: "var(--pf-v6-chart-line--data--Fill, transparent)"; opacity: 1; stroke: "var(--pf-v6-chart-line--data--stroke--Color, --pf-t--chart--global--fill--color--900)"; strokeWidth: "--pf-t--chart--global--stroke--width--sm"; }; labels: { ...; }; }; padding: "--pf-t--chart--global--layout--padding";...' is not assignable to type '{ style?: { data?: VictoryStyleObject; labels?: VictoryLabelStyleObject | VictoryLabelStyleObject[]; }; } & VictoryCommonThemeProps & VictoryDatableProps'.
  Type '{ style: { data: { fill: "var(--pf-v6-chart-line--data--Fill, transparent)"; opacity: 1; stroke: "var(--pf-v6-chart-line--data--stroke--Color, --pf-t--chart--global--fill--color--900)"; strokeWidth: "--pf-t--chart--global--stroke--width--sm"; }; labels: { ...; }; }; padding: "--pf-t--chart--global--layout--padding";...' is not assignable to type 'VictoryCommonThemeProps'.
    Types of property 'height' are incompatible.
      Type 'string' is not assignable to type 'number'.
Error: packages/react-charts/src/victory/components/ChartTheme/themes/base-theme.ts(337,3): error TS2322: Type '{ style: { data: { fill: "var(--pf-v6-chart-scatter--data--Fill, --pf-t--chart--global--fill--color--900)"; opacity: 1; stroke: "var(--pf-v6-chart-scatter--data--stroke--Color, transparent)"; strokeWidth: 0; }; labels: { ...; }; }; padding: "--pf-t--chart--global--layout--padding"; height: "--pf-t--chart--global--la...' is not assignable to type '{ style?: { data?: VictoryStyleObject; labels?: VictoryLabelStyleObject | VictoryLabelStyleObject[]; }; } & VictoryCommonThemeProps & VictoryDatableProps'.
  Type '{ style: { data: { fill: "var(--pf-v6-chart-scatter--data--Fill, --pf-t--chart--global--fill--color--900)"; opacity: 1; stroke: "var(--pf-v6-chart-scatter--data--stroke--Color, transparent)"; strokeWidth: 0; }; labels: { ...; }; }; padding: "--pf-t--chart--global--layout--padding"; height: "--pf-t--chart--global--la...' is not assignable to type 'VictoryCommonThemeProps'.
    Types of property 'height' are incompatible.
      Type 'string' is not assignable to type 'number'.
Error: packages/react-charts/src/victory/components/ChartTheme/themes/base-theme.ts(374,3): error TS2322: Type '{ style: { data: { fill: "var(--pf-v6-chart-voronoi--data--Fill, transparent)"; stroke: "var(--pf-v6-chart-voronoi--data--stroke--Color, transparent)"; strokeWidth: 0; }; labels: { fill: "var(--pf-v6-chart-voronoi--labels--Fill, --pf-t--chart--global--fill--color--200)"; ... 6 more ...; stroke: "var(--pf-v6-chart-gl...' is not assignable to type '{ style?: { data?: VictoryStyleObject; labels?: VictoryLabelStyleObject | VictoryLabelStyleObject[]; flyout?: VictoryStyleObject; }; } & VictoryCommonThemeProps & VictoryDatableProps'.
  Type '{ style: { data: { fill: "var(--pf-v6-chart-voronoi--data--Fill, transparent)"; stroke: "var(--pf-v6-chart-voronoi--data--stroke--Color, transparent)"; strokeWidth: 0; }; labels: { fill: "var(--pf-v6-chart-voronoi--labels--Fill, --pf-t--chart--global--fill--color--200)"; ... 6 more ...; stroke: "var(--pf-v6-chart-gl...' is not assignable to type 'VictoryCommonThemeProps'.
    Types of property 'height' are incompatible.
      Type 'string' is not assignable to type 'number'.
Error: packages/react-charts/src/victory/components/ChartUtils/chart-label.ts(105,34): error TS2362: The left-hand side of an arithmetic operation must be of type 'any', 'number', 'bigint' or an enum type.
Error: packages/react-charts/src/victory/components/ChartUtils/chart-legend.ts(149,8): error TS2365: Operator '>' cannot be applied to types 'string | number' and 'number'.
Error: packages/react-charts/src/victory/components/ChartBullet/ChartBulletTitle.tsx(207,41): error TS2362: The left-hand side of an arithmetic operation must be of type 'any', 'number', 'bigint' or an enum type.

@logonoff logonoff force-pushed the 11803-dark-theme-tokens branch from 4d56fec to 733ca08 Compare October 30, 2025 16:58
@patternfly-build
Copy link
Collaborator

patternfly-build commented Oct 30, 2025

Closes patternfly#11803

Enhanced the @patternfly/react-tokens package to include dark theme
values in semantic token definitions, making it easier for developers
to access both light and dark theme values programmatically.

Changes:
- Added getDarkThemeDeclarations() to extract dark theme CSS rules
- Added getDarkLocalVarsMap() to build dark theme variable mappings
- Updated token generation to include darkValue and darkValues properties
- Enhanced variable resolution to support dark theme context
- Updated legacy token support to include dark values

Result:
- 1,998 tokens now include dark theme values
- Tokens with dark overrides expose darkValue property
- Backward compatible with existing code
- Enables programmatic theme switching and consistency

Implements: patternfly#11803

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@logonoff logonoff force-pushed the 11803-dark-theme-tokens branch from 733ca08 to 9cb59a1 Compare October 30, 2025 17:13
@rebeccaalpert rebeccaalpert requested review from a team, dlabaj and nicolethoen and removed request for a team October 31, 2025 18:37
Copy link
Contributor

@nicolethoen nicolethoen left a comment

Choose a reason for hiding this comment

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

Thank you for this PR! I looked a bit closer into the implementation and token generation and tested it locally.

When i ran it locally to generate the tokens, i'm only seeing 476 tokens have dark values. I think it should be many more than that.

It's currently only extracting dark values for tokens explicitly overridden in dark theme CSS blocks, but does not resolve dark values through the variable dependency chain.

Example:

The semantic token has a dark value:

// t_global_text_color_regular.js
{
  "name": "--pf-t--global--text--color--regular",
  "value": "#151515",      // Light theme: dark text
  "darkValue": "#ffffff"   // Dark theme: white text
}

But the component token that references it does not:
// c_content.js
"c_content_Color": {
  "name": "--pf-v6-c-content--Color",
  "value": "#151515",      // Light theme: resolved correctly
  "values": [
    "--pf-t--global--text--color--regular",  // References token with darkValue
    "--pf-t--global--text--color--100",
    "--pf-t--color--gray--95",
    "#151515"
  ]
  // Missing: "darkValue": "#ffffff"
}

The implementation needs to recursively resolve dark values through the dependency chain. When getVarsMap() encounters a reference to another token, it should check if that token has a dark override and propagate that through the resolution. This would make the feature significantly more useful for developers trying to programmatically access theme values.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

react-tokens - include dark theme value in semantic token definitions

4 participants