-
Notifications
You must be signed in to change notification settings - Fork 734
Feat/picker use dialog 2 #2804
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/picker use dialog 2 #2804
Conversation
enableModalBlur={false} | ||
onChange={item => this.setState({option: item})} | ||
topBarProps={{title: 'Languages'}} | ||
useDialog |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure it's related to the PR, but passing useSafeArea
does not work together with useDialog
- It can be passed via
customPickerProps.dialogProps
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix the flexness issue of the FlatList inside the dialog
searchPlaceholder={'Search a language'} | ||
> | ||
{_.map(dialogOptions, option => ( | ||
<Picker.Item key={option.value} value={option.value} label={option.label} disabled={option.disabled}/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Last items are not properly scrollable \ visible
testID | ||
} = props; | ||
const context = useContext(PickerContext); | ||
const [wheelPickerValue, setWheelPickerValue] = useState<PickerSingleValue>(context.value ?? items?.[0].value); | ||
const wrapperContainerStyle = useMemo(() => { | ||
const shouldFlex = Constants.isWeb ? 1 : useDialog ? 0 : 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like it should be only flex
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didnt change the name, but changed the logic
src/components/picker/index.tsx
Outdated
)} | ||
</ExpandableOverlay> | ||
{ | ||
// @ts-expect-error |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not {/* @ts-expect-error */}
? (I assume the whole change here is due to alignment)
@M-i-k-e-l ping on this one |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved with a small suggetsion
const shouldFlex = true; | ||
const style = {flex: shouldFlex ? 1 : 0, maxHeight: Constants.isWeb ? Constants.windowHeight * 0.75 : undefined}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const shouldFlex = true; | |
const style = {flex: shouldFlex ? 1 : 0, maxHeight: Constants.isWeb ? Constants.windowHeight * 0.75 : undefined}; | |
const style = {flex: 1, maxHeight: Constants.isWeb ? Constants.windowHeight * 0.75 : undefined}; |
Description
This PR based on PR Adi started working for a fix for Web.
We're adding some support for features that we later use in private's Picker.
useDialog - ability to show Picker as dialog
renderCustomDialogHeader - support rendering a custom dialog header by passing a callback. We pass to the callback to onDone, onCancel of the Picker so the user (us in this case) can do what they wish
Changelog
Support rendering Picker as dialog and also render custom dialog header
Additional info