Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Updated Palette Generation #3481

Merged
merged 11 commits into from
Dec 12, 2023
Merged

Updated Palette Generation #3481

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
f03e6e3
Initial palette generation
trevoring-okta Dec 2, 2023
849603a
Use designTokensOverride for palette
trevoring-okta Dec 4, 2023
8421bf8
Refine design token overrides with palette gen and update tests
trevoring-okta Dec 5, 2023
a909f04
Update e2e test
trevoring-okta Dec 5, 2023
9a829f2
Empty commit for Bacon
trevoring-okta Dec 5, 2023
6c9b511
Update base VRT screenshots
trevoring-okta Dec 6, 2023
6251abf
Removing token-based palette generation
trevoring-okta Dec 6, 2023
2068be9
Bump to Odyssey 1.9.1
trevoring-okta Dec 6, 2023
b13905b
Create mergedTokens for SIW specific overrides
trevoring-okta Dec 6, 2023
733438b
Revert responseConfig change
trevoring-okta Dec 6, 2023
ad8c34a
Update Tokens -> tokens naming
trevoring-okta Dec 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/v3/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,16 @@
"ie 11"
],
"dependencies": {
"@adobe/leonardo-contrast-colors": "1.0.0-alpha.13",
"@babel/runtime": "^7.18.6",
"@emotion/cache": "^11.9.3",
"@emotion/react": "^11.9.3",
"@emotion/styled": "^11.9.3",
"@hcaptcha/react-hcaptcha": "^1.8.1",
"@mui/icons-material": "^5.8.4",
"@mui/material": "^5.8.5",
"@okta/odyssey-design-tokens": "^1.2.0",
"@okta/odyssey-react-mui": "^1.2.0",
"@okta/odyssey-design-tokens": "1.9.1",
"@okta/odyssey-react-mui": "1.9.1",
Comment on lines +50 to +51
Copy link
Contributor

Choose a reason for hiding this comment

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

You're intentionally pinning to the exact semver?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

"@okta/odyssey-react-mui-legacy": "npm:@okta/odyssey-react-mui@^0.17.0",
"@okta/okta-auth-js": "^7.4.2",
"chroma-js": "^2.4.2",
Expand Down
Binary file modified src/v3/screenshots/base/UI_demo/UI_demo_RTL_VRT.png
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified src/v3/screenshots/base/UI_demo/UI_demo_VRT.png
View file
Open in desktop
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
28 changes: 16 additions & 12 deletions src/v3/src/components/Images/AppIcon.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,20 +10,24 @@
* See the License for the specific language governing permissions and limitations under the License.
*/

import { Theme } from '@mui/material';
import { useOdysseyDesignTokens } from '@okta/odyssey-react-mui';
import { AppsIcon } from '@okta/odyssey-react-mui/icons';
import { FunctionComponent, h } from 'preact';

import { loc } from '../../util';

export const AppIcon: FunctionComponent = () => (
<AppsIcon
sx={(theme: Theme) => ({
color: theme.palette.primary.main,
width: '16px',
height: '16px',
})}
titleAccess={loc('icon.title.application', 'login')}
aria-hidden
/>
);
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
/>
);
};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

28 changes: 16 additions & 12 deletions src/v3/src/components/Images/DeviceIcon.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,20 +10,24 @@
* See the License for the specific language governing permissions and limitations under the License.
*/

import { Theme } from '@mui/material';
import { useOdysseyDesignTokens } from '@okta/odyssey-react-mui';
import { DevicesIcon } from '@okta/odyssey-react-mui/icons';
import { FunctionComponent, h } from 'preact';

import { loc } from '../../util';

export const DeviceIcon: FunctionComponent = () => (
<DevicesIcon
sx={(theme: Theme) => ({
color: theme.palette.primary.main,
width: '16px',
height: '16px',
})}
titleAccess={loc('icon.title.browser', 'login')}
aria-hidden
/>
);
export const DeviceIcon: FunctionComponent = () => {
const Tokens = useOdysseyDesignTokens();

return (
<DevicesIcon
sx={{
color: Tokens.PalettePrimaryMain,
width: '16px',
height: '16px',
}}
titleAccess={loc('icon.title.browser', 'login')}
aria-hidden
/>
);
};
28 changes: 16 additions & 12 deletions src/v3/src/components/Images/LocationIcon.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,20 +10,24 @@
* See the License for the specific language governing permissions and limitations under the License.
*/

import { Theme } from '@mui/material';
import { useOdysseyDesignTokens } from '@okta/odyssey-react-mui';
import { GlobeIcon } from '@okta/odyssey-react-mui/icons';
import { FunctionComponent, h } from 'preact';

import { loc } from '../../util';

export const LocationIcon: FunctionComponent = () => (
<GlobeIcon
sx={(theme: Theme) => ({
color: theme.palette.primary.main,
width: '16px',
height: '16px',
})}
titleAccess={loc('icon.title.location', 'login')}
aria-hidden
/>
);
export const LocationIcon: FunctionComponent = () => {
const Tokens = useOdysseyDesignTokens();

return (
<GlobeIcon
sx={{
color: Tokens.PalettePrimaryMain,
width: '16px',
height: '16px',
}}
titleAccess={loc('icon.title.location', 'login')}
aria-hidden
/>
);
};
30 changes: 17 additions & 13 deletions src/v3/src/components/Widget/GlobalStyles.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,36 +13,40 @@
import {
GlobalStyles as MuiGlobalStyles,
GlobalStylesProps,
Theme,
} from '@mui/material';
import { DesignTokens, useOdysseyDesignTokens } from '@okta/odyssey-react-mui';
import { FunctionComponent, h } from 'preact';

// TODO we could scope these to just widget container
const svgStyles = (theme: Theme): GlobalStylesProps['styles'] => ({
const svgStyles = (Tokens: DesignTokens): GlobalStylesProps['styles'] => ({
'.siwFillPrimary': {
fill: theme.palette.primary.main,
fill: Tokens.PalettePrimaryMain,
},
'.siwFillPrimaryDark': {
fill: theme.palette.primary.dark,
fill: Tokens.PalettePrimaryDark,
},
'.siwFillSecondary': {
fill: theme.palette.primary.light,
fill: Tokens.PalettePrimaryLight,
},
'.siwFillBg': {
fill: theme.palette.grey[50],
fill: Tokens.HueNeutral50,
},
'.siwIconFillPrimaryDark': {
fill: theme.palette.primary.dark,
fill: Tokens.PalettePrimaryDark,
},
'.siwIconFillSecondary': {
fill: theme.palette.primary.light,
fill: Tokens.PalettePrimaryLight,
},
});

const GlobalStyles: FunctionComponent = () => (
<MuiGlobalStyles
styles={(theme) => svgStyles(theme)}
/>
);
const GlobalStyles: FunctionComponent = () => {
const Tokens = useOdysseyDesignTokens();
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: capitalization?

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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.


return (
<MuiGlobalStyles
styles={svgStyles(Tokens)}
/>
);
};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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


export default GlobalStyles;
21 changes: 15 additions & 6 deletions src/v3/src/components/Widget/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ import {
triggerEmailVerifyCallback,
} from '../../util';
import { getEventContext } from '../../util/getEventContext';
import { createTheme } from '../../util/theme';
import { createThemeAndTokens } from '../../util/theme';
import AuthContainer from '../AuthContainer/AuthContainer';
import AuthContent from '../AuthContent/AuthContent';
import AuthHeader from '../AuthHeader/AuthHeader';
Expand Down Expand Up @@ -136,10 +136,16 @@ export const Widget: FunctionComponent<WidgetProps> = (widgetProps) => {
const { stateHandle, unsetStateHandle } = useStateHandle(widgetProps);

// merge themes
const theme = useMemo(() => mergeThemes(
createTheme(brandColors, customTheme?.tokens ?? {}),
{ direction: languageDirection },
), [brandColors, customTheme, languageDirection]);
const { theme, tokens } = useMemo(() => {
const { themeOverride, tokensOverride } = createThemeAndTokens(
brandColors,
customTheme?.tokens ?? {},
);
return {
theme: mergeThemes(themeOverride, { direction: languageDirection }),
tokens: tokensOverride,
};
}, [brandColors, customTheme, languageDirection]);

// on unmount, remove the language
useEffect(() => () => {
Expand Down Expand Up @@ -498,7 +504,10 @@ export const Widget: FunctionComponent<WidgetProps> = (widgetProps) => {
<CustomPluginsOdysseyCacheProvider nonce={cspNonce}>
{/* remove this provider when all Odyssey legacy imports are removed */}
<MuiThemeProvider theme={theme}>
<OdysseyProvider themeOverride={theme}>
<OdysseyProvider
themeOverride={theme}
designTokensOverride={tokens}
>
<GlobalStyles />
{/* the style is to allow the widget to inherit the parent's bg color */}
<ScopedCssBaseline sx={{ backgroundColor: 'inherit' }}>
Expand Down