Updated Palette Generation #3481

trevoring-okta · 2023-12-06T16:58:21Z

Description:

Odyssey 1.x expanded their palette to include 10 different hues rather than the original 5(?)
Uses Adobe's leonardo-contrast-colors package (same tool Odyssey used to generate their palettes for 1.x) to generate a WCAG-friendly palette that matches Odyssey's required contrast ratios based on the brandColors.primaryColorconfig option
- Pinned on Leonardo 1.0.0-alpha.13 because that is the last stable version. Also, Leonardo does not have proper TS support for their main API, so we had to define types via leonardo.d.ts. These types were taken from a PR that has been merged into Leonardo but for some reason not been published to NPM. Ideally this will get published soon and we can bump the package and remove the type definitions
Also starts passing customer-defined tokens that can be passed in custom code editor via theme.tokens into OdysseyProvider's designTokensOverride, otherwise token overrides are not accessible via the useOdysseyDesignTokens hook

PR Checklist

Have you verified the basic functionality for this change?
Did you add tests, as appropriate, following our Automated Test guidelines?
Did you follow our Security Best Practices?
Did you verify the change by running downstream monolith artifact? (Provide link to build below)
Does this PR include noticeable changes to the UI? (If yes, attach screenshots/video below)

Issue:

OKTA-667943

Reviewers:

Screenshot/Video:

Switching from Odyssey 1.x default palette to custom green palette

Screen.Recording.2023-12-06.at.11.54.30.AM.mov

Downstream Monolith Build:

trevoring-okta · 2023-12-06T16:59:46Z

src/v3/src/components/Images/AppIcon.tsx

+export const AppIcon: FunctionComponent = () => {
+  const Tokens = useOdysseyDesignTokens();
+
+  return (
+    <AppsIcon
+      sx={{
+        color: Tokens.PalettePrimaryMain,
+        width: '16px',
+        height: '16px',
+      }}
+      titleAccess={loc('icon.title.application', 'login')}
+      aria-hidden
+    />
+  );
+};


Updated custom icons that Denys added to use token overrides instead of theme.palette. We're trying to remove all uses of theme in favor of tokens

trevoring-okta · 2023-12-06T17:00:13Z

src/v3/src/components/Widget/GlobalStyles.tsx

+const GlobalStyles: FunctionComponent = () => {
+  const Tokens = useOdysseyDesignTokens();
+
+  return (
+    <MuiGlobalStyles
+      styles={svgStyles(Tokens)}
+    />
+  );
+};


See other comment about moving all uses of theme to use tokens when possible

trevoring-okta · 2023-12-06T17:02:34Z

src/v3/src/util/theme.ts

-  if (customTokens) {
-    if (customTokens.PalettePrimaryMain) {
-      const p = generatePalette(customTokens.PalettePrimaryMain);
-      set(theme, 'palette.primary.lighter', customTokens.PalettePrimaryLighter ?? p.lighter);
-      set(theme, 'palette.primary.light', customTokens.PalettePrimaryLight ?? p.light);
-      set(theme, 'palette.primary.main', customTokens.PalettePrimaryMain ?? p.main);
-      set(theme, 'palette.primary.dark', customTokens.PalettePrimaryDark ?? p.dark);
-      set(theme, 'palette.primary.contrastText', p.contrastText);
-    }
-    if (customTokens.PaletteDangerMain) {


There was some miscommunication with the pre-1.x implementation of palette generation. We thought we should be generating palettes whenever the customer passed in token overrides for any of the Main hues but @alexdahl-okta clarified that palette generation should only be via brandColors.primaryColor

trevoring-okta · 2023-12-06T17:08:19Z

src/v3/src/util/theme.ts

-      // ruleset with :focus-visible pseudo-selector break entire ruleset in
-      // ie11 because its not supported. re-define the :hover rule separately
-      // again so the ruleset is applied in ie11
-      MuiButton: {
-        styleOverrides: {
-          root: ({ ownerState, theme: t }) => ({
-            ...(ownerState.variant === 'primary' && {
-              '&:hover': {
-                backgroundColor: t.palette.primary.dark,
-              },
-            }),
-            ...(ownerState.variant === 'secondary' && {
-              '&:hover': {
-                backgroundColor: t.palette.primary.lighter,
-                borderColor: t.palette.primary.light,
-                color: t.palette.primary.main,
-              },
-            }),
-            ...(ownerState.variant === 'floating' && {
-              '&:hover': {
-                backgroundColor: 'rgba(29, 29, 33, 0.1)',
-                borderColor: 'transparent',
-              },
-            }),
-            // OKTA-657762 - remove this when odyssey fix is done
-            textTransform: 'none',
-          }),
-        },
-      },
-      // ruleset with :focus-visible pseudo-selector break entire ruleset in
-      // ie11 because its not supported. re-define the :hover rule separately
-      // again so the ruleset is applied in ie11
-      MuiIconButton: {
-        styleOverrides: {
-          root: {
-            '&:hover': {
-              backgroundColor: 'rgba(29, 29, 33, 0.1)',
-              borderColor: 'transparent',
-            },
-          },
-        },
-      },
-


These overrides were for IE11 breaking changes that existed in the 0.x Odyssey version that we used to be pinned on. Since we have bumped to 1.x, these DNE anymore

trevoring-okta · 2023-12-06T17:17:02Z

src/v3/src/util/theme.ts

+  const mergedTokens = { ...Tokens, ...tokensOverride };
+
+  // Odyssey 1.x does not export their theme, but we can recreate it
+  const baseOdysseyTheme = createOdysseyMuiTheme({
+    odysseyTokens: mergedTokens,
+  });
+


One niche thing to note: palette generation is now limited to the design tokens layer and will not apply at the base MUI theming layer (the base MUI theme appears in code via theme.palette.*). This is fine because even though Odyssey does create a MUI theming layer via this createOdysseyMuiTheme call, they use their design tokens to achieve essentially all of their styling.

TLDR: If the customer begins seeing any styling that can be traced to the base MUI theme layer, there's probably something wrong and the styling is just a fallback

Would a bug like that be fixed in SIW or ODS?

That scenario would probably be fixed in SIW and would be a case where the SIW dev decided to use the base MUI theme API rather than the Odyssey useOdysseyDesignTokens hook

denysoblohin-okta · 2023-12-07T16:36:20Z

src/v3/src/util/theme.ts

-  chroma.contrast(color, WHITE_HEX) >= 4.5 ? WHITE_HEX : BLACK_HEX
-);
+// Odyssey-defined contrast ratios for their WCAG-friendly palettes
+const ODYSSEY_RATIOS = [1.1, 1.31, 1.61, 2.22, 3.32, 4.5, 4.95, 8.72, 11.73, 14.94];


Where are these numbers coming from? Odyssey source code/document?

Yeah these numbers have been specifically defined by Odyssey and design

lesterchoi-okta · 2023-12-07T16:35:08Z

src/v3/package.json

+    "@okta/odyssey-design-tokens": "1.9.1",
+    "@okta/odyssey-react-mui": "1.9.1",


You're intentionally pinning to the exact semver?

Yeah I decided to pin exactly because even though Odyssey promised no breaking changes they have still been having some breaking changes. For example, the OdysseyProvider API was broken for a few versions and only just got fixed at this 1.9.1 update

lesterchoi-okta · 2023-12-07T16:36:41Z

src/v3/screenshots/base/UI_demo/UI_demo_RTL_VRT.png

These VRT screenshots are really useful. Is the background color of secondary button changing due to palette generation or a separate change in Odyssey?

Looks like it's a change in Odyssey 1.5.0 https://github.com/okta/odyssey/blob/main/CHANGELOG.md#150-2023-10-27

lesterchoi-okta · 2023-12-07T16:38:22Z

src/v3/src/components/Widget/GlobalStyles.tsx

-  />
-);
+const GlobalStyles: FunctionComponent = () => {
+  const Tokens = useOdysseyDesignTokens();


nit: capitalization?

Suggested change

const Tokens = useOdysseyDesignTokens();

const tokens = useOdysseyDesignTokens();

I've been using capital T for all uses of useOdyssyeDesignTokens() in the codebase to mimic how Odyssey imports their tokens: https://github.com/okta/odyssey/blob/782475cc222069f0417b08cb40ed57d298903b1d/packages/odyssey-react-mui/src/theme/createOdysseyMuiTheme.ts#L14C1-L14C55

The capital T feels like it signals to the dev that these tokens are from ultimately from an import rather than something like a prop, but if this violates some syntax I don't mind switching all of the naming to use lowercase T

Nope not a blocker. Just pointing it out because TitleCase is usually reserved for classes, types, interface, and enums(?), i.e., wanted to prevent ambiguity or confusion.

lesterchoi-okta · 2023-12-07T16:40:07Z

src/v3/src/util/theme.ts

+  const mergedTokens = { ...Tokens, ...tokensOverride };
+
+  // Odyssey 1.x does not export their theme, but we can recreate it
+  const baseOdysseyTheme = createOdysseyMuiTheme({
+    odysseyTokens: mergedTokens,
+  });
+


Would a bug like that be fixed in SIW or ODS?

* Odyssey 1.x Palette Generation using Brand Colors

trevoring-okta added 8 commits December 4, 2023 15:25

Initial palette generation

f03e6e3

Use designTokensOverride for palette

849603a

Refine design token overrides with palette gen and update tests

8421bf8

Update e2e test

a909f04

Empty commit for Bacon

9a829f2

Update base VRT screenshots

6c9b511

Removing token-based palette generation

6251abf

Bump to Odyssey 1.9.1

2068be9

trevoring-okta commented Dec 6, 2023

View reviewed changes

trevoring-okta added 2 commits December 6, 2023 12:14

Create mergedTokens for SIW specific overrides

b13905b

Revert responseConfig change

733438b

trevoring-okta commented Dec 6, 2023

View reviewed changes

trevoring-okta marked this pull request as ready for review December 6, 2023 18:04

trevoring-okta requested review from denysoblohin-okta, glenfannin-okta, jaredperreault-okta, leemchale-okta, lesterchoi-okta, robertdamphousse-okta, shawyu-okta, shuowu-okta and vijetmahabaleshwar-okta as code owners December 6, 2023 18:04

denysoblohin-okta reviewed Dec 7, 2023

View reviewed changes

lesterchoi-okta approved these changes Dec 7, 2023

View reviewed changes

Update Tokens -> tokens naming

ad8c34a

trevoring-okta merged commit 4fbf37b into odyssey-1.x Dec 12, 2023

trevoring-okta added a commit that referenced this pull request Feb 12, 2024

Updated Palette Generation (#3481)

b1cbd4a

* Odyssey 1.x Palette Generation using Brand Colors

trevoring-okta added a commit that referenced this pull request Feb 13, 2024

Updated Palette Generation (#3481)

6b9b746

* Odyssey 1.x Palette Generation using Brand Colors

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Updated Palette Generation #3481

Updated Palette Generation #3481

trevoring-okta commented Dec 6, 2023 •

edited

trevoring-okta Dec 6, 2023

trevoring-okta Dec 6, 2023

trevoring-okta Dec 6, 2023

trevoring-okta Dec 6, 2023

trevoring-okta Dec 6, 2023

lesterchoi-okta Dec 7, 2023

trevoring-okta Dec 8, 2023

denysoblohin-okta Dec 7, 2023

trevoring-okta Dec 8, 2023

lesterchoi-okta Dec 7, 2023

trevoring-okta Dec 8, 2023

lesterchoi-okta Dec 7, 2023

denysoblohin-okta Dec 7, 2023

lesterchoi-okta Dec 7, 2023

trevoring-okta Dec 8, 2023

lesterchoi-okta Dec 11, 2023

lesterchoi-okta Dec 7, 2023

		"@okta/odyssey-design-tokens": "1.9.1",
		"@okta/odyssey-react-mui": "1.9.1",

	const Tokens = useOdysseyDesignTokens();
	const tokens = useOdysseyDesignTokens();

Updated Palette Generation #3481

Updated Palette Generation #3481

Conversation

trevoring-okta commented Dec 6, 2023 • edited

Description:

PR Checklist

Issue:

Reviewers:

Screenshot/Video:

Switching from Odyssey 1.x default palette to custom green palette

Downstream Monolith Build:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

trevoring-okta commented Dec 6, 2023 •

edited