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

[Feature Request] NextUI V2, When user opens dropdown/popup, close the another dropdown/popup #1287

Closed
alibabayev0 opened this issue Aug 5, 2023 · 6 comments · Fixed by #2854
Assignees
Labels
🔦 Type: Feature New feature or request

Comments

@alibabayev0
Copy link

Is your feature request related to a problem? Please describe.

Yes, when a user opens a modal or popup while another modal/popup is already open, it can create confusion or lead to visual clutter. Multiple overlays can be distracting, and the user might not know which modal to focus on. It can also lead to potential UI glitches, especially when z-index values or other styles conflict between multiple modals.

Describe the solution you'd like

I'd like the nextui dropdown/popupover components to have built-in functionality that ensures only one dropdown or popup is visible at a given time for specific key or same labeled.

Can be resolved with: Context || DOM Events etc

Describe alternatives you've considered

.

Screenshots or Videos

No response

@alibabayev0 alibabayev0 added the 🔦 Type: Feature New feature or request label Aug 5, 2023
@alibabayev0 alibabayev0 changed the title [Feature Request] When user opens dropdown/popup, close the another dropdown/popup [Feature Request] NextUI V2, When user opens dropdown/popup, close the another dropdown/popup Aug 5, 2023
@jrgarciadev
Copy link
Member

Hey @alibabayev0 could you send a repo/codesandbox/stackblitz ?, I couldn't replicate this issue, on the other hand, we rely on @react-aria overlays implementations which already has a context to control which modals/popovers are visible https://github.com/adobe/react-spectrum/blob/main/packages/%40react-aria/overlays/src/useOverlay.ts#L79-L86

@alibabayev0
Copy link
Author

Hey, @jrgarciadev, check this out: https://codesandbox.io/p/sandbox/eager-fog-l9smkq. In the simplified version, I noticed we need to place the button within the Dropdown trigger, or it won't work correctly. I believe you've already implemented this, but just in case, it might be good to handle 'div' and other view types as well.

@ynn1k
Copy link
Contributor

ynn1k commented Aug 18, 2023

I found shouldCloseOnBlur in the docs, which seems not to work - but doesnt this describe your wanted behaviour? @alibabayev0

@SellersEvan
Copy link

I also tried out using shouldCloseOnBlur and it does not work as I originally expected. However, after reviewing I remembered that focus has to mainly do with input elements. You can see an example from Mozilla here.

I did look into the code and it looks like their using Adobe's React Spectrum, specifically the useOverlay. In the React Spectrum's issue page a fix to this issue has been discussed here. But it appears there is no solution currently.

But I currently do not see away to make the click away from a dropdown also be an event to activate a new dropdown. If anyone knows a fix let me know.

@alibabayev0 I was unable to replicate the effect where you when you click away, it activates the other popup. How were you able to do this?

@SellersEvan
Copy link

Here is a solution I developed. Honestly, not sure why it works but it does.

import { Dropdown as NativeDropdown, DropdownProps } from "@nextui-org/react";
import React from "react";
import { useState } from "react";

export function Dropdown(props:DropdownProps) {
    const [visible, setVisible] = useState(false);
    return (
        <NativeDropdown
            {...props}
            isOpen={visible}
            onOpenChange={(isOpen) => setVisible(isOpen)}
            shouldCloseOnInteractOutside={() => {
                setVisible(false);
                return false;
            }}
        />
    );
}

@buchananwill
Copy link

buchananwill commented May 17, 2024

I agree that this part of the API is extremely counter-intuitive.

This is a more generic hook version I use to fix it:

const fixProps = usePopoverFix();

 <Popover
            {...fixProps}
            // other props and children etc.

defined as:

import { useEffect, useMemo, useState } from "react";

export function usePopoverFix() {
  const [popoverVisible, setPopoverVisible] = useState(false);

  const shouldCloseOnInteractOutside = useMemo(() => {
    return () => {
      setPopoverVisible(false);
      return false;
    };
  }, [setPopoverVisible]);

  useEffect(() => {
    if (!popoverVisible) {
      return;
    }

    function keyDownHandler(e: KeyboardEvent) {
      if (popoverVisible && e.key === "Escape") {
        e.preventDefault();
        setPopoverVisible(false);
      }
    }

    document.addEventListener("keydown", keyDownHandler);

    return () => {
      document.removeEventListener("keydown", keyDownHandler);
    };
  }, [popoverVisible, setPopoverVisible]);

  return {
    isOpen: popoverVisible,
    onOpenChange: setPopoverVisible,
    isKeyboardDismissDisabled: false,
    shouldCloseOnInteractOutside,
  };
}

EDITED: I added an escape key listener, because that doesn't seem to be working correctly either.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔦 Type: Feature New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants