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

MultiSelect: Dropdown closes on deselect #4523

Closed
ShaneCallananAerlytix opened this issue Jun 20, 2023 · 21 comments · Fixed by #4539
Closed

MultiSelect: Dropdown closes on deselect #4523

ShaneCallananAerlytix opened this issue Jun 20, 2023 · 21 comments · Fixed by #4539
Assignees
Labels
Type: Bug Issue contains a defect related to a specific component.
Milestone

Comments

@ShaneCallananAerlytix
Copy link

ShaneCallananAerlytix commented Jun 20, 2023

Describe the bug

Hi, firstly I'd like to show my appreciation for this project. It's been great!

The bug:
It appears to be inconsistent, but when deselecting items in MultiSelect, the dropdown panel closes sometimes.
This is even happening on the component demo on your website (see video below).
https://youtu.be/cvIG21mjfWc

Perhaps there's a reason for this behavior that I'm not seeing. I notice that clicking "deselect all" always closes the dropdown panel. Perhaps this behaviour is getting triggered incorrectly when deselecting individual items.

I'll keep the video accessible for 1 month. If you need longer, please let me know.

Reproducer

No response

PrimeReact version

9.5.0

React version

17.x

Language

TypeScript

Build / Runtime

Create React App (CRA)

Browser(s)

Ubuntu 22.04.2: Chrome 114.0.5735.133, Firefox 114.0.1, Edge 114.0.1823.43 (all 64-bit)
Windows 10/11: Not yet produced
MacOS 12.6.6: Edge 114.0.1823.51 (x84_64)

Steps to reproduce the behavior

  1. Open MultiSelect dropdown
  2. Randomly select and deselect items

Expected behavior

Items are deselected without the dropdown panel closing

@ShaneCallananAerlytix ShaneCallananAerlytix added the Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible label Jun 20, 2023
@melloware
Copy link
Member

I checked out your video. I am trying to reproduce but so far can't make it happen on the Showcase using Windows 11 and Chrome 114.0.5735.134 (Official Build) (64-bit).

I tried clicking fast and clicking slow. Very strange

@ShaneCallananAerlytix
Copy link
Author

ShaneCallananAerlytix commented Jun 20, 2023

I was using Ubuntu myself. Testing on windows 10 doesn't produce the issue for me.
However I've asked a few colleagues to try reproducing, and getting mixed/inconsistent results across browsers/OS's

@melloware
Copy link
Member

OK can you update the Browsers Section above with exactly what OS + Browser combinations you can make it happen on?

@ShaneCallananAerlytix
Copy link
Author

ShaneCallananAerlytix commented Jun 20, 2023

Updated.
Note that the same behaviour occurs with "deselect all" and is far more consistent

@ShaneCallananAerlytix
Copy link
Author

I should also note that it appears to only happen when clicking on the checkboxes themselves. I haven't been able to produce it by clicking the labels

@melloware
Copy link
Member

My only thought is its this code detecting "outside" clicks to close the panel.

const isOutsideClicked = (event) => {
return targetRef.current && !(targetRef.current.isSameNode(event.target) || targetRef.current.contains(event.target) || (overlayRef.current && overlayRef.current.contains(event.target)));
};

@jerlam06
Copy link

jerlam06 commented Jun 21, 2023

Exactly the same problem. Windows 11, Google Chrome. Clicking checkboxes closes the dropdown randomly. And deselecting all closes the dropdown EVERY time. I have recently enforced primereact for my saas company, but this bug makes the multiselect unusable for prod.
Beside that, the library is awesome, thanks a lot! :)

EDIT: The bug happens exclusively on deselecting, only on checkbox click.

@melloware
Copy link
Member

melloware commented Jun 21, 2023

@jerlam06 i can't make it happen on Windows 11. When I click the deselect ALL or any checkbox it all works over and over and over again in Windows 11 Chrome 114.0.5735.134 (Official Build) (64-bit).

I am testing just with the public showcase PR 9.5.0: https://primereact.org/multiselect/

Edit: Looks like it happens on the Chips example when you select "De-select all" https://primereact.org/multiselect/#chips

@jerlam06
Copy link

jerlam06 commented Jun 21, 2023

@melloware I am glad you could reproduce it on the public showcase. For me I have the issue of the Chips on every other multiselect of the live showcase (Group, Template, Filter...etc). The issue seems to be happening on p-checkbox-box click.

@jerlam06
Copy link

So, I have been investigating on this issue since I need a fix asap. It would seem that the issue is only triggered on .p-checkbox-icon (svg) click, that's why it only happens on unselect.
I came up with a quick and very dirty fix to be able to use the multiselect component normally while we wait for an official fix: I stopped the immediate propagation of click events on the .p-checkbox-icon elements and programmatically triggered the click on their parent elements.

useEffect(() => {
    const observer = new MutationObserver((mutations) => {
      mutations.forEach(({ addedNodes, removedNodes }) => {
        addedNodes.forEach((node) => {
          if (node.nodeType === 1) {
            // Check if it's an HTML element node
            if (node.classList.contains('p-checkbox-icon')) {
              // Perform your logic here
              node.addEventListener('click', stopPropagationEventCallback);
            }
          }
        });
        removedNodes.forEach((node) => {
          if (node.nodeType === 1) {
            // Check if it's an HTML element node
            if (node.classList.contains('p-checkbox-icon')) {
              // Perform your logic here
              node.removeEventListener('click', stopPropagationEventCallback);
            }
          }
        });
      });
    });

    observer.observe(document.body, { childList: true, subtree: true });

    // Cleanup observer on component unmount
    return () => observer.disconnect();
  }, []);

  function stopPropagationEventCallback(e) {
    e.stopImmediatePropagation();
    e.target.parentNode.click();
  }

@melloware
Copy link
Member

Great debugging let me look at that now thatI know the issue

@melloware melloware added Type: Bug Issue contains a defect related to a specific component. and removed Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible labels Jun 22, 2023
@melloware melloware self-assigned this Jun 22, 2023
@melloware melloware added this to the 9.6.0 milestone Jun 22, 2023
@melloware
Copy link
Member

melloware commented Jun 22, 2023

@jerlam06 can you try this...using a passthrough prop to turn off the onClick on the Checkbox icon: pt={{ checkboxContainer: { onClick: null } }}

<MultiSelect
        value={selectedCities}
        onChange={(e) => setSelectedCities(e.value)}
        options={cities}
        optionLabel="name"
        placeholder="Select Cities"
        maxSelectedLabels={3}
        className="w-full md:w-20rem"
        pt={{ checkboxContainer: { onClick: null } }}
      />

Edit: Passthrough might not be available yet until 9.6.0 anyway.

@melloware
Copy link
Member

I submitted the correct fix. It was adding the onClick to the checkbox item when it should not have it should only be on the ListItem parent as you mentioned.

@leoperria
Copy link

@melloware @jerlam06 Thanks a lot for your effort! Really appreciated

@melloware
Copy link
Member

Great teamwork. Just let me know when 9.6.0 comes out that its truly fixed but I know i was able to reproduce the problem. Made this fix and then I could no longer reproduce the problem so I am pretty sure its fixed!

@ShaneCallananAerlytix
Copy link
Author

@melloware @jerlam06 Thanks a lot guys!

@melloware
Copy link
Member

9.6.0 is out if you guys want to give this bug a try?

@jerlam06
Copy link

jerlam06 commented Aug 29, 2023

@melloware Hi there. Sorry I did not react earlier. Sadly, the issue is still there in 9.6.2. I can reproduce it here: https://primereact.org/multiselect/ when I click the select/unselect all. However, I cannot reproduce it on the single items below, which is still buggy on my local environment.

I could temporarily fix it with pt:

pt={{
        checkboxIcon: {
          onClick: (e) => {
            e.stopPropagation();
            e.target.parentNode.click();
          },
        },
        headerCheckbox: {
          onClick: (e) => {
            e.stopPropagation();
            e.target.parentNode.click();
          },
        },
      }}

@MarcHazime
Copy link

Hi there ! I use primereact 10.0.5 and I still have the same issue on my multiselect component.

@jerlam06
Copy link

@MarcHazime It is fixed on the latest version.

@MarcHazime
Copy link

@MarcHazime It is fixed on the latest version.

Amazing thanks a lot dude ;-) happy christmas !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Issue contains a defect related to a specific component.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants