web-ridge / react-native-paper-dates

Smooth and fast cross platform Material Design date and time picker for React Native Paper
https://www.reactnativepaperdates.com
MIT License
668 stars 174 forks source link

Add option to pass modal props to DatePickerModal #21

Closed ykliuiev closed 3 years ago

ykliuiev commented 4 years ago

Hi,

First of all wanted to say thank you, this package looks very nice 👍

I think we should have an option to pass modal props such as animationType, presentationStyle instead of hard coded values like this https://github.com/web-ridge/react-native-paper-dates/blob/master/src/DatePickerModal.tsx#L97

Let me know what do you think. I can send a PR if this sounds ok for you

RichardLindhout commented 4 years ago

I will accept the animationType PR. The presentationStyle needs to fixed but it's not good now it needs to be like in the development branch: https://github.com/web-ridge/react-native-paper-dates/blob/development/src/Date/DatePickerModal.tsx

To fix iOS en Android behaviour inside statusbar.

I'll try to release a new version of this package soon where some of these things are fixed.

RichardLindhout commented 4 years ago

I'll also extract the modal content as a different component so it will be possible to include the datepicker inside a custom modal

ykliuiev commented 3 years ago

Cool, so I thought about picking animationType from ModalProps:

type PickedModalProps = Pick<ModalProps, 'animationType'>

Separate type with Pick will allow us to pick more props from ModalProps at a later stage. For example if there would be a need to pick 'presentationStyle' also: type PickedModalProps = Pick<ModalProps, 'animationType' | 'presentationStyle'>

then extend it from DatePickerModalProps:

interface DatePickerModalProps
  extends HeaderPickProps,
    BaseCalendarProps,
    PickedModalProps {
  mode: ModeType
  visible: boolean
  onDismiss: () => any
  inputFormat?: string
}

after that extract from props:

const {
    ...,
    animationType,
  } = props

and provide it to Modal as: animationType={animationType}

Since animationType is set to none by default provide it also to example app component. const animationType = Platform.select<ModalProps['animationType']>({ web: 'fade', default: 'slide', });

and use it later animationType={animationType}

Let me know what do you think about this implementation

RichardLindhout commented 3 years ago

For now I rather have an explicit slide/fade instead of picking from modal, for web I think we could pick 'none' as default.

Maybe in a newer version I want to support custom animations .

The presentationStyle should not be set, I rather want to control this in the modal and always use the fullScreen mode (which is now not the case)

ykliuiev commented 3 years ago

got it, so this should do the job?

interface DatePickerModalProps extends HeaderPickProps, BaseCalendarProps {
  mode: ModeType
  visible: boolean
  onDismiss: () => any
  inputFormat?: string
  animationType?: 'slide' | 'fade' | 'none'
}

And for example app pass it inline so there is no warning from the typescript

animationType={Platform.select({
  web: 'none',
  default: 'slide',
})}
RichardLindhout commented 3 years ago

Let get this merged!