Skip to content
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

parent.contains is not a function when used with styled components #490

Closed
davereid91 opened this issue Jun 27, 2018 · 15 comments · Fixed by #492, #525 or #535
Closed

parent.contains is not a function when used with styled components #490

davereid91 opened this issue Jun 27, 2018 · 15 comments · Fixed by #492, #525 or #535

Comments

@davereid91
Copy link

  • downshift version: 2.0.10
  • node version: 9.5.0
  • npm (or yarn) version: 1.5.1

Relevant code or config

        <Downshift
            defaultSelectedItem={selectedOption}
            itemToString={item => (item ? item.message : '')}
        >
            {({
                highlightedIndex,
                itemToString,
                getItemProps,
                getMenuProps,
                getToggleButtonProps,
                isOpen,
                selectedItem,
                toggleMenu,
            }) => (
                <div>
                    <Dropdown.Container>
                        <Button {...getToggleButtonProps()}>Button Toggle</Button>
                        <Dropdown isOpen={isOpen} {...getMenuProps()}>
                            {options.map((option, index) => (
                                 <div
                                    {...getItemProps({
                                        key: option.id,
                                        index,
                                        item: option,
                                        isActive: highlightedIndex === index,
                                    })}
                                />
                            ))}
                        </Dropdown>
                    </Dropdown.Container>
                </div>
            )}
        </Downshift>

What you did:
The code above is pseudo code but roughly equates to what I am doing. I am building a dropdown using downshift that toggles when you click a button. I am combining this with styled components and when I click outside of the div I am getting an error from within downshift.

What happened:
The error that I am getting is the following:

screen shot 2018-06-27 at 14 40 47

When digging deeper into this, it's because the parent in isOrContainsNode is coming through as a Styled Component wrapper as you can see in the screenshot below:

screen shot 2018-06-27 at 14 45 08

Problem description:
Clicking outside of the downshift container div when using styled components is throwing an error and the dropdown window stays open.

@alexandernanberg
Copy link
Collaborator

I think the problem is that the getMenuProps is trying to set a ref on the <Dropdown />. Styled components accepts an innerRef prop for this, so please try with this instead

<Dropdown isOpen={isOpen} {...getMenuProps({ refKey: 'innerRef' })} />

Altough we should probably add a check if the ref exists before we try accessing it.

@kentcdodds
Copy link
Member

we should probably add a check if the ref exists before we try accessing it.

Agreed. I'd accept a PR for that 👍

@davereid91
Copy link
Author

Yep that fixed it, thanks very much @alexandernanberg :)

arthurdenner added a commit to arthurdenner/downshift that referenced this issue Jun 28, 2018
kentcdodds pushed a commit that referenced this issue Jun 28, 2018
…#492)

* fix(utils): Checking if parent.contains is a function before using it

Closes #490

* chore: Adding myself as a contributor
@alexandernanberg
Copy link
Collaborator

@kentcdodds Been thinking a bit more about this one. Wouldn't it be good to also print a warning when this happens? The ref is there for solving issues when using the menu inside a portal, so this could lead to unexpected behavior 🤔

@morajabi
Copy link
Contributor

Also, it seems it's not in type definitions.

@kentcdodds
Copy link
Member

Yes, we should log a warning 👍

@kentcdodds kentcdodds reopened this Jun 29, 2018
@kentcdodds
Copy link
Member

And we should add type definitions.

@franklixuefei
Copy link
Collaborator

franklixuefei commented Jun 29, 2018 via email

@xshyne88
Copy link

Is there any update on this, I'm still hitting it in my codebase.

@kentcdodds
Copy link
Member

@xshyne88, this is the solution: #490 (comment)

@franklixuefei
Copy link
Collaborator

Uh... I need some help here...

I applied getMenuProps() to a primitive element, <div>, but it still complains that getMenuProps() was not applied correctly.

my code looks something like below, where I applied getMenuProps on a div, but that div itself is under some composite component.

<SomeCompositeComponent>
  <div {...getMenuProps()}>
    {here goes the menu items}
  </div>
</SomeCompositeComponent>

I thought we only need to specify a refKey if getMenuProps is applied to a composite component, no?

I literally have no idea how to fix this. Please help! Thanks!

@franklixuefei franklixuefei reopened this Aug 4, 2018
@kentcdodds
Copy link
Member

@franklixuefei
Copy link
Collaborator

franklixuefei commented Aug 7, 2018

@kentcdodds Okay I found the problem. I was mounting and unmounting the entire menu as needed. Apparently menu is meant to be mounted all the time. Not sure if we want to document this explicitly?

franklixuefei added a commit that referenced this issue Aug 7, 2018
Fixes the typing part of #490
@kentcdodds
Copy link
Member

Yes, for accessibility reasons it should always be rendered. Feel free to make a PR to improve docs. I think it is mentioned but could probably be called out better.

@kentcdodds
Copy link
Member

🎉 This issue has been resolved in version 2.1.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

kentcdodds pushed a commit that referenced this issue Aug 20, 2018
* Improved typings for getMenuProps

Fixes the typing part of #490

* Update index.d.ts

* Fixed build error

* Update index.d.ts
Rendez pushed a commit to Rendez/downshift that referenced this issue Sep 30, 2018
…downshift-js#492)

* fix(utils): Checking if parent.contains is a function before using it

Closes downshift-js#490

* chore: Adding myself as a contributor
Rendez pushed a commit to Rendez/downshift that referenced this issue Sep 30, 2018
* Improved typings for getMenuProps

Fixes the typing part of downshift-js#490

* Update index.d.ts

* Fixed build error

* Update index.d.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment