Skip to content

Conversation

@mannycarrera4
Copy link
Contributor

Summary

Fixes: #1234

Release Category

Components

Release Note

Optional release note message. Changelog and release summaries will contain a pull request title. This section will add additional notes under that title. This section is not a summary, but something extra to point out in release notes. An example might be calling out breaking changes in a labs component or minor visual changes that need visual regression updates. Remove this section if no additional release notes are required.

BREAKING CHANGES

Optional breaking changes message. If your PR includes breaking changes. It is extremely rare to put breaking changes outside a prerelease/major branch. Anything in this section will show up in release notes. Remove this section if no breaking changes are present.


Checklist

For the Reviewer

  • PR title is short and descriptive
  • PR summary describes the change (Fixes/Resolves linked correctly)
  • PR Release Notes describes additional information useful to call out in a release message or removed if not applicable
  • Breaking Changes provides useful information to upgrade to this code or removed if not applicable

Where Should the Reviewer Start?

Areas for Feedback? (optional)

  • Code
  • Documentation
  • Testing
  • Codemods

Testing Manually

Screenshots or GIFs (if applicable)

Thank You Gif (optional)

@cypress
Copy link

cypress bot commented Jan 7, 2026

Workday/canvas-kit    Run #10040

Run Properties:  status check passed Passed #10040  •  git commit 17ffe424e4 ℹ️: Merge 27843c00854614d8678ded09021e040fc1f042ea into 54d08f77d0527af840c9f102ce9a...
Project Workday/canvas-kit
Branch Review mc-palette-generator
Run status status check passed Passed #10040
Run duration 02m 20s
Commit git commit 17ffe424e4 ℹ️: Merge 27843c00854614d8678ded09021e040fc1f042ea into 54d08f77d0527af840c9f102ce9a...
Committer Manuel Carrera
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 17
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 800
View all changes introduced in this branch ↗︎
UI Coverage  19.39%
  Untested elements 1520  
  Tested elements 363  
Accessibility  99.5%
  Failed rules  5 critical   5 serious   0 moderate   2 minor
  Failed elements 71  

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds a comprehensive palette generator feature to Canvas Kit that creates WCAG-compliant color palettes from any input color using the OKLCH color space.

Changes:

  • Adds palette generation utilities that create 13-step accessible color scales
  • Introduces colorjs.io dependency (v0.6.0) for color space conversions
  • Includes an interactive documentation example with visual palette preview

Reviewed changes

Copilot reviewed 9 out of 11 changed files in this pull request and generated 9 comments.

Show a summary per file
File Description
package.json Adds colorjs.io dependency to root package
yarn.lock Locks colorjs.io@^0.6.0 dependency
modules/react/common/lib/theming/palette/types.ts Defines TypeScript types for palette generation
modules/react/common/lib/theming/palette/index.ts Exports palette generation API
modules/react/common/lib/theming/palette/generateAccessiblePalette.ts Core palette generation algorithm
modules/react/common/lib/theming/palette/gamut.ts Gamut calculation utilities for sRGB and P3
modules/react/common/lib/theming/palette/conversion.ts Color space conversion utilities
modules/react/common/lib/theming/palette/colorjs.d.ts TypeScript declarations for colorjs.io
modules/react/common/lib/theming/index.ts Re-exports palette utilities from theming module
modules/react/common/stories/mdx/PaletteGenerator.mdx Interactive documentation for palette generator
modules/react/common/stories/mdx/examples/PaletteGenerator.tsx React component demonstrating palette generation
Comments suppressed due to low confidence (1)

modules/react/common/lib/theming/palette/generateAccessiblePalette.ts:1

  • The comment states "Amount of hue shift to apply (0-10)" but the implementation doesn't enforce these bounds. The slider in PaletteGenerator.tsx uses max="30", which contradicts the documented range. Either update the documentation to match the actual allowed range (0-30) or add validation to enforce the 0-10 range.
/**

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +157 to +158
"dependencies": {
"colorjs.io": "^0.6.0"
Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

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

The colorjs.io dependency is added to the root package.json, but for a monorepo using lerna/yarn workspaces, dependencies should typically be added to the specific module's package.json (modules/react/common/package.json) rather than the root. This ensures proper dependency isolation and allows individual modules to be published independently. If modules/react/common doesn't have its own package.json, this may need to be addressed in the build configuration.

Copilot uses AI. Check for mistakes.
Comment on lines +84 to +85
// Create background color based on luminance
const bgValue = backgroundLuminance > 0.5 ? 1 : 0;
Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

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

The background color calculation uses a simple binary threshold (>0.5) to determine if the background should be white (1) or black (0). However, the backgroundLuminance parameter can have any value between 0-1, and this binary conversion loses that precision. This could result in inaccurate contrast calculations for mid-tone backgrounds. Consider using the actual backgroundLuminance value to create a more accurate background color for contrast calculations.

Suggested change
// Create background color based on luminance
const bgValue = backgroundLuminance > 0.5 ? 1 : 0;
// Create background color based on provided luminance (clamped to [0, 1])
const bgValue = clamp(backgroundLuminance, 0, 1);

Copilot uses AI. Check for mistakes.

## Mapping to Canvas Tokens

The primary use case for this tool is to generate values for `@workday/canvas-tokens-web` brand tokens. The palette steps map to brand tokens as follows:
Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

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

The documentation refers to this as "this tool" but it would be clearer to refer to it as "the palette generator" or "generateAccessiblePalette" for consistency with the rest of the documentation.

Suggested change
The primary use case for this tool is to generate values for `@workday/canvas-tokens-web` brand tokens. The palette steps map to brand tokens as follows:
The primary use case for the palette generator is to generate values for `@workday/canvas-tokens-web` brand tokens. The palette steps map to brand tokens as follows:

Copilot uses AI. Check for mistakes.
Comment on lines +232 to +238
const rgb = hex
.replace('#', '')
.match(/.{2}/g)!
.map(x => {
const c = parseInt(x, 16) / 255;
return c <= 0.03928 ? c / 12.92 : Math.pow((c + 0.055) / 1.055, 2.4);
});
Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

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

Using the non-null assertion operator (!) assumes that match() will never return null, but this could fail for invalid hex strings. While the code checks for null lightHex/darkHex earlier, it's better practice to handle the null case explicitly here or add a type guard to ensure the regex always matches.

Suggested change
const rgb = hex
.replace('#', '')
.match(/.{2}/g)!
.map(x => {
const c = parseInt(x, 16) / 255;
return c <= 0.03928 ? c / 12.92 : Math.pow((c + 0.055) / 1.055, 2.4);
});
const parts = hex.replace('#', '').match(/.{2}/g);
if (!parts || parts.length < 3) {
// Fallback for invalid hex strings
return 0;
}
const rgb = parts.slice(0, 3).map(x => {
const c = parseInt(x, 16) / 255;
return c <= 0.03928 ? c / 12.92 : Math.pow((c + 0.055) / 1.055, 2.4);
});

Copilot uses AI. Check for mistakes.
minChroma?: number;

/**
* Amount of hue shift to apply (0-10)
Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

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

The comment states "Amount of hue shift to apply (0-10)" but the slider in PaletteGenerator.tsx uses max="30", which contradicts the documented range. This should be updated to accurately reflect the allowed range.

Suggested change
* Amount of hue shift to apply (0-10)
* Amount of hue shift to apply (0-30)

Copilot uses AI. Check for mistakes.
};

const copyToClipboard = (step: PaletteStep) => {
navigator.clipboard.writeText(step.hex);
Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

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

All clipboard write operations lack error handling. The Clipboard API can fail in various scenarios (permissions not granted, browser doesn't support it, etc.). Consider adding .catch() handlers or try-catch blocks to gracefully handle these failures and potentially show an error message to the user.

Copilot uses AI. Check for mistakes.
acc[step.step] = step.hex;
return acc;
}, {} as Record<number, string>);
navigator.clipboard.writeText(JSON.stringify(colorMap, null, 2));
Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

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

All clipboard write operations lack error handling. The Clipboard API can fail in various scenarios (permissions not granted, browser doesn't support it, etc.). Consider adding .catch() handlers or try-catch blocks to gracefully handle these failures and potentially show an error message to the user.

Copilot uses AI. Check for mistakes.
};

const copyTokenValue = (text: string, token: string) => {
navigator.clipboard.writeText(text);
Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

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

All clipboard write operations lack error handling. The Clipboard API can fail in various scenarios (permissions not granted, browser doesn't support it, etc.). Consider adding .catch() handlers or try-catch blocks to gracefully handle these failures and potentially show an error message to the user.

Copilot uses AI. Check for mistakes.
Comment on lines +323 to +350
const copyToClipboard = (step: PaletteStep) => {
navigator.clipboard.writeText(step.hex);
setCopiedStep(step.step);
setTimeout(() => setCopiedStep(null), 1500);
};

const copyAllColors = () => {
const colorMap = palette.steps.reduce((acc, step) => {
acc[step.step] = step.hex;
return acc;
}, {} as Record<number, string>);
navigator.clipboard.writeText(JSON.stringify(colorMap, null, 2));
setCopiedStep(-1);
setTimeout(() => setCopiedStep(null), 1500);
};

const copyTokenValue = (text: string, token: string) => {
navigator.clipboard.writeText(text);
setCopiedToken(token);
setTimeout(() => setCopiedToken(null), 1500);
};

const copyAllBrandTokens = () => {
const css = BRAND_TOKEN_MAPPING.map(
({step, cssVar}) => ` ${cssVar}: ${palette.getHex(step)};`
).join('\n');
const fullCss = `:root {\n${css}\n}`;
navigator.clipboard.writeText(fullCss);
Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

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

All clipboard write operations lack error handling. The Clipboard API can fail in various scenarios (permissions not granted, browser doesn't support it, etc.). Consider adding .catch() handlers or try-catch blocks to gracefully handle these failures and potentially show an error message to the user.

Suggested change
const copyToClipboard = (step: PaletteStep) => {
navigator.clipboard.writeText(step.hex);
setCopiedStep(step.step);
setTimeout(() => setCopiedStep(null), 1500);
};
const copyAllColors = () => {
const colorMap = palette.steps.reduce((acc, step) => {
acc[step.step] = step.hex;
return acc;
}, {} as Record<number, string>);
navigator.clipboard.writeText(JSON.stringify(colorMap, null, 2));
setCopiedStep(-1);
setTimeout(() => setCopiedStep(null), 1500);
};
const copyTokenValue = (text: string, token: string) => {
navigator.clipboard.writeText(text);
setCopiedToken(token);
setTimeout(() => setCopiedToken(null), 1500);
};
const copyAllBrandTokens = () => {
const css = BRAND_TOKEN_MAPPING.map(
({step, cssVar}) => ` ${cssVar}: ${palette.getHex(step)};`
).join('\n');
const fullCss = `:root {\n${css}\n}`;
navigator.clipboard.writeText(fullCss);
const copyTextToClipboard = async (text: string): Promise<boolean> => {
if (!navigator.clipboard || !navigator.clipboard.writeText) {
console.error('Clipboard API is not available in this browser.');
return false;
}
try {
await navigator.clipboard.writeText(text);
return true;
} catch (error) {
console.error('Failed to write to clipboard:', error);
return false;
}
};
const copyToClipboard = async (step: PaletteStep) => {
const success = await copyTextToClipboard(step.hex);
if (!success) {
return;
}
setCopiedStep(step.step);
setTimeout(() => setCopiedStep(null), 1500);
};
const copyAllColors = async () => {
const colorMap = palette.steps.reduce((acc, step) => {
acc[step.step] = step.hex;
return acc;
}, {} as Record<number, string>);
const success = await copyTextToClipboard(JSON.stringify(colorMap, null, 2));
if (!success) {
return;
}
setCopiedStep(-1);
setTimeout(() => setCopiedStep(null), 1500);
};
const copyTokenValue = async (text: string, token: string) => {
const success = await copyTextToClipboard(text);
if (!success) {
return;
}
setCopiedToken(token);
setTimeout(() => setCopiedToken(null), 1500);
};
const copyAllBrandTokens = async () => {
const css = BRAND_TOKEN_MAPPING.map(
({step, cssVar}) => ` ${cssVar}: ${palette.getHex(step)};`
).join('\n');
const fullCss = `:root {\n${css}\n}`;
const success = await copyTextToClipboard(fullCss);
if (!success) {
return;
}

Copilot uses AI. Check for mistakes.
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.

1 participant