Dropdown don't close on select

[VlodkoMr]

Describe the bug I use Dropdown and when select item from this dropdown, it is still opened. Documentation also don't have any information how can we do it.

Reproduce:

• Click on dropdown to open it.
• Click on any option element
• Result: nothing changed. Action that was added as onClick for Dropdown.Item work but dropdown is opened.

Project information:

• Tailwind: 3.1.8
• Flowbite: 1.5.3
• Flowbite React: 0.1.10

[Nefarioum]

I am experiencing the same issue with this. Is there some sort of way I can hide it temporarily for now until a fix is release?

[AstrisDev]

I am experiencing with this bug too. Is any idea how to close dropdown on item click?

[thedaviddias]

Up!

[tiagossa1]

+1

[vonnue-be]

+1

[ShaneMaglangit]

Can I work on this?

[rluders]

Sure, @ShaneMaglangit

[thiendang]

It's still not working on v0.1.11

[sazzer]

I've got the same problem. However, I'm putting navigation links into my dropdown - courtesy of react-router - and I'm finding that once I've used one of these links the dropdown not only remains open but doesn't close if I click outside it. The only way to close it is to click on the opener button again. The dropdown does close when clicking outside it before I click on one of these links though.

I'm assuming that the Dropdown.Item has an onClick to cause the menu to close, and that something is going wrong with it when the item has other functionality as well.

[ShaneMaglangit]

@sazzer actually, its precisely that Dropdown.Item does not have any functionality that updates the state of the floating menu. It is nothing more than a wrapper to a <li> element. The way that it is designed to be entirely independent of Dropdown makes it challenging to add that, since it would require passing the setOpen of Floating to its children up to the dropdown item.

I do have a proposed solution but I've had problems with commitlint when trying to commit my changes. Haven't really had the extra time to sort it out, so it remains on my local environment.

[donnguyen]

We might need to completely re-implement the Dropdown component. React DOM Interactions now support a tree with nested item, it could be a good way to go, the example is available here: https://codesandbox.io/s/admiring-lamport-5wt3yg?file=/src/DropdownMenu.tsx

[rluders]

PR is very welcome to fix this one.

[sayinmehmet47]

+1

[ShaneMaglangit]

@rluders perhaps we can add the hacktoberfest tag on the repo to attract more help on this?

[vylink]

+1

[fs-innonova]

+1 please

[WenLonG12345]

+1

[nsoubelet]

@sazzer @ShaneMaglangit To me this is the kind of issues one could find when designing using a composite approach with isolation, I mean, separating components completely. In this case Dropdown knows nothing about Dropdown.Items and viceversa. I rather prefer to think of events, therefore Dropdown.Items could handle clicks internally and first notify its parent (Dropdown) and then execute component's handler function.

@ShaneMaglangit instead of passing down setOpen to children, would not it be cleaner to only notify parent and let it decide so concerns keep separated?

[ShaneMaglangit]

@nsoubelet May you clarify what is your proposed solution? Based on how I initially understood your suggestion, propagating setOpen is doing exactly what you hope to achieve. The Dropdown container is notified of the event when setOpen is called by the child.

[nsoubelet]

@ShaneMaglangit I did not say your solution won't work, I just said that maybe a better practice would be notify Dropdown instead of calling setOpen in a child component. open is a Dropdown inner property and ideally should be changed and only visible by the component itself.

Having said that, I agree with you the effect will be the same using either style and we all need this fixed ASAP.

[ShaneMaglangit]

@ShaneMaglangit I did not say your solution won't work, I just said that maybe a better practice would be notify Dropdown instead of calling setOpen in a child component. open is a Dropdown inner property and ideally should be changed and only visible by the component itself.

Having said that, I agree with you the effect will be the same using either style and we all need this fixed ASAP.

I do have a proposed solution in my local environment, I'll try to deal with the issues so I can submit a PR tonight.

I do understand what you are suggesting. However, I'm having trouble figuring out how to do this precisely on React. If you can clarify how to do this, perhaps I can modify my work a bit to better align with your idea.

To be clear, I would appreciate it if you can explain how to notify Dropdown from DropdownItem aside from propagating setOpen?

[nsoubelet]

@ShaneMaglangit I did not say your solution won't work, I just said that maybe a better practice would be notify Dropdown instead of calling setOpen in a child component. open is a Dropdown inner property and ideally should be changed and only visible by the component itself. Having said that, I agree with you the effect will be the same using either style and we all need this fixed ASAP.

I do have a proposed solution in my local environment, I'll try to deal with the issues so I can submit a PR tonight.

I do understand what you are suggesting. However, I'm having trouble figuring out how to do this precisely on React. If you can clarify how to do this, perhaps I can modify my work a bit to better align with your idea.

To be clear, I would appreciate it if you can explain how to notify Dropdown from DropdownItem aside from propagating setOpen?

Something like this:

In Dropdown component

const handleClickDropDown = () => {
  setOpen(!open);
}

In Dropdown.Item component

const handleClickItem = () => {
  handleClickDropDown();
  itemClickHook();
}

handleClickDropDown you pass same way you are planning to pass setOpen. itemClickHook is the function that comes from the top-level component, passed in onClick prop. In this approach item handles the click internally, notifies its parent and then continues upstream.

[rluders]

Some reference: https://github.com/fraserxu/react-dropdown/blob/master/index.js

[ShaneMaglangit]

@ShaneMaglangit I did not say your solution won't work, I just said that maybe a better practice would be notify Dropdown instead of calling setOpen in a child component. open is a Dropdown inner property and ideally should be changed and only visible by the component itself. Having said that, I agree with you the effect will be the same using either style and we all need this fixed ASAP.

I do have a proposed solution in my local environment, I'll try to deal with the issues so I can submit a PR tonight. I do understand what you are suggesting. However, I'm having trouble figuring out how to do this precisely on React. If you can clarify how to do this, perhaps I can modify my work a bit to better align with your idea. To be clear, I would appreciate it if you can explain how to notify Dropdown from DropdownItem aside from propagating setOpen?

Something like this:

In Dropdown component

const handleClickDropDown = () => {
  setOpen(!open);
}

In Dropdown.Item component

const handleClickItem = () => {
  handleClickDropDown();
  itemClickHook();
}

handleClickDropDown you pass same way you are planning to pass setOpen. itemClickHook is the function that comes from the top-level component, passed in onClick prop. In this approach item handles the click internally, notifies its parent and then continues upstream.

@nsoubelet I see, I better understand what you meant now. I've seen debates about this, it is just a matter of preference whether to pass setState directly or wrap it in a function. Neither is a better practice than the other. IMO the latter is great for more complex structures but the former is totally fine for simple values like a boolean. Either way, I ended up not following these approaches in my PR.

Some reference: https://github.com/fraserxu/react-dropdown/blob/master/index.js

@rluders Seems like this reference has a very different structure than our implementation. Thus, it isn't really transferable to this. I did submit a PR, please do check it out and let me know your thoughts about it.

[rluders]

I'll check the PR later. Or if someone else is interested in testing it and reviewing it I'll also accept it.

I must say that I do prefer it done than perfect. We can rework the component if we felt that it is too complicated or if it doesn't fully match our expectations.

I guess that our main goal here is to relieve the existing pain and let users close the Dropdown by:

• Clicking outside the dropdown;
• Selecting an Item;
• Pressing ESC;

What do you folks think?

[ShaneMaglangit]

Sounds great to me, specially the third option that improves accessibility.

[rluders]

Thanks @ShaneMaglangit for the hard work on this one.