react-native-modal-selector

react-native-modal-selector copied to clipboard

Since version 2, as documented, iOS onChange is called multiple times. Please could you explain why this decision was made and how to work around it?

onChange wasn't called for some users. What version of RN are you using? There was a breaking change in RN around 0.5 or 0.6 that necessitated the recent PR. We could add a prop to control this behavior so that it doesn't get called multiple times.

Sorry, should have said, we're on "0.61.5". I appreciate this was a fix for some users - unfortunately it's proved hard for us to prevent multiple callbacks. Any help appreciated. Thanks.

There are unfortunately too many variations in RN's modal behavior of onDismiss. It may be better to not use that internally.

Would you mind testing these options:

1. Delete the call to onDismiss here. Easiest way would be to edit node_modules/react-native-modal-selector/index.js
2. Use the prop onModalClose instead of onChange.

Hi, I've tried the above, but with no success. Deleting the call to onDismiss just dismisses the dialog when clicked, with no callbacks. Using onModalClose instead of onChange has the same effect. Both changes together also give the same effect. Thanks

That is surprising. onChange should be called every time a TouchableOpacity is pressed. In the case that it doesn't call onChange, are you selecting an item or pressing "cancel"? Please paste your <ModalSelector> usage in case there are any other props interfering.

To break this down: If I comment out line 313 of index.js, then the onChange gets called but the resultant dialog that we open (to select a document/photo) gets closed immediately.

<ModalSelector
      style={styles.thumbnailContainerStyle}
      data={fileOptions}
      onChange={onChange}
      closeOnChange
      cancelText="Cancel"
      optionTextStyle={styles.optionsText}
      optionContainerStyle={styles.optionContainer}
      cancelTextStyle={styles.cancelText}
      cancelStyle={styles.cancelContainer}
    >

And our onChange is:

const onChange = (option) => {
    switch (option.key) {
      case 0:
        ImagePicker.openCamera(...)
        break;
      case 1:
        ImagePicker.openPicker(...)
        break;
      case 2:
        DocumentPicker.pickMultiple(...)
        break;
      default:
        break;
    }
  };

If I don't comment out that line, then our onChange gets called twice, so for instance the document picker dialog opens, closes, then re-opens.

If I don't comment out that line, and use 'onModalClose' instead of 'onChange', the behaviour is the same as commenting out the line ie our dialog opens, then immediately closes.

If I both comment out the line and use 'onModalClose', the behaviour is the same as above.

Is it closing after you select an option, or immediately after the modal opens? If you don't want the modal to close upon a selection, use the prop closeOnChange={false}. Your code passes in that prop without a value which makes it true. https://reactjs.org/docs/jsx-in-depth.html#props-default-to-true

If that's not the case, this is very puzzling. onDismiss isn't a required prop, so omitting that shouldn't cause the modal to close on its own. https://reactnative.dev/docs/modal/#ondismiss

Does ImagePicker.openPicker() open this modal?

Yes, it's closing immediately after an option is selected. closeOnChange={false} fixes this immediate closing when onDismiss is commented out but in that case, I need to close the modal manually by pressing "cancel", which can be annoying for the user.

@dx-pranta-bot This modal is designed to close when an option is selected. Since closeOnChange={false} doesn't do what you need it to, what different behavior would you like?

@justincrow I re-read your notes and think I understand what I missed earlier. It sounds like you use <ModalSelector> to open another dialog (e.g. Document picker) and want that second dialog to remain open. However if <ModalSelector> closes, the Document picker also closes.

<ModalSelector> is designed by default to close on selection. In your use case, use the prop closeOnChange={false} to prevent it from closing on selection. You'll then need to close the modal after the picker is done. The visible prop allows for manual control of the modal open/close state. In your case it may be more convenient to imperatively close it. To do that, obtain a ref to the <ModalSelector> instance and call its close() method.

PS: What are the values passed to the onChange callback when it's fired multiple times?

Sorry for the radio silence. The values passed to the onChange are the same on both 1st and 2nd callbacks eg {"key": 1, "label": "Photo library"}

If we use closeOnChange={false}, then the first callback successfully opens our (eg Document Picker) dialog, but duplicate callback is delayed until we close the dialog by pressing cancel.

Using the visible prop gives the following: when set to true, the dialog never opens, and when set to false, the dialog behaves the same as omitting the visible property altogether.

When closing the modal selector manually using a ref, the 2nd duplicate callback is again invoked upon the close.

In summary, it would seem we always get the duplicate callback regardless of how the selector is closed.

Thanks.

Thanks for the updated notes. Does your Document picker live inside or outside of the modal? It's surprising that the modal state change would cause the picker to close. Perhaps the OS native modal interferes with the native picker.

The close() method calls onModalClose(), but shouldn't call onChange() as long as you comment out the onDismiss prop on line 313. Have you tried that combination?

@justincrow A race condition could be contributing to your corner case. As the modal is closing, it calls onChange (or onModalClose). However if the modal hasn't fully completed closing, and your Document picker starts to open, it could interfere. What may help is to delay the onModalClose callback. Since not everyone may want that delay, I propose adding a delay value prop to let users set it for their use case.

Some historical context related to this issue: #114

I found that first time it call onChange with key-label pair object {onChange: {key: "CHECK_IN", label: "Check in"}} But the second time it call it just with key {onChange: "CHECK_IN"}

so I did a weird thing:

<ModalSelector
              data={EventTypes}
              style={EVENT_TYPE_CONTAINER}
              onChange={(option) => {
                console.log({ onChange: option })
                if (option.key) { // workaround for ios https://github.com/peacechen/react-native-modal-selector/issues/140
                  setType(option.key)
                }
              }}
              selectedKey={type}
            >

And it works for me

Thanks @AlexeyKhmelev for reporting this. Would you mind opening a new issue? It helps with tracking bug fixes.

<ModalSelector
                                data={this.props.listSupporters}
                                accessible={true}
                                cancelText={'Huỷ'}
                                overlayStyle={{
                                    paddingBottom: Metrics.doubleBaseMargin,
                                    paddingTop: Metrics.screenHeight / 20
                                }}
                                cancelTextStyle={Fonts.style.normal_bold}
                                onModalClose={(option) => {
                                    option.key && this.onChangeSupporter(option)
                                }}
                                // onChange={(option) => {
                                //     this.onChangeSupporter(option)
                                //     console.log(option);
                                // }}
                                keyExtractor={(item) => item.key}>

This is my solution. Use onModalClose instead of onChage. Hope it useful!

Is there any chance this is going to be fixed?

I found that first time it call onChange with key-label pair object {onChange: {key: "CHECK_IN", label: "Check in"}} But the second time it call it just with key {onChange: "CHECK_IN"}

so I did a weird thing:

<ModalSelector
              data={EventTypes}
              style={EVENT_TYPE_CONTAINER}
              onChange={(option) => {
                console.log({ onChange: option })
                if (option.key) { // workaround for ios https://github.com/peacechen/react-native-modal-selector/issues/140
                  setType(option.key)
                }
              }}
              selectedKey={type}
            >

And it works for me

Thanks! It works for me.

Any update on when this is properly fixed?

Apologies for the late response. Android and iOS native modals behave differently, and changing it for iOS could break Android. However it looks like @AlexeyKhmelev 's check for the empty key field would work.

Fix has been published in 2.0.5. Please test this on both iOS and Android.

good catch @glinda93 . Published 2.0.6 to address the key=0 bug.

It sounds like there are different causes for the reported issue. Have you been able to debug it?

The recent change does not use the keyExtractor function thus breaking use cases where the keyExtractor function is used. I just realised that onChanged is not called anymore.

Sorry about that @aschmitz-wbd . Fix released in 2.0.7

Thx a lot for the quick response and fix :)

Hi, onChange firing twice is still happening for me on iOS 14.3, RN 0.64.2 using release 2.0.7. The onModalClose workaround as suggested by @imginnn is working

onChange is unreliable, onModalClose works without a problem

However, onModalClose does not give you the key/label info of the option you selected

Update: I see it does in the code, but the typings for this package do not indicate this. Please update your typings/docs

@scousino

I see it does in the code, but the typings for this package do not indicate this. Please update your typings/docs

maybe you can open a PR for this

#166

I just lost part of my day looking for errors in my logic due to onChange behavior being incompletely documented.

onModalClose is a more narrowly focused event and works as I expected onChange to behave. The documentation states onChange:

callback function, when the users has selected an option

It's definitely triggered by other actions and can result in a double toggle behavior