Adding customCancelButton and customConfirmButton types

[luizaugustoventura]

Thank you guys for an amazing lib!!

PS: I am re-creating this PR because the I did not have the right email set in my commit, so the commit signature was unverified. Sorry for that!

Overview

The library is missing the CancelButton and ConfirmButton components for the iOS only properties customCancelButtonIOS and customConfirmButtonIOS. I also found a good idea to export their styles, just in case someone (myself included) wants to do some slight changes.

All I mentioned works fine, but whenever you try to import any of these two components or their styles you get some red on your code editor. That's because the declaration file is missing these elements. So all I did was I exported from ìndex.d.ts the CancelButton and ConfirmButton components, told the declaration file the customConfirmButtonIOS and customCancelButtonIOS properties were a React.FunctionComponenet<props> with some extra props. I also exported the style types of both components.

Test Plan

There is not much to do actually, all I did was passing the buttonTextColorIOS property to the custom button I was using. For example:

import ModalDateTimePicker, { CancelButton };

<ModalDateTimePicker
    // ... <normally used props>
    customCancelButtonIOS={(props) => <CancelButton {...props} buttonTextColorIOS="red"  />}
/>

If you want to change the button styles, you might want to do something like that:

import ModalDateTimePicker, { CancelButton, CancelButtonStylePropTypes, cancelButtonStyles };

const customStyles: CancelButtonStylePropTypes = {
    ...cancelButtonStyles,
    text: {
        ...cancelButtonStyles.text,
        color: 'red',
    },
};

<ModalDateTimePicker
    // ... <normally used props>
    customCancelButtonIOS={(props) => <CancelButton {...props} style={customStyles} />}
/>

[mmazzarolo]

Hey @luizaugustoventura ! Thanks for submitting this PR :)
I added a few comments. In general, I'm not sure I'm following most changes here, but it's been ages since I updated/used this lib, so maybe I'm missing something.
From a quick check, I think the only thing we really had to update was to add the buttonTextColorIOS and cancelButtonTestID here — event though they won't matter much anyway since, if you're providing a custom component, you could also just put them in it instead of accepting them as props.

[mmazzarolo]

I'm not sure why we're exporting these styles. Froma a quick check it doesn't seem they will be used anyway, since style is not passed down to the custom component.

[luizaugustoventura]

Well, your point makes sense. But when I was doing these modifications, what I first did was importing the cancelButtonStyles and passing to a constant in my code; I modified this constant (just like I showed in the in the Test Plan) and then passed it to the style of the CancelButton on the customCancelButtonIOS property. I got this solution here.

All I wanted to do was to change the color of the CancelButton to red. So I did all of this without changing any line of code of your lib and everything worked just like the way I expected. But as I was on a TS project, the editor showed me some errors.

As I said, I just wanted to change the color of the CancelButton. But I decided to do these changes to extend the possibilities, just in case somebody wanted to change some other properties of the button, so I exported the format of the styles so that the code editor would show them how exactly the object looks like.

I did the modifications based on this issue. I did exactly what was done there, but got TypeScript errors. That's why I created this PR. Could you please take a look back there?

[mmazzarolo]

Ah! Gotcha. Could you share the TypeScript issue you were having?
If the issue is that TypeScript complains for using properties not listed in ConfirmButtonStylePropTypes, I think the right solution would be to relax the ConfirmButtonStylePropTypes so that we allow any prop other than the one we're currently passing down to into it. Let me know if it makes sense!

[luizaugustoventura]

Alright, there we go. I'm going to separate this into 2 approaches.

1. Styling the CancelButton

import DateTimePicker, { CancelButton, cancelButtonStyles } from 'react-native-modal-datetime-picker';

const customCancelButtonStyles = {
  ...cancelButtonStyles,
  text: {
    ...cancelButtonStyles.text,
    color: 'red',
  }
};

return (
   <RNModalDateTimePicker
       // ...
       customCancelButtonIOS={(props) => <CancelButton {...props} style={customCancelButtonStyles} /> }
   />
);

I get what I want by doing this. But I see some problems:

• The editor will not tell me anything about the style in CancelButton, so it will be taken as any. No TypeScript error here, but I think it's nice to have a type, so we know exactly what this component is taking in that prop;
• The cancelButtonStyles is of type any. Again, this shows no error, but I think it would be very nice if we could see how exactly the original style of the component looks like. That is useful when we want to do slight changes to the component's styles, so we are going to use the same style properties, just like it's my case in the Test Plan;
• And last and more importantly, on the imports I get two errors:

  •  import CancelButton
    
     Module '"react-native-modal-datetime-picker"' has no exported member 'CancelButton'. Did you mean to use 
     'import CancelButton from "react-native-modal-datetime-picker"' instead?ts(2614)
    

  • import cancelButtonStyles
    
    Module '"react-native-modal-datetime-picker"' has no exported member 'cancelButtonStyles'. Did you mean to 
    use 'import cancelButtonStyles from "react-native-modal-datetime-picker"' instead?ts(2614)
    

Note that the style property does not light up an error only because the CancelButton is not being exported from the declaration file, so the editor assumes CancelButton as any. But if we export the CancelButton as CancelButtonComponent, the editor certainly will say that this component does not take the property style.

2. Using the buttonTextColorIOS

import DateTimePicker, { CancelButton } from 'react-native-modal-datetime-picker';

return (
   <RNModalDateTimePicker
       // ...
       customCancelButtonIOS={(props) => <CancelButton {...props} buttonTextColorIOS="purple" /> }
   />
);

The editor will say the same thing about the CancelButton import. I know the second approach makes a lot more sense for my case, but someone (or even me) might change something else in another occasion.

In addition, the buttonTextColorIOS does not light up a red in my editor only because the CancelButton is assumed as any, as it is not being exported. But just like I said about the afore mentioned CancelButtonComponent, the editor will say CancelButton does not take the property buttonTextColorIOS.

Now going to your point, relaxing the ConfirmButtonStylePropTypes is complicated because if you look at the the way the CancelButton is being styled, you can see the styles object is being used in a very specific way as it is applied to both the Buton and its Text.

I know all of this looks bloat code maybe, but I just wanted to create more styling possibilities (something you could perfectly do in a JS project) for TS developers. That is my second ever contribution to an Open Source project, besides I created this one at the same day I created the first one.

[mmazzarolo]

Thanks for your detailed reply!
Before I start answering each bullet point separately, I wanted to make something clear.
The main discussion point here is that I think (and, again, I'm not sure about this, maybe I'm just missing something) we shouldn't mark the custom button styles as strict as we're doing in this PR.
With the current PR approach, we're basically saying that the custom button components styles can only be of the type ConfirmButtonStylePropTypes/CancelButtonStylePropTypes. Which is not true.

If you're using a custom button component, this library doesn't care about what style you pass to it.
Here's how it works for the confirm button, for example:

1. If you provide a custom component, this library won't care about the internal confirm button
2. It will just render the custom component as-it-is, without passing any style to it, just a few props.

Something like the following should be perfectly valid (just an example):

import DateTimePicker from 'react-native-modal-datetime-picker';
import { View, ViewStyle } from "react-native";

function MyCustomButton ({ style }: { style: ViewStyle }) {
  return <View style={style} />
}

// ... and then

return (
   <RNModalDateTimePicker
       customCancelButtonIOS={<MyCustomButton />}
   />
);

But with these PR changes it would break (I think so, right?) because we're saying that customCancelButtonIOS accepts only a component with a style prop that matches ConfirmButtonStylePropTypes... while in our example MyCustomButton has a style props that is just a ViewStyle.

[mmazzarolo]

I know all of this looks bloat code maybe, but I just wanted to create more styling possibilities (something you could perfectly do in a JS project) for TS developers. That is my second ever contribution to an Open Source project, besides I created this one at the same day I created the first one.

No worries! I appreciate the contribution. Let's make sure the issue I'm mentioning is clear so that we can decide how to go forward.

P.S.: I'm also not a typing expert, and I didn't create the types of this component 😅
P.P.S.: I'm quite busy lately, so I might be slow to respond in the next days. Sorry.

[luizaugustoventura]

OMG! That's right!! 😅️😅️

I'm sorry I got carried away. I was only attempting to change the originally exported CancelButton and ConfirmButton but not passing my own component to customCancelButtonIOSor customConfirmButtonIOS.

Your point is absolutely clear for me now! I think I'd only set darkModeEnabled to optional to your CancelButtomComponent and ConfirmButtonComponent. So the type definition remained actually quite the same.
P.S.: I also changed the type's name, I don't know if that's a problem to you.

export type CustomCancelButtonPropTypes = {
  isDarkModeEnabled?: boolean,
  onPress: () => void,
  label: string,
};

export type CustomConfirmButtonPropTypes = {
  isDarkModeEnabled?: boolean,
  onPress: () => void,
  label: string,
};

Now, for the originally exported CancelButton and ConfirmButton I kept those properties (style included) because I think it would be handy, as these components aren't much flexible.

I'm still also exporting the CancelButtonStylePropTypes and ConfirmButtonStylePropTypes because I think they're still useful in case we want to do some slight changes to the original CancelButton and ConfirmButton components. Also, for both of these components I think it's interesting yet to set their style property strictly as CancelButtonStylePropTypes and ConfirmButtonStylePropTypes because both CancelButton and ConfirmButton's styles are being used in a very specific (take a look here and right here) way. Some keys of the style object are being used in a TouchableHighlight and others in a Text.

But with these PR changes it would break (I think so, right?) because we're saying that customCancelButtonIOS > accepts only a component with a style prop that matches ConfirmButtonStylePropTypes... while in our example > MyCustomButton has a style props that is just a ViewStyle.

Yes, it would. I tested them the way you explained. That's why I'm pushing a new commit to my PR with the changes I explained above is this comment. Please check it if makes sense to you now.

P.P.S.: I'm quite busy lately, so I might be slow to respond in the next days. Sorry.

Same situation here. Thank you so much for you time 😉️

[mmazzarolo]

Same as above, I don't think we're passing this down to the custom cancel/confirm components.

[luizaugustoventura]

Well, I tried changing the buttonTextColorIOS and the style. Both worked like a charm.

As I said in the previous conversation, all I wanted was to change the button color. But these other properties might be useful for someone in the future.

For example: Maybe before cancel or confirm you want to prompt an Alert.

[mmazzarolo]

What are these being exported for?

[luizaugustoventura]

Are you talking about the CancelButton and ConfirmButton components?

If so, well... if you try to use them in a TypeScript project, it will say it refers to a type, not a component (when passing it to customCancelButtonIOSor customConfirmButtonIOS).

Also, if you try to import the CancelButton or ConfirmButton the editor will say these components are not being exported from the declaration file.

[mmazzarolo]

Gotcha! I'm not a TS expert, but given that we're already defining these types here, I'd update the type to be export const instead, instead of exporting them as classes (especially because they're not classes).

[luizaugustoventura]

Yeah, I did see that CancelButtonComponent and ConfirmButtonComponent were being exported, I just don't find (with all due respect) those props enough.

That's right, they're not classes. I just exported them as classes to follow the pattern in use (exporting the main component as a class), but I did not realize that the DateTimePickerModal was a class. I'm sorry for that, I'm not a TS expert either.

But you're right! I'm doing this right now!

[mmazzarolo]

Changes are looking good to me now! Thanks so much for your PR and the discussion 🙌 I'm planning to merge it as a major change. Could you double-check that it works correctly for you one last time before I release it? Thanks!

[luizaugustoventura]

Changes are looking good to me now! Thanks so much for your PR and the discussion 🙌 I'm planning to merge it as a major change. Could you double-check that it works correctly for you one last time before I release it? Thanks!

Yeah, sure. I created this repository to test some cases.

Here are the tested cases:

1. Change ConfirmButton and CancelButton's styles (using the originally exported styles)
2. a. Changing buttonTextColorIOS
   b. Changing onPress default behavior
3. Using a different component

Feel free to clone it and test as many cases as you feel necessary.

To do the tests I pulled the package from my on PR as you can check here.

P.S.: I didn't use the latest version of React Native because I reused most of a project I was working on.

Screenshots:

[github-actions]

🎉 This PR is included in version 14.0.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀