Implement support for React Native

[gricard]

• downshift version: 1.5.0
• node version: 8.3.0
• npm (or yarn) version: yarn 1.0.1

Relevant code or config

import React from 'react';
import { Input } from 'native-base';
import Downshift from 'downshift'

export default function AutocompleteInput({items, onChange}) {
    return (
        <Downshift onChange={onChange}>
            {({
                getInputProps,
                getItemProps,
                getRootProps,
                isOpen,
                inputValue,
                selectedItem,
                highlightedIndex
              }) => {
                return (
                    <Input {...getRootProps({refKey: "categoryTesting"})} {...getInputProps()} />
                );
            }}
        </Downshift>
    )
}

What you did:
Added an autocomplete input component to my React Native project and attached Downshift to it.

What happened:

window.addEventListener is not a function. (In 'window.addEventListener('mousedown', onMouseDown)', 'window.addEventListener' is undefined)

componentDidMount
downshift.cjs.js:699:30
measureLifeCyclePerf
ReactNativeStack-dev.js:1610:15

ReactNativeStack-dev.js:1657:33
notifyAll
ReactNativeStack-dev.js:2121:107
close
ReactNativeStack-dev.js:2138:8
closeAll
ReactNativeStack-dev.js:1412:101
perform
ReactNativeStack-dev.js:1388:52
batchedMountComponentIntoNode
ReactNativeStack-dev.js:2004:24
perform
ReactNativeStack-dev.js:1382:99
renderComponent
ReactNativeStack-dev.js:2032:45
renderApplication
renderApplication.js:34:4
runApplication
AppRegistry.js:191:26
__callFunction
MessageQueue.js:266:47

MessageQueue.js:103:26
__guard
MessageQueue.js:231:6
callFunctionReturnFlushedQueue
MessageQueue.js:102:17

Reproduction repository:
https://github.com/gricard/downshift-reactnative-repro

Problem description:
Downshift does not currently have support for the React Native environment, which has some minor differences to running in the browser.

Suggested solution:
I had a brief discussion about the issue with Kent, and he provided these three links which are areas where there will need to be changes in order to support React Native:

https://github.com/paypal/downshift/blob/a1490f070575ffcd71dac8c063fe7e092f840a67/src/downshift.js#L208-L209

https://github.com/paypal/downshift/blob/a1490f070575ffcd71dac8c063fe7e092f840a67/src/downshift.js#L150

https://github.com/paypal/downshift/blob/a1490f070575ffcd71dac8c063fe7e092f840a67/src/downshift.js#L655-L681

I'd like to take a shot at implementing a fix and providing a PR. I'm looking for guidance on that, as this is the first time I've taken the time to attempt contributing back to a project, and I'm also fairly new at RN as well.

(FYI, there is no CODE_OF_CONDUCT.md file that is mentioned in the issue template)

[kentcdodds]

Hi @sumorai!

Thanks so much for filing this issue and taking up the challenge to bring downshift to React Native! I have no experience with React Native either. I'll reach out to folks on twitter and see if we can get some direction there.

As mentioned to you privately, I would be totally fine to add a few lines of logic that make things compatible. I'd also be fine to add another build if that's necessary (though I'd prefer to avoid that).

(FYI, there is no CODE_OF_CONDUCT.md file that is mentioned in the issue template)

Thanks for pointing that out. It's in other/CODE_OF_CONDUCT.md.

[Swizec]

I had a look around and I'm not having luck using npm link with npm that comes with the latest node version (needed to run kcd-scripts and compile downshift). That's why I can't test the changes I made.

But from what I can tell, making downshift support React Native is going to be trickier than it looks at first glance. Avoiding window.addEventListener is easy enough, you wrap it in an if and check that the function exists. React Native isn't going to need the global mousedown listener, the concept doesn't exist as far as I know.

getItemNodeFromIndex is going to have to be rewritten to use refs. As per idiomatic React, downshift should use refs for this sort of thing anyway. My understanding is that accessing the DOM directly is frowned upon :)

The tricky part are going to be all those event listeners.

onClick needs to understand onPress as well.

keyDownArrowUp, keyDownEnter etc. none of those mean anything in React Native. People don't navigate using their keyboard, they swipe around with their fingers.

onChange and onBlur are okay.

onInput doesn't exist on React Native. I think onChangeText is its equivalent.

Finally, using Preact as the build target is going to be an issue. Preact purposefully drops support for React Native as part of the whole "be super small" thing.

My assessment is that adapting the library itself for RN is doable, but it's almost certainly going to need a separate build target because of Preact.

[kkemple]

Agree with @Swizec 100%, accessibility would be different as well I think.

It seems like a glamorous situation IMO, maybe make it another package altogether?

[kentcdodds]

Thank you so much for your audits @Swizec and @kkemple!

React Native isn't going to need the global mousedown listener, the concept doesn't exist as far as I know.

The reason for the global mousedown listener is to avoid this situation when the user clicks an item:

input onblur -> menu isOpen set to false -> onclick event fired on whatever the menu was covering

So we have the global mousedown to track whether the mouse is still down when the onBlur event happens. If that's the case, then we don't close the menu until the mouseup event fires.

Will that be a problem with React Native?

getItemNodeFromIndex is going to have to be rewritten to use refs. As per idiomatic React, downshift should use refs for this sort of thing anyway. My understanding is that accessing the DOM directly is frowned upon :)

The problem with using ref is if you're rendering anything other than the raw DOM node, then you have to forward my ref function to whatever component does render the raw DOM node (using something like innerRef from glamorous) otherwise your ref will be called with the component instance which will not help.

I suppose an alternative here would be to simply use onClick on each item rather than delegating to the root node. I think that should work, though I feel like there was a reason I went with the root node onClick handler and I can't remember what it was. We'll have to experiment with that a bit... I think that if that works, we can avoid the ref/DOM nonsense altogether which would be great. At least for items...

onInput doesn't exist on React Native. I think onChangeText is its equivalent.

No worries, that's only used for the preact build. For the react build (which would be the one to support react native) it'll be onChange, is that ok?

using Preact as the build target is going to be an issue. Preact purposefully drops support for React Native as part of the whole "be super small" thing.

No worries there, preact isn't the target, it's a target. downshift supports both and simply has a separate build for preact :)

accessibility would be different as well I think.

Yeah, I was thinking about that too. I don't expect that set-a11y-status.js would apply at all, and we'd need a different thing for React Native...

It seems like a glamorous situation IMO, maybe make it another package altogether?

Let's see if we can make it work. downshift is a much simpler component, so I think we can make it work :)

[gricard]

Awesome. Thanks for the input, everyone. :)

I forked it and started fiddling around on the train this morning. I'll spend some more time on it tonight.

[gricard]

Yeah, I was thinking about that too. I don't expect that set-a11y-status.js would apply at all, and we'd need a different thing for React Native...

From the RN docs it looks like we just need to apply some accessibility props to the component, and potentially to the list items. So, those could just be passed back in getInputProps() and getItemProps(), right?

I'm curious exactly how set-a11y-status.js works. Why does it maintain an array of the statuses, and what are those statuses intended to be? I played around with one of the examples and sometimes it would be the most likely match in the list, and other times it had an informational message. I suppose I'd have to play around with it in a test app after it's working and see what makes sense.

The AccessibilityInfo API can be queried to see if the screen reader is enabled, although, considering that the user can probably turn that on/off and go back to the app, it wouldn't likely make sense to use that API to decide whether or not to add a11y props to the elements.

Input would have a prop "accessibilityTraits" of "search", and the items would have a "accessibilityTraits" value of "selected" when selected, otherwise, perhaps "text". Input and items would all have "accessible" prop set to true. The "accessibilityLabel" prop can also be used. I'm not sure if that would be a good place for the status, or not.

[gricard]

@kentcdodds In validateGetRootPropsCalledCorrectly(), the onClick check there on the root node is probably not needed for React Native, right? At least, from reading the discussion above, that seems like the case.

At any rate, I updated the repro code a bit and it a baby step towards functional with some changes to my fork of downshift.

[kentcdodds]

Thanks for your work on this @gricard!

I'm curious exactly how set-a11y-status.js works.

Don't concern yourself as much with how set-a11y-status.js work and just know that it's responsibility is to alert the user whenever there's a relevant change in the state of the menu based on their interaction. The best way to get an idea of the potential statuses, you can look at the implementation of getA11yStatusMessage : https://github.com/paypal/downshift/blob/4e05ca71f87e77c33ded119c525b42b4f3945d0e/src/utils.js#L156-L180

It would be simple to add an if statement here and use a react-native implementation rather than the web one...

In validateGetRootPropsCalledCorrectly(), the onClick check there on the root node is probably not needed for React Native, right?

Possibly... If we rework downshift to use a click handler on the items rather than on the root node then we could probably get rid of the root's onClick handler entirely.

One last thing. My wife and I just had a baby, so I'll be pretty busy with that. I'll try to be as available as I can but I can't make any promises for the next few weeks. Good luck!

[gricard]

@kentcdodds Congrats! Speaking as a dad of a toddler, get as much sleep as you can. :D

I'll keep fiddling with things and keep this updated.

[kentcdodds]

With some of the changes recently I think that support will be even easier. Especially the new environment prop I think will be useful. What do you all think? There's probably still work to be done around the input element...

[eliperkins]

I'm pretty interested in this too! Currently reimplementing a set of Downshift-like inputs and filters in a React Native codebase.

@gricard let me know if I can lend a hand at all!

[kentcdodds]

I honestly don't think that it'd take a whole lot of work to make this happen. Someone's just gotta put in the effort. It can't be me because I'm afraid that I don't have experience with React Native, but I look forward to seeing someone work on this :)

[eliperkins]

@kentcdodds Sounds good! I'll take a stab at this next week after consuming some turkey.

@gricard I assume your branch was here? https://github.com/gricard/downshift/tree/test/gr I'll grab that work and start to build off it a bit.

[eliperkins]

I took a quick swipe at this here: #265

Feedback is welcome!

[kentcdodds]

Re-opening as what was just merged is experimental support. That means that things may break intentionally without a semver major bump. To get to official support we need:

1. A committed maintainer (I don't use RN, so I can't maintain it). I think at least 2 of you are committed here :)
2. At least one integration test (see ./other/misc-tests). I don't know what would be involved, but I would like to make sure we have something in place to not break RN before we commit to supporting it.
3. Documentation
4. Someone (hopefully multiple someones) who can confirm that this works on a real device :)

[donysukardi]

I don't really use RN as much as I'd like either. But I whipped up a quick Expo Snack of Downshift in RN in action https://snack.expo.io/@donysukardi/downshift-basic-example just to try it out.

Awesome work @Andarist, @eliperkins!

We might also want consider adding a separate Example section for RN, @kentcdodds.

[kentcdodds]

Awesome! Thank you! Yes another example section would be wonderful!

[eliperkins]

@donysukardi Heh, awesome! I had a very similar version of this I was using while working on #265.

A committed maintainer (I don't use RN, so I can't maintain it). I think at least 2 of you are committed here :)

I'm certainly committed to maintaining this, as we'll be using this in production at Clubhouse!

At least one integration test (see ./other/misc-tests). I don't know what would be involved, but I would like to make sure we have something in place to not break RN before we commit to supporting it.

Agreed. I think we can put together some tests for this using Jest's React Native snapshot testing, to at least ensure we're able to render the React Native components we need.

Documentation

Absolutely! What do you envision this looking like? The usage of Downshift on React Native is analogous to it's usage with ReactDOM, so I'm not sure that we need separate documentation for it. Would an example be enough?

Someone (hopefully multiple someones) who can confirm that this works on a real device :)

We'll be launching our app in ~week that uses this in several places. I'll be sure to link ya'll to it.

[kentcdodds]

Would an example be enough?

Let's have a section in the docs that talk about react native usage. It should mention any differences and show an example 👍

[eliperkins]

I think what might be the first use of Downshift on React Native is now live in the App Store! We're using Downshift for selecting and filtering lists in a few places:

If you're a Clubhouse user, check it out here

[kentcdodds]

I have so many happy feelings about this that I tweeted a long thread: https://twitter.com/kentcdodds/status/964176513657724928

So here's what we've got:

• A committed maintainer (I don't use RN, so I can't maintain it). I think at least 2 of you are committed here :)
• At least one integration test (see ./other/misc-tests). I don't know what would be involved, but I would like to make sure we have something in place to not break RN before we commit to supporting it.
• Documentation
• Someone (hopefully multiple someones) who can confirm that this works on a real device :) (IT NOT ONLY WORKS IT'S SHIPPED TO THE APP STORE! WAT!!! 🎉)

[wKovacs64]

Are you using a FlatList in renderList, @eliperkins? I tried forking @donysukardi's example and replacing the map with a FlatList, but it seems to break Downshift. onChange is no longer called, selectedItem is not updated, etc.

https://snack.expo.io/SkRIbspdM

[eliperkins]

@wKovacs64 Yup, we're using FlatList and it's working for our use-case.

This sounds similar to #363 that was just reported as well. Let's use that issue to track this, and see if it solves your problem, rather than overloading this issue.

[kentcdodds]

Hey folks! I'm getting things ready for a v2 release this week and would love react-native support to be official for it. Hope are we doing on that?

[eliperkins]

@kentcdodds it looks like the last thing in that checklist of yours that we need is documentation and an example!

Based on the discussion in #363, it might be helpful to note the fix to that problem (including the keyboardShouldPersistTaps="handled" prop on ScrollViews used within Downshift) within the documentation for it as well.

Beyond that, I would say this is good to go in v2!

[kentcdodds]

Awesome. I think it'd also be great to close existing react-native issues 👍 Can we get any of those resolved?