Feat/SectionsWheelPicker new component #1260
Conversation
demo/src/screens/componentScreens/SectionsWheelPickerScreen.tsx
Outdated
Show resolved
Hide resolved
|
Looks great! I wrote a few comments.. |
| @@ -56,7 +54,7 @@ const SectionsWheelPicker = (props: SectionsWheelPickerProps) => { | |||
| const renderSections = () => | |||
| _.map(sections, (section, index) => { | |||
| return ( | |||
| <View height={1} width={section.style?.width || defaultWidth} key={index} testID={testID}> | |||
| <View height={1} width={section.style?.width} key={index} testID={testID}> | |||
There was a problem hiding this comment.
Passing the width here is redundant cause the width should come from the wheel picker content and the style is passed there
| @@ -56,7 +54,7 @@ const SectionsWheelPicker = (props: SectionsWheelPickerProps) => { | |||
| const renderSections = () => | |||
| _.map(sections, (section, index) => { | |||
| return ( | |||
| <View height={1} width={section.style?.width || defaultWidth} key={index} testID={testID}> | |||
| <View height={1} width={section.style?.width} key={index} testID={testID}> | |||
| <WheelPicker testID={`${testID}.${index}`} {...wheelPickerProps} {...section} style={{width: '100%'}}/> | |||
There was a problem hiding this comment.
Same here, no reason to pass the width, the content of the wheel picker should determine its size.
Also, regardless, passing the style like that overrides the user style
src/incubator/WheelPicker/Item.tsx
Outdated
| style={{ | ||
| height: itemHeight | ||
| height: itemHeight, | ||
| minWidth: Spacings.s10 | ||
| }} |
There was a problem hiding this comment.
Use StyleSheet for static styles.
And I suggest memoizing the style we pass here, It can save performance.
src/index.ts
Outdated
| export {default as SegmentedControl, SegmentedControlProps} from './components/segmentedControl'; | ||
| export {default as SegmentedControl, SegmentedControlProps, SegmentedControlItemProps} from './components/segmentedControl'; | ||
| export {default as Switch, SwitchProps} from './components/switch'; |
There was a problem hiding this comment.
I don't think this one is part of the PR (:
Merging master will probably fix that
There was a problem hiding this comment.
There was a conflict here after merging master
src/incubator/WheelPicker/Item.tsx
Outdated
| import Animated, {interpolateColors} from 'react-native-reanimated'; | ||
| import Text from '../../components/text'; | ||
| import TouchableOpacity from '../../components/touchableOpacity'; | ||
| import {TextStyle} from 'react-native'; | ||
| import {TextStyle, StyleSheet} from 'react-native'; | ||
| import {Colors, Spacings} from '../../../src/style'; |
There was a problem hiding this comment.
keep module imports at the top, relative imports after
Description
Create a new SectionsWheelPicker component to present a set of wheelPickers
Jira 155
Changelog
Create a new SectionsWheelPicker component to present a set of wheelPickers