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

Accordion mounting is a big problem #673

Open
Andynopol opened this issue Jan 21, 2025 · 0 comments
Open

Accordion mounting is a big problem #673

Andynopol opened this issue Jan 21, 2025 · 0 comments

Comments

@Andynopol
Copy link

Hello guys!

Before I describe the issue, I want to address the fact that I've already searched for the answer on this repo and other forums and I know there have been some back and forth with this, but I hope to make a good point here.

How I got here:

I've just got a contract with a customer of mine and decided to give @radix-ui a spin for the app that he needs, and in this project, I got myself building a menu. This menu can be collapsible and dropdown. So as you can see, a great combo for this would be the Accordion and DropdownMenu components.

In this component I started making the expaneded only first(so the Accordion component). This menu can have items that can be menus on their own. Kind of like the antd or rc-menu. But after some time, I've seen that the components inside the accordion are not rendered until I open it. And that sounds good if you are planning to have a component that is not in view and it doesn't need to be rendered, but not when you want to develop a component with a persistent config. Especially one that re-mounts every component an re-runs the mounting life-cycle EVERY SINGLE TIME you click that item.

Research:

So after some poking around, I found the forceMount prop on the AccordionContent and thought that I've fixed the issue...nope... The content is permanently rendered and it never hides. I've come to find out Accordion depends on Collapsible component, the one responsible for lack of component mounting.

After some light reading on the topic on this repo, I found out that this is an intended behavior and if we want to have the same experience as we have without the forceMount flag, then we should use other react animation libraries...

Why is this wrong:

The main big reason I picked this library was the fact that is not opinionated. It's basically provides a "contoured canvas" that you can modify to your liking. Well this intended feature is doing the opposite. This component tells me: "If you are not using an animation library, you can't use this." or "You can only use this to render stateless components."

You took my choice away. And it's not just my use case. It's a choice that every single one of us has to consciously make base on whether it suits our needs or not.

I know that some people found ways to kind of fix this issue, and I found one too, I will post mine at the end of this text, but they are either:

  1. Junky - implies DOM manipulation outside react
  2. Render nightmare(like mine is)

Here is my solution:

Menu.tsx

export const Menu: React.FC<MenuProps> = ({
  minLevel = 3,
  minimal = false,
  children,
  rootPath,
  ...props
}: MenuProps) => {
  const [openLevels, setOpenLevels] = useState<string[]>([]);
  const addOpenLevel = useCallback(
    (lvl: string) => setOpenLevels(prev => [...prev, lvl]),
    [openLevels, setOpenLevels],
  );

  const removeOpenLevel = useCallback(
    (lvl: string) => setOpenLevels(prev => prev.filter(level => level !== lvl)),
    [openLevels, setOpenLevels],
  );
  return (
    <MenuContext.Provider
      value={{ minLevel, minimal, openLevels, addOpenLevel, removeOpenLevel }}
    >
      <SubMenu
        {...props}
        className="babylon-ml-0 babylon-border-b"
        level={0}
        isRoot
        path={rootPath || 'root'}
      >
        {children}
      </SubMenu>
    </MenuContext.Provider>
  );
};

MenuItem.tsx

export const MenuItem: React.FC<MenuItemProps> = ({
  className,
  index,
  level = 0,
  items,
  title,
  rootClassName,
  children,
  selectable = false,
  onClick,
  path,
  selected: propSelected,
  disabled,
  ...props
}: MenuItemProps) => {
  const { minLevel, minimal, openLevels, addOpenLevel, removeOpenLevel } =
    useMenu();
  const isCollapsible = useMemo(
    () =>
      minimal ? false : (level || 0) < minLevel && items && items.length !== 0,
    [minimal, minLevel, items],
  );

  const defaultValue = useMemo(
    () => (openLevels.includes(index.toString()) ? `${index}` : undefined),
    [open, isCollapsible],
  );



  const handleAccordionValueChange = useCallback(
    (value: string) => {
      if (value === index) {
        return addOpenLevel(value);
      }
      removeOpenLevel(`${index}`);
    },
    [addOpenLevel, removeOpenLevel],
  );

  return (
    <Accordion
      type="single"
      className={cn('babylon-w-full', rootClassName)}
      defaultValue={defaultValue}
      collapsible={isCollapsible}
      onValueChange={handleAccordionValueChange}
    >
      {...}
    </Accordion>
  );
};

Short explanation: Basically, to preserve the state of each SubMenu, I am saving the accordion values in the openLevels array.

With this issue open, I really hope that I make a string point in the necessity of a choice in this matter and you might consider adding it to Accordion - Probably means not using Collapsible.

If you made it this far, thank you and I am waiting for your insight.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant