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.

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

feat(ui): component theming #5170

Open
wants to merge 37 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
5fa49fb
feat(ui): component theming (#4883)
dbanksdesign Apr 18, 2024
9084dc2
Merge branch 'main' into component-theming/main
dbanksdesign Apr 18, 2024
4652209
fixing test
dbanksdesign Apr 18, 2024
d675c4e
fixes
dbanksdesign Apr 19, 2024
89c5cfe
LFG
dbanksdesign Apr 23, 2024
bff60ab
updates
dbanksdesign Jun 19, 2024
014ab1b
Merge branch 'main' into component-theming/main
dbanksdesign Jun 19, 2024
3b7594b
updates
dbanksdesign Jun 20, 2024
861d1c6
adding more components and fixing things
dbanksdesign Jun 20, 2024
74cc803
fixes
dbanksdesign Jun 20, 2024
d07f5ae
starting to migrate React primitive classnames
dbanksdesign Jun 20, 2024
f38ebe1
more work
dbanksdesign Jun 21, 2024
3929d5b
working
dbanksdesign Jun 21, 2024
1027758
migrating more primitives
dbanksdesign Jun 21, 2024
10d4556
finishing removing ComponentClassNames
dbanksdesign Jun 24, 2024
dc83aa2
update accordion
dbanksdesign Jun 24, 2024
8569e78
undo primitives changes
dbanksdesign Jun 24, 2024
7031852
fixes
dbanksdesign Jun 24, 2024
c8ca4ba
fixing size increase
dbanksdesign Jun 25, 2024
1790554
fixing size increase
dbanksdesign Jun 25, 2024
098b9ce
fixing size increase
dbanksdesign Jun 25, 2024
57198dd
fixing docs page
dbanksdesign Jun 25, 2024
26e8fbd
updating internal names, removing unnecessary exports
dbanksdesign Jun 26, 2024
9b73ca0
final cleanup
dbanksdesign Jun 26, 2024
112be0f
add changeset
dbanksdesign Jun 26, 2024
2a490a8
removing one more type export
dbanksdesign Jun 26, 2024
fa620ea
changing to defineComponentTheme and adding containerProps function
dbanksdesign Jun 26, 2024
1b92c66
adding type benchmarks for themeing functions
dbanksdesign Jun 26, 2024
98b1142
cleanup
dbanksdesign Jun 26, 2024
745151e
Merge branch 'main' into component-theming/main
dbanksdesign Jun 26, 2024
6dfa6a5
fiddling with the yarn.lock
dbanksdesign Jun 26, 2024
86c1118
more fiddling
dbanksdesign Jun 26, 2024
c1bbcdb
please work
dbanksdesign Jun 26, 2024
badf3a7
fixing tests
dbanksdesign Jun 26, 2024
c3e0560
Merge branch 'main' into component-theming/main
dbanksdesign Jun 26, 2024
b68c1be
Merge branch 'main' into component-theming/main
dbanksdesign Jun 27, 2024
f937f27
adding animation support
dbanksdesign Jul 9, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions packages/react/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@
"import": "./dist/esm/internal.mjs",
"require": "./dist/internal.js"
},
"./theme": {
"types": "./dist/types/theme.d.ts",
"import": "./dist/esm/theme.mjs",
"require": "./dist/theme.js"
},
"./styles.css": "./dist/styles.css",
"./styles.layer.css": "./dist/styles.layer.css",
"./styles/*": "./dist/styles/*",
Expand Down
2 changes: 1 addition & 1 deletion packages/react/rollup.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import typescript from '@rollup/plugin-typescript';
import externals from 'rollup-plugin-node-externals';

// common config settings
const input = ['src/index.ts', 'src/internal.ts'];
const input = ['src/index.ts', 'src/internal.ts', 'src/server.ts'];
const sourceMap = false;
const tsconfig = 'tsconfig.dist.json';

Expand Down
7 changes: 7 additions & 0 deletions packages/react/server/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"private": true,
"name": "@aws-amplify/ui-react-server",
"main": "../dist/server.js",
"module": "../dist/esm/server.mjs",
"types": "../dist/types/server.d.ts"
}
62 changes: 62 additions & 0 deletions packages/react/src/primitives/Theme/Theme.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
import * as React from 'react';
import { WebTheme } from '@aws-amplify/ui';
import {
AriaProps,
BaseComponentProps,
ElementType,
ForwardRefPrimitive,
Primitive,
PrimitiveProps,
} from '../types';
import { ThemeContainer } from './ThemeContainer';
import { ThemeStyle } from './ThemeStyle';
import { primitiveWithForwardRef } from '../utils/primitiveWithForwardRef';

type ColorMode = 'system' | 'light' | 'dark';

interface BaseThemeProps extends BaseComponentProps, AriaProps {
colorMode?: ColorMode;
/**
* Provide a server generated nonce which matches your CSP `style-src` rule.
* This will be attached to the generated <style> tag.
* @see: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/style-src
*/
nonce?: string;
theme: WebTheme;
}

export type ThemeProps<Element extends ElementType = 'div'> = PrimitiveProps<
BaseThemeProps,
Element
>;

const ThemePrimitive: Primitive<ThemeProps, 'div'> = (
dbanksdesign marked this conversation as resolved.
Show resolved Hide resolved
{ children, theme, nonce, colorMode = 'system', ...rest },
ref
) => {
return (
<ThemeContainer {...rest} ref={ref} theme={theme} colorMode={colorMode}>
{children}
<ThemeStyle theme={theme} nonce={nonce} />
</ThemeContainer>
);
};

type ThemeType = ForwardRefPrimitive<BaseThemeProps, 'div'> & {
Container: typeof ThemeContainer;
Style: typeof ThemeStyle;
};

/**
* @experimental
dbanksdesign marked this conversation as resolved.
Show resolved Hide resolved
* [📖 Docs](https://ui.docs.amplify.aws/react/components/theme)
*/
export const Theme: ThemeType = Object.assign(
{},
primitiveWithForwardRef(ThemePrimitive),
{
Container: ThemeContainer,
Style: ThemeStyle,
displayName: 'Theme',
}
);
55 changes: 55 additions & 0 deletions packages/react/src/primitives/Theme/ThemeContainer.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
import * as React from 'react';
import { WebTheme } from '@aws-amplify/ui';
import {
AriaProps,
BaseComponentProps,
ElementType,
ForwardRefPrimitive,
Primitive,
PrimitiveProps,
} from '../types';
import { primitiveWithForwardRef } from '../utils/primitiveWithForwardRef';

type ColorMode = 'system' | 'light' | 'dark';

interface BaseThemeContainerProps extends BaseComponentProps, AriaProps {
colorMode?: ColorMode;
/**
* Provide a server generated nonce which matches your CSP `style-src` rule.
* This will be attached to the generated <style> tag.
* @see: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/style-src
*/
nonce?: string;
theme: WebTheme;
}

export type ThemeContainerProps<Element extends ElementType = 'div'> =
PrimitiveProps<BaseThemeContainerProps, Element>;

const ThemeContainerPrimitive: Primitive<ThemeContainerProps, 'div'> = (
{ children, theme, testId, colorMode = 'system', ...rest },
ref
) => {
const { name } = theme;
return (
<div
{...rest}
{...(testId ? { 'data-testid': testId } : {})}
ref={ref}
data-amplify-theme={name}
data-amplify-color-mode={colorMode}
>
{children}
</div>
);
};

/**
* [📖 Docs](https://ui.docs.amplify.aws/react/components/theme)
*/
export const ThemeContainer: ForwardRefPrimitive<
BaseThemeContainerProps,
'div'
> = primitiveWithForwardRef(ThemeContainerPrimitive);

ThemeContainer.displayName = 'Theme.Container';
97 changes: 97 additions & 0 deletions packages/react/src/primitives/Theme/ThemeStyle.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
import * as React from 'react';
import { WebTheme } from '@aws-amplify/ui';
import {
BaseComponentProps,
ElementType,
ForwardRefPrimitive,
Primitive,
PrimitiveProps,
} from '../types';
import { primitiveWithForwardRef } from '../utils/primitiveWithForwardRef';

interface BaseStyleThemeProps extends BaseComponentProps {
/**
* Provide a server generated nonce which matches your CSP `style-src` rule.
* This will be attached to the generated <style> tag.
* @see: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/style-src
*/
nonce?: string;
theme: WebTheme;
}

export type ThemeStyleProps<Element extends ElementType = 'style'> =
PrimitiveProps<BaseStyleThemeProps, Element>;

const ThemeStylePrimitive: Primitive<ThemeStyleProps, 'style'> = (
{ theme, nonce, ...rest },
ref
) => {
const { name, cssText } = theme;
/*
dbanksdesign marked this conversation as resolved.
Show resolved Hide resolved
Only inject theme CSS variables if given a theme.
The CSS file users import already has the default theme variables in it.
This will allow users to use the provider and theme with CSS variables
without having to worry about specificity issues because this stylesheet
will likely come after a user's defined CSS.

Q: Why are we using dangerouslySetInnerHTML?
A: We need to directly inject the theme's CSS string into the <style> tag without typical HTML escaping.
For example, JSX would escape characters meaningful in CSS such as ', ", < and >, thus breaking the CSS.
Q: Why not use a sanitization library such as DOMPurify?
A: For our use case, we specifically want to purify CSS text, *not* HTML.
DOMPurify, as well as any other HTML sanitization library, would escape/encode meaningful CSS characters
and break our CSS in the same way that JSX would.

Q: Are there any security risks in this particular use case?
A: Anything set inside of a <style> tag is always interpreted as CSS text, *not* HTML.
Reference: “Restrictions on the content of raw text elements” https://html.spec.whatwg.org/dev/syntax.html#cdata-rcdata-restrictions
And in our case, we are using dangerouslySetInnerHTML to set CSS text inside of a <style> tag.

Thus, it really comes down to the question: Could a malicious user escape from the context of the <style> tag?
For example, when inserting HTML into the DOM, could someone prematurely close the </style> tag and add a <script> tag?
e.g., </style><script>alert('hello')</script>
The answer depends on whether the code is rendered on the client or server side.

Client side
- To prevent XSS inside of the <style> tag, we need to make sure it's not closed prematurely.
- This is prevented by React because React creates a style DOM node (e.g., React.createElement(‘style’, ...)), and directly sets innerHTML as a string.
- Even if the string contains a closing </style> tag, it will still be interpreted as CSS text by the browser.
- Therefore, there is not an XSS vulnerability on the client side.

Server side
- When React code is rendered on the server side (e.g., NextJS), the code is sent to the browser as HTML text.
- Therefore, it *IS* possible to insert a closing </style> tag and escape the CSS context, which opens an XSS vulnerability.

Q: How are we mitigating the potential attack vector?
A: To fix this potential attack vector on the server side, we need to filter out any closing </style> tags,
as this the only way to escape from the context of the browser interpreting the text as CSS.
We also need to catch cases where there is any kind of whitespace character </style[HERE]>, such as tabs, carriage returns, etc:
</style

>
Therefore, by only rendering CSS text which does not include a closing '</style>' tag,
we ensure that the browser will correctly interpret all the text as CSS.
*/
if (typeof theme === 'undefined' || /<\/style/i.test(cssText)) {
return null;
} else {
return (
<style
{...rest}
ref={ref}
id={`amplify-theme-${name}`}
// eslint-disable-next-line react/no-danger
dangerouslySetInnerHTML={{ __html: cssText }}
nonce={nonce}
/>
);
}
};

/**
* [📖 Docs](https://ui.docs.amplify.aws/react/components/theme)
*/
export const ThemeStyle: ForwardRefPrimitive<BaseStyleThemeProps, 'style'> =
primitiveWithForwardRef(ThemeStylePrimitive);

ThemeStyle.displayName = 'Theme.Style';
68 changes: 68 additions & 0 deletions packages/react/src/primitives/Theme/__tests__/Theme.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
import * as React from 'react';

import { render, screen } from '@testing-library/react';

import { Theme } from '../Theme';
import { createTheme } from '@aws-amplify/ui';

describe('Theme:', () => {
const themeText = 'Hello from inside a theme';
const theme = createTheme();

it('renders correct defaults', async () => {
const themeId = 'themeId';
const themeTestId = 'themeTestId';
render(
<Theme theme={theme} id={themeId} testId={themeTestId}>
{themeText}
</Theme>
);

const view = await screen.findByTestId(themeTestId);
expect(view.id).toBe(themeId);
expect(view.nodeName).toBe('DIV');
});

it('should render class name', async () => {
const className = 'class-test';
const testId = 'theme-test';
render(<Theme theme={theme} className={className} testId={testId} />);

const view = await screen.findByTestId(testId);
expect(view).toHaveClass(className);
});

it('should forward ref to DOM element', async () => {
const ref = React.createRef<HTMLDivElement>();
const testId = 'theme-test';

render(<Theme theme={theme} ref={ref} testId={testId} />);

await screen.findByTestId(testId);
expect(ref.current?.nodeName).toBe('DIV');
});

it('can render any arbitrary data-* attribute', async () => {
render(
<Theme theme={theme} data-demo="true" testId="dataTest">
{themeText}
</Theme>
);
const view = await screen.findByTestId('dataTest');
expect(view.dataset['demo']).toBe('true');
});

it('should have default colorMode of "system"', async () => {
const testId = 'test-id';
render(<Theme theme={theme} testId={testId} />);
const view = await screen.findByTestId(testId);
expect(view.getAttribute('data-amplify-color-mode')).toBe('system');
});

it('should add the theme name to the data-amplify-theme attribute', async () => {
const testId = 'test-id';
render(<Theme theme={theme} testId={testId} />);
const view = await screen.findByTestId(testId);
expect(view.getAttribute('data-amplify-theme')).toBe(theme.name);
});
});
1 change: 1 addition & 0 deletions packages/react/src/primitives/Theme/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { Theme, ThemeProps } from './Theme';
22 changes: 22 additions & 0 deletions packages/react/src/primitives/shared/styleUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,28 @@ const isComponentStyleProp = (key: string): key is AllStylePropKey => {
return key in ComponentPropsToStylePropsMap;
};

export const themeKeyToVar = (key: string): string => {
return `var(--amplify-${key.replace('.', '-')})`;
};

type PseudoStates =
| 'hover'
| 'active'
| 'focus'
| 'focus-within'
| 'focus-visible';

export const styleState = (
state: PseudoStates,
value: string,
prevValue?: string
): string => {
if (prevValue) {
return `var(--${state}-off, ${prevValue}) var(--${state}-on, ${value})`;
}
return `var(--${state}-on, ${value})`;
};

/**
* Convert style props to CSS variables for React style prop
* Note: Will filter out undefined, null, and empty string prop values
Expand Down
3 changes: 3 additions & 0 deletions packages/react/src/primitives/types/view.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,9 @@ export interface BaseViewProps
*/
isDisabled?: boolean;

_hover?: BaseStyleProps;
_active?: BaseStyleProps;

/**
* @description
* Accepts a JavaScript object with camelCased properties rather than a CSS string.
Expand Down
7 changes: 7 additions & 0 deletions packages/react/src/server.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export { Theme, ThemeProps } from './primitives/Theme';
export {
createTheme,
createComponentTheme,
createComponentClasses,
ComponentTheme,
} from '@aws-amplify/ui';
19 changes: 0 additions & 19 deletions packages/ui/src/theme/__tests__/cssNameTransform.test.ts

This file was deleted.

15 changes: 0 additions & 15 deletions packages/ui/src/theme/__tests__/cssValue.test.ts

This file was deleted.

Loading
Loading