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

chore(ui-react-native): Migrate Checkbox primitive #2621

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 5 additions & 5 deletions examples/react-native/ios/Podfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -284,11 +284,11 @@ PODS:
- glog
- react-native-netinfo (8.3.1):
- React-Core
- react-native-safe-area-context (4.3.1):
- react-native-safe-area-context (4.3.3):
- RCT-Folly
- RCTRequired
- RCTTypeSafety
- React
- React-Core
- ReactCommon/turbomodule/core
- React-perflogger (0.68.1)
- React-RCTActionSheet (0.68.1):
Expand Down Expand Up @@ -355,7 +355,7 @@ PODS:
- React-jsi (= 0.68.1)
- React-logger (= 0.68.1)
- React-perflogger (= 0.68.1)
- RNCAsyncStorage (1.17.9):
- RNCAsyncStorage (1.17.10):
- React-Core
- SocketRocket (0.6.0)
- Yoga (1.14.0)
Expand Down Expand Up @@ -543,7 +543,7 @@ SPEC CHECKSUMS:
React-jsinspector: 218a2503198ff28a085f8e16622a8d8f507c8019
React-logger: f79dd3cc0f9b44f5611c6c7862badd891a862cf8
react-native-netinfo: 1a6035d3b9780221d407c277ebfb5722ace00658
react-native-safe-area-context: 6c12e3859b6f27b25de4fee8201cfb858432d8de
react-native-safe-area-context: b456e1c40ec86f5593d58b275bd0e9603169daca
React-perflogger: 30ab8d6db10e175626069e742eead3ebe8f24fd5
React-RCTActionSheet: 4b45da334a175b24dabe75f856b98fed3dfd6201
React-RCTAnimation: d6237386cb04500889877845b3e9e9291146bc2e
Expand All @@ -556,7 +556,7 @@ SPEC CHECKSUMS:
React-RCTVibration: 9e344c840176b0af9c84d5019eb4fed8b3c105a1
React-runtimeexecutor: 7285b499d0339104b2813a1f58ad1ada4adbd6c0
ReactCommon: bf2888a826ceedf54b99ad1b6182d1bc4a8a3984
RNCAsyncStorage: b2489b49e38c85e10ed45a888d13a2a4c7b32ea1
RNCAsyncStorage: 0c357f3156fcb16c8589ede67cc036330b6698ca
SocketRocket: fccef3f9c5cedea1353a9ef6ada904fde10d6608
Yoga: 17cd9a50243093b547c1e539c749928dd68152da
YogaKit: f782866e155069a2cca2517aafea43200b01fd5a
Expand Down
24 changes: 23 additions & 1 deletion examples/react-native/metro.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ const WORKSPACE_NODE_MODULES_PATH = path.resolve(
'../../node_modules'
);

const config = { resolver: {}, transformer: {} };
const config = { resolver: {}, transformer: {}, server: {} };

const dependencies = {
...EXAMPLE_APP_PACKAGE_JSON.dependencies,
Expand Down Expand Up @@ -111,4 +111,26 @@ config.transformer.getTransformOptions = async () => ({
},
});

/**
* Below workaround is needed because assets resolutions don't work for Android in monorepo
* Ref:
* https://github.com/react-native-community/cli/issues/1238
* https://github.com/facebook/metro/issues/290#issuecomment-543746458
*/
config.transformer.publicPath = '/assets/dir1/dir2/dir3';
config.server.enhanceMiddleware = (middleware) => {
ioanabrooks marked this conversation as resolved.
Show resolved Hide resolved
return (req, res, next) => {
if (req.url.startsWith('/assets/dir1/dir2/dir3')) {
req.url = req.url.replace('/assets/dir1/dir2/dir3', '/assets');
} else if (req.url.startsWith('/assets/dir1/dir2')) {
req.url = req.url.replace('/assets/dir1/dir2', '/assets/..');
} else if (req.url.startsWith('/assets/dir1')) {
req.url = req.url.replace('/assets/dir1', '/assets/../..');
} else if (req.url.startsWith('/assets')) {
req.url = req.url.replace('/assets', '/assets/../../..');
}
return middleware(req, res, next);
};
};

module.exports = config;
2 changes: 0 additions & 2 deletions examples/react-native/src/assets/icons/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ import { ImageSourcePropType } from 'react-native';
* Example app icon assets.
*/
const icons = {
checkboxFilled: require('./checkboxFilled.png') as ImageSourcePropType,
checkboxOutline: require('./checkboxOutline.png') as ImageSourcePropType,
radioButtonChecked:
require('./radioButtonChecked.png') as ImageSourcePropType,
radioButtonUnchecked:
Expand Down
1 change: 0 additions & 1 deletion examples/react-native/src/ui/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,5 @@ export * from '@aws-amplify/ui-react-native/dist/primitives';
* e.g. have full type documentation, controlled vs uncontrolled handling, stories, touch feedback,
* accessibility props, unit tests, etc
*/
export * from './Checkbox';
export * from './Radio';
export * from './RadioGroup';
17 changes: 16 additions & 1 deletion examples/react-native/storybook/index.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,14 @@
import React from 'react';
import { LogBox, Platform } from 'react-native';
import { getStorybookUI, configure } from '@storybook/react-native';
import {
getStorybookUI,
configure,
addDecorator,
} from '@storybook/react-native';
import { withKnobs } from '@storybook/addon-knobs';
import noop from 'lodash/noop';
import { loadStories } from './storyLoader';
import { Screen } from './ui/Screen';

const STORYBOOK_REQUIRE_CYCLE_PREFIX =
'Require cycle: node_modules/core-js/internals/microtask.js';
Expand All @@ -15,6 +22,14 @@ export function setupStorybook(initStorybook: boolean) {
configure(() => {
loadStories();
}, module);

// add decorators
addDecorator(withKnobs);
addDecorator((Story: any) => (
ioanabrooks marked this conversation as resolved.
Show resolved Hide resolved
<Screen>
<Story />
</Screen>
));
}

// Refer to https://github.com/storybookjs/react-native/tree/master/app/react-native#getstorybookui-options
Expand Down
6 changes: 0 additions & 6 deletions examples/react-native/storybook/stories/Button.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,8 @@ import { action } from '@storybook/addon-actions';
import { text } from '@storybook/addon-knobs';
import { storiesOf } from '@storybook/react-native';
import { Button } from '@aws-amplify/ui-react-native/dist/primitives';
import { Screen } from '../ui';

storiesOf('Button', module)
.addDecorator((Story: any) => (
<Screen>
<Story />
</Screen>
))
.add('with text', () => (
<Button onPress={action('clicked-text')}>
<Text>{text('Button text', 'Hello Button')}</Text>
Expand Down
27 changes: 27 additions & 0 deletions examples/react-native/storybook/stories/Checkbox.stories.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import React, { useState } from 'react';
import { storiesOf } from '@storybook/react-native';
import { select } from '@storybook/addon-knobs';
import { Checkbox } from '@aws-amplify/ui-react-native/dist/primitives';

const StatefulCheckbox = ({ ...props }: any) => {
const [value, setValue] = useState(props.value);
const onChange = (nextValue: string) => {
setValue(nextValue);
};

return <Checkbox {...props} value={value} onChange={onChange} size={20} />;
ioanabrooks marked this conversation as resolved.
Show resolved Hide resolved
};
storiesOf('Checkbox', module)
.add('default', () => <StatefulCheckbox />)
.add('with Label', () => (
<StatefulCheckbox
label="Label"
labelPosition={select(
'LabelPosition',
['start', 'end', 'top', 'bottom'],
'end'
)}
/>
))
.add('selected', () => <StatefulCheckbox selected />)
.add('disabled', () => <StatefulCheckbox label="Disabled" disabled />);
6 changes: 0 additions & 6 deletions examples/react-native/storybook/stories/Heading.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,11 @@ import {
Heading,
HeadingProps,
} from '@aws-amplify/ui-react-native/dist/primitives';
import { Screen } from '../ui';
import { StyleSheet } from 'react-native';

const levels: HeadingProps['level'][] = [1, 2, 3, 4, 5, 6];

storiesOf('Heading', module)
.addDecorator((Story: any) => (
<Screen>
<Story />
</Screen>
))
.add('default', () => <Heading>Default Heading</Heading>)
.add('level', () => (
<>
Expand Down
6 changes: 0 additions & 6 deletions examples/react-native/storybook/stories/Label.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,8 @@ import React from 'react';
import { StyleSheet } from 'react-native';
import { storiesOf } from '@storybook/react-native';
import { Label } from '@aws-amplify/ui-react-native/dist/primitives';
import { Screen } from '../ui';

storiesOf('Label', module)
.addDecorator((Story: any) => (
<Screen>
<Story />
</Screen>
))
.add('default Label', () => <Label>Default Label</Label>)
.add('style', () => <Label style={styles.redText}>This should be red</Label>);

Expand Down
2 changes: 2 additions & 0 deletions examples/react-native/storybook/storyLoader.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
function loadStories() {
require('./stories/Button.stories');
require('./stories/Checkbox.stories');
require('./stories/Heading.stories');
require('./stories/Label.stories');
}

const stories = [
'./stories/Button.stories',
'./stories/Checkbox.stories',
'./stories/Heading.stories',
'./stories/Label.stories',
];
Expand Down
6 changes: 5 additions & 1 deletion examples/react-native/storybook/ui/Screen.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,9 @@ export const Screen = ({ children }: ScreenProps) => {
};

const styles = StyleSheet.create({
container: { flex: 1, alignItems: 'center', justifyContent: 'center' },
container: {
flex: 1,
alignItems: 'center',
justifyContent: 'center',
},
});
1 change: 1 addition & 0 deletions packages/react-native/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
"@babel/core": "^7.17.10",
"@babel/preset-env": "^7.17.10",
"@testing-library/react-hooks": "^8.0.0",
"@testing-library/react-native": "^11.1.0",
"@types/react": "^17.0.2",
"@types/react-native": "^0.67.6",
"@types/react-test-renderer": "^17.0.1",
Expand Down
2 changes: 2 additions & 0 deletions packages/react-native/src/assets/icons/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { ImageSourcePropType } from 'react-native';

const icons = {
checkboxFilled: require('./checkboxFilled.png') as ImageSourcePropType,
checkboxOutline: require('./checkboxOutline.png') as ImageSourcePropType,
close: require('./close.png') as ImageSourcePropType,
};

Expand Down
Original file line number Diff line number Diff line change
@@ -1,27 +1,20 @@
import React, { useCallback, useMemo } from 'react';
import React, { useCallback, useMemo, useState } from 'react';
import {
Pressable,
PressableStateCallbackType,
StyleProp,
View,
ViewStyle,
} from 'react-native';

import { icons } from '../../assets';
import {
IconButton,
Label,
} from '@aws-amplify/ui-react-native/dist/primitives';

import { CheckboxProps, CheckboxStyle } from './types';

const styles: CheckboxStyle = {
container: {
alignItems: 'center',
padding: 4,
},
};
import { styles } from './styles';
import { IconButton } from '../IconButton';
import { Label } from '../Label';
import { CheckboxProps } from './types';
import { getFlexDirectionFromLabelPosition } from '../Label/utils';

export default function Checkbox<T>({
accessibilityRole = 'checkbox',
buttonStyle,
disabled,
label,
Expand All @@ -34,20 +27,17 @@ export default function Checkbox<T>({
value,
...rest
}: CheckboxProps<T>): JSX.Element {
const labelPrecedesIcon =
labelPosition === 'start' || labelPosition === 'top';
const [checked, setChecked] = useState(selected ?? false);

const handleOnChange = useCallback(() => {
onChange?.(value);
}, [onChange, value]);
setChecked(!checked);
}, [onChange, value, checked]);

const containerStyle: ViewStyle = useMemo(
() => ({
...styles.container,
flexDirection:
labelPosition === 'bottom' || labelPosition === 'top'
? 'column'
: 'row',
flexDirection: getFlexDirectionFromLabelPosition(labelPosition),
opacity: disabled ? 0.6 : 1,
}),
[disabled, labelPosition]
Expand All @@ -67,22 +57,23 @@ export default function Checkbox<T>({
[buttonStyle]
);

// TODO: replace IconButton with icon once Icon primitive is added

ioanabrooks marked this conversation as resolved.
Show resolved Hide resolved
return (
<View style={[containerStyle, style]}>
{label && labelPrecedesIcon ? (
<Label style={labelStyle}>{label}</Label>
) : null}
<Pressable
accessibilityRole={accessibilityRole}
disabled={disabled}
onPress={handleOnChange}
style={[containerStyle, style]}
>
<IconButton
{...rest}
disabled={disabled}
onPress={handleOnChange}
source={selected ? icons.checkboxFilled : icons.checkboxOutline}
disabled
source={checked ? icons.checkboxFilled : icons.checkboxOutline}
size={size}
style={iconButtonStyle}
Copy link
Member

Choose a reason for hiding this comment

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

Let's make sure we clean up the buttonStyle prop and iconButtonStyle when we replace the IconButton

/>
{label && !labelPrecedesIcon ? (
<Label style={labelStyle}>{label}</Label>
) : null}
</View>
{label ? <Label style={labelStyle}>{label}</Label> : null}
</Pressable>
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import React from 'react';
import { fireEvent, render } from '@testing-library/react-native';
import Checkbox from '../Checkbox';

const onChange = jest.fn();

describe('Checkbox', () => {
beforeEach(() => {
onChange.mockClear();
});

[true, false].forEach((value) => {
it(`renders as expected when selected is ${value}`, () => {
const { toJSON } = render(
<Checkbox selected={value} value={value} onChange={onChange} />
);
expect(toJSON()).toMatchSnapshot();
});
});

it('renders as expected when disabled', () => {
ioanabrooks marked this conversation as resolved.
Show resolved Hide resolved
const { toJSON } = render(
<Checkbox disabled value="" onChange={onChange} />
);
expect(toJSON()).toMatchSnapshot();
});

it('renders as expected with accessibilityRole', () => {
const { toJSON } = render(
<Checkbox value="" onChange={onChange} accessibilityRole={'none'} />
);
expect(toJSON()).toMatchSnapshot();
});

it('calls the expected handler when selected', () => {
const { getByRole } = render(<Checkbox value="" onChange={onChange} />);
const checkbox = getByRole('checkbox');
fireEvent.press(checkbox);
expect(onChange).toHaveBeenCalledTimes(1);
});

it('does nothing when disabled', () => {
const { getByRole } = render(
<Checkbox value="" onChange={onChange} disabled />
);
const checkbox = getByRole('checkbox');
fireEvent.press(checkbox);
expect(onChange).not.toHaveBeenCalled();
});
});
Loading