Skip to content

Conversation

@jzempel
Copy link
Member

@jzempel jzempel commented Mar 28, 2024

Description

Sets a new PALETTE baseline for Garden component redesign. A deprecated PALETTE_V8 export was added to temporarily solve for color-related unit tests. Throughout v9 component color refactoring, these tests will be updated to use PALETTE (which should be the goto for color testing rather than getColor or DEFAULT_THEME.palette, etc). PALETTE_V8 will be removed prior to the v9 release.

Detail

Redux of #1749

Checklist

  • 👌 design updates will be Garden Designer approved (add the designer as a reviewer)
  • 🌐 demo is up-to-date (npm start)
  • ⬅️ renders as expected with reversed (RTL) direction
  • 🤘 renders as expected with Bedrock CSS (?bedrock)
  • 💂‍♂️ includes new unit tests. Maintain existing coverage (always >= 96%)
  • tested for WCAG 2.1 AA accessibility compliance
  • 📝 tested in Chrome, Firefox, Safari, and Edge

@jzempel jzempel requested a review from a team as a code owner March 28, 2024 13:41
@coveralls
Copy link

coveralls commented Mar 28, 2024

Coverage Status

coverage: 96.037%. remained the same
when pulling 30597cd on jzempel/v9-palette
into 78f531b on next.

Copy link
Contributor

@geotrev geotrev left a comment

Choose a reason for hiding this comment

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

Just one comment here, otherwise :shipit:

- The default theming `palette` has been redesigned and expanded to support
light/dark modes. A temporary, deprecated `getColorV8` utility has been added.
Use the temporary utility to apply legacy version 8 colors to custom
(non-Garden) components they can be upgraded to use the enhanced version 9
Copy link
Contributor

Choose a reason for hiding this comment

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

This wording/flow feels off.

@jzempel jzempel requested a review from geotrev March 28, 2024 20:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

5 participants