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][Collapsible][Tabs][…] Consider adding an option to control how Content is mounted/unmounted over time/activation cycle #1155

Open
Kilian opened this issue Feb 14, 2022 · 24 comments
Labels
Difficulty: Medium This issue is quite difficult Package: react/accordion Package: react/collapsible Package: react/tabs Resolution: Needs Investigation This issue needs more investigation Type: Enhancement Small enhancement to existing primitive/feature
Milestone

Comments

@Kilian
Copy link

Kilian commented Feb 14, 2022

Feature request

Overview

For the tabs I would like an option to keep the inactive tab contents around in the DOM. For components with a heavy mounting cost, or components with internal state, the current solution is suboptimal. Instead of unmounting, tabs could be set to aria-hidden.

Examples in other libraries

Reach works like this: https://reach.tech/tabs/

Who does this impact? Who is this for?

Anyone using the tabs for components with internal state.

Additional context

n/a


Edit (April 5th 2022): People tend to ask for one or the other, so the gist is for us to see if we can provide an option to cater for both.

@jjenzz
Copy link
Contributor

jjenzz commented Feb 14, 2022

We had a previous discussion about all this for reference #855

Notably a "lazy" state which could be useful where the tab contents start unmounted but remain mounted when opened and then closed #855 (reply in thread)

@benoitgrelard benoitgrelard changed the title [tabs] consider keeping contents around in DOM [Tabs] consider keeping contents around in DOM Apr 5, 2022
@benoitgrelard benoitgrelard added Type: Enhancement Small enhancement to existing primitive/feature Resolution: Needs Investigation This issue needs more investigation Difficulty: Medium This issue is quite difficult Resolution: Backlog Package: react/tabs labels Apr 5, 2022
@benoitgrelard benoitgrelard changed the title [Tabs] consider keeping contents around in DOM [Tabs] Consider adding an option to control how Content is mounted/unmounted over time/activation cycle Apr 5, 2022
@salvadorgonmo
Copy link

salvadorgonmo commented Jul 9, 2023

Hi, just looping back on this. I saw you mentioned about the previous discussion as reference, but is there a conclusion over this request? Seems that both behavior are useful, so I can think of this as a prop like hasDestructiveBehavior being acting as flag, and then if the flag is enabled then the elements are going to be unmounted from the DOM, if not enabled simply hiding them but not removing them from the DOM.

Asking about this cause we are using this tabs and we have some text edit boxes implemented on our site that are placed over the tabs content that needs to retain its state if the other tab content is active, like when you are writing a big amount of text and it gets lost because of changing the tab. Happy to contribute to add this change if needed :)

Thanks in advance!

@aboveyunhai
Copy link

aboveyunhai commented Sep 8, 2023

Based on @jjenzz 's solution from #855 (reply in thread) ,
You can keep tab content mounted and visually hidden by

  const [tab, setActiveTab] = useState("0");
  <Accordion value={tab} onValueChange={setActiveTab}>
    // ...
     <Accordion.Content forceMount={true} hidden={activeTab !== tab} />
   // ...
  </Accordion>

The problem is that because now the visibility is controlled by hidden={activeTab !== tab}, we lost the animation of collapsing, From a user's perspective, this will be simply viewed as buggy or laggy. So we still end up lifting the heavy computation above the Accordion component.
I'm actually not sure how to resolve this problem.
Apologize for the tagging, @jjenzz is there anyway we can utilize the current tool to enable the collapse animation to further expand your solution as a workaround in this case?

@rubby-c
Copy link

rubby-c commented Apr 2, 2024

For those that use forceMount, you also have to add an onPointerLeave on the NavigationMenuPrimitive.Viewport primitive. This way the content won't close when your mouse goes out of it.

@sebi75
Copy link

sebi75 commented Apr 4, 2024

For those that use forceMount, you also have to add an onPointerLeave on the NavigationMenuPrimitive.Viewport primitive. This way the content won't close when your mouse goes out of it.

I personally don't have this issue. Animations would be nice though

@ajayvignesh01
Copy link

You could also use the state data attribute as the conditional. I personally find this a bit cleaner than having useState.

<Tabs.Content
  value='tab-1'
  forceMount
  className='data-[state=inactive]:hidden'
>
  {children}
</Tabs.Content>

@Loque-
Copy link

Loque- commented Apr 17, 2024

It would be great to have an option for preserving content on Accordion Content hide

You could also use the state data attribute as the conditional. I personally find this a bit cleaner than having useState.

<Tabs.Content
  value='tab-1'
  forceMount
  className='data-[state=inactive]:hidden'
>
  {children}
</Tabs.Content>

How do you use this? Super new to Radix, and would also like to preserve content when the accordion is collapsed without having to useState. Thank you!

@kenny019
Copy link

kenny019 commented Apr 21, 2024

It would be great to have an option for preserving content on Accordion Content hide

You could also use the state data attribute as the conditional. I personally find this a bit cleaner than having useState.

<Tabs.Content
  value='tab-1'
  forceMount
  className='data-[state=inactive]:hidden'
>
  {children}
</Tabs.Content>

How do you use this? Super new to Radix, and would also like to preserve content when the accordion is collapsed without having to useState. Thank you!

For Accordion in tailwind it should be data-[state=closed]:hidden instead of inactive for it to work.

@tim-soft
Copy link

tim-soft commented May 31, 2024

How can this work with Navigation Menu (and also maintain the animations)?
Having the content rendered when closed is really important for websites where SEO is a concern

Lets say I have a basic menu with two Content components

<NavigationMenuPrimitive.List>
    <NavigationMenuPrimitive.Item>
        <NavigationMenuPrimitive.Trigger />
        <NavigationMenuPrimitive.Content forceMount className="data-[state=closed]:hidden">
            {/* Menu 1 */}
        </NavigationMenuPrimitive.Content>
    </NavigationMenuPrimitive.Item>

    <NavigationMenuPrimitive.Item>
        <NavigationMenuPrimitive.Trigger />
        <NavigationMenuPrimitive.Content forceMount className="data-[state=closed]:hidden">
            {/* Menu 2 */}
        </NavigationMenuPrimitive.Content>
    </NavigationMenuPrimitive.Item>
</NavigationMenuPrimitive.List>

The first problem is both menus are visible in the DOM when the menu is closed
Adding data-[state=closed]:hidden as a class hides the content when all menus are closed, but are all menus visible when any menu is open

Edit: for anyone looking for a cheap hack so links are in the page during SSR... here's a cheap hack, I'm not saying it's great

Force mount the content and keep it hidden until the user interacts w/ the menu for the first time, then revert to default behavior (forceMount off, remove hidden style)
It makes sure links/text are crawlable by google bot but looks "normal" when human beings interact w/ the menus

const [forceMount, setForceMount] = React.useState(true);

return (
<NavigationMenuPrimitive.Root  onValueChange={() => setForceMount(false)}>
    <NavigationMenuPrimitive.List>
        <NavigationMenuPrimitive.Item>
            <NavigationMenuPrimitive.Trigger />
            <NavigationMenuPrimitive.Content  {...(forceMount && { forceMount })} className={cn(forceMount && "hidden")}>
                {/* Menu 1 */}
            </NavigationMenuPrimitive.Content>
        </NavigationMenuPrimitive.Item>

        <NavigationMenuPrimitive.Item>
            <NavigationMenuPrimitive.Trigger />
            <NavigationMenuPrimitive.Content  {...(forceMount && { forceMount })} className={cn(forceMount && "hidden")}>
                {/* Menu 2 */}
            </NavigationMenuPrimitive.Content>
        </NavigationMenuPrimitive.Item>
    </NavigationMenuPrimitive.List>
</NavigationMenuPrimitive.Root>
);

@seans84
Copy link

seans84 commented Jun 4, 2024

Simple solution, don't use React.

@lucasfontesgaspareto
Copy link

You could also use the state data attribute as the conditional. I personally find this a bit cleaner than having useState.

<Tabs.Content
  value='tab-1'
  forceMount
  className='data-[state=inactive]:hidden'
>
  {children}
</Tabs.Content>

thats why i choose react

@divinsmathew
Copy link

With forceMount and some tweaks in CSS, I was able to do this without sacrificing animations.

Radix already calculates content height and stores it in --radix-collapsible-content-height for us. But when accordion is open, this value is lost. So the idea is to preserve the value so that the Content div will always have a starting and ending height to transition.

.accordion--content {
   height: 0;
   &[data-state="open"] {
      height: auto;
   }
}

We need to store the expanded height of the content to a React state variable.

useEffect(() => {
  if (!ref.current) {
    return;
  }

  const currentHeight = ref.current.clientHeight;
  const height = Math.max(collapsedHeight, currentHeight);

  setCollapsedHeight(height);
}, [collapsedHeight, ref]);

Now, we can restore the lost --radix-collapsible-content-height in JS.

<RadixAccordion.Content
   className={ACCORDION_CLASSES.content}
   forceMount
   style={{
       "--radix-collapsible-content-height": `${collapsibleHeight}px`,
   }}
>
   {content}
</RadixAccordion.Content>

@fernandezremi
Copy link

fernandezremi commented Jul 10, 2024

const MegaMenu = () => {
  const [initialMount, setInitialMount] = useState<true | undefined>(true);

  useEffect(() => {
    setInitialMount(undefined);
  }, []);

  return (
    <NavigationMenu>
      <NavigationMenuList>
        <NavigationMenuItem className="[&>div]:hidden">
          <NavigationMenuTrigger>Getting started</NavigationMenuTrigger>
          <NavigationMenuContent forceMount={initialMount}>
             {/* Menu 1 */}
          </NavigationMenuContent>
        </NavigationMenuItem>
        <NavigationMenuItem className="[&>div]:hidden">
          <NavigationMenuTrigger>Components</NavigationMenuTrigger>
          <NavigationMenuContent forceMount={initialMount}>
              {/* Menu 2 */}
          </NavigationMenuContent>
        </NavigationMenuItem>
      </NavigationMenuList>
    </NavigationMenu>
  );
};

A possible workaround is to force the component to mount on first render and remove the property after the page loads.

On my project this practice works and the content of the two menus is well rendered in the first html !

className="[&>div]:hidden" on NavigationMenuItem property prevents mounted components from being displayed on first render.

If we only use the CSS class and force the menus to be mounted at any time, the content is displayed in the first HTML rendering but all the menus opens when hovering over a single menu

Tell me if you have another idea

@Eylen
Copy link

Eylen commented Jul 29, 2024

Is there any option to have this functionality for Dialog?

We're currently using Vaul which is a wrapper over Radix Dialog and we would like to have this functionality to avoid recreating the DOM each time the drawer is open

@mugavri
Copy link

mugavri commented Sep 10, 2024

@Kilian

Looking for an answer to this question too

@fabiancgc12
Copy link

I found a way to do it with pure css for the accordions when forceMount is true, I'm using Shadcn but i think the code should also work for normal radix

.accordion {
  display: grid;
  grid-template-rows: 0fr;
  overflow: hidden;
  transition: grid-template-rows 200ms ease !important;
  animation: unset;
}

.accordion[data-state="open"] {
  grid-template-rows: 1fr;
}

.accordion > div {
  min-height: 0px;
}

demo

explanation:

  • basically, we need to animate the grid rows
  • In it's closed state it's going to be 0fr but when opened it will be 1fr witch happens to be the size of our content
  • overflow hidden so the mounted content is not being shown
  • we need to wrap all of our content within a div or some kind of wrapper element, to that wrapper we add min-height: 0px, without it the element would be displayed, so you need the wrapper
  • lastly, the transition needs the !important flag to override some behavior of the default accordion
  • remove the animations if you are using shadcn

@appdeveloper-gf
Copy link

I found a way to do it with pure css for the accordions when forceMount is true, I'm using Shadcn but i think the code should also work for normal radix

.accordion {
  display: grid;
  grid-template-rows: 0fr;
  overflow: hidden;
  transition: grid-template-rows 200ms ease !important;
  animation: unset;
}

.accordion[data-state="open"] {
  grid-template-rows: 1fr;
}

.accordion > div {
  min-height: 0px;
}

demo

explanation:

  • basically, we need to animate the grid rows
  • In it's closed state it's going to be 0fr but when opened it will be 1fr witch happens to be the size of our content
  • overflow hidden so the mounted content is not being shown
  • we need to wrap all of our content within a div or some kind of wrapper element, to that wrapper we add min-height: 0px, without it the element would be displayed, so you need the wrapper
  • lastly, the transition needs the !important flag to override some behavior of the default accordion
  • remove the animations if you are using shadcn

Are you putting this on the entire accordion ,or accordionItem, or accordionContent? I'm having the same issues, and I can't use a useEffect /listener as others have suggested, since i need this component to be server rendered in next.

@appdeveloper-gf
Copy link

I found a way to do it with pure css for the accordions when forceMount is true, I'm using Shadcn but i think the code should also work for normal radix

.accordion {
  display: grid;
  grid-template-rows: 0fr;
  overflow: hidden;
  transition: grid-template-rows 200ms ease !important;
  animation: unset;
}

.accordion[data-state="open"] {
  grid-template-rows: 1fr;
}

.accordion > div {
  min-height: 0px;
}

demo

explanation:

  • basically, we need to animate the grid rows
  • In it's closed state it's going to be 0fr but when opened it will be 1fr witch happens to be the size of our content
  • overflow hidden so the mounted content is not being shown
  • we need to wrap all of our content within a div or some kind of wrapper element, to that wrapper we add min-height: 0px, without it the element would be displayed, so you need the wrapper
  • lastly, the transition needs the !important flag to override some behavior of the default accordion
  • remove the animations if you are using shadcn

your demo link is broken and i would love to see an example :) ive been hard stuck on this for a while

@mugavri
Copy link

mugavri commented Oct 28, 2024

any updates?

@Lexachoc
Copy link

Lexachoc commented Nov 2, 2024

Any updates on how to keep the Content mounted all the time? I have a table in a Dialog that takes some time to load. And since the Dialog.Content is unmounted when closed, it takes a while to restore the states later, which is not so efficient.

For Tabs, the forceMount is fine, as it does not affect the pointer event, scroll bar, focus, etc. However, it is different for Dialog.

When using forceMount on Dialog.Content, it breaks the ESC escape behavior for DropdownMenu (but Select is is no problem, as well as Popover with modal={true}) to close it.

A workaround is to use onEscapeKeyDown on Dialog.Content and manually close the DropdownMenu.

onEscapeKeyDown={(e) => {
    setOpen(false)
}}

As answered in #2476: (and also being closed)

This is expected because forceMount is not supposed to be used to keep the content mounted at all time, it's there only to support JS-based exit animations, so the content should still be unmounted, otherwise things like focus trapping are still active (hence what you see).

I will close the issue because this is the intended behaviour based on what we currently offer.

Note that we have been discussing other options to keep content mounted for other reasons such as SEO, etc over in #1155

The oldest post I found is probably this one from 2021 #998, but it was closed again.
So there is still no clear answer on how to keep the Dialog. Content available at all times.

@Sephster
Copy link

This is causing issues for my application as well. We were hoping to use the accordion to house search filters but because the contents are unmounted, any form elements within the accordion disappear and aren't applied on a search. It would be great if there was a solution to this so just wanted to add my own scenario and how this is currently causing issues. Happy to answer and further questions that might pop up.

@JakeSchroeder
Copy link

In case this helps someone, I managed a pretty gross hack but it works for my usecase: I wanted forceMount on all the time so as to not re fetch a bunch of images and state etc each time a nav item was triggered. Its hacky but works:

  const [currentNavItem, setCurrentNavItem] = useState('');
  
  <NavigationMenu
        onValueChange={(e) => {
          setCurrentNavItem(e);
        }}
      >
      //... other NavMenuStuff
      <NavigationMenuContent
          forceMount
          className={cn(
            'absolute left-0 right-0 top-0 opacity-0 data-[state=closed]:hidden',
            currentNavItem === 'navItem1' && 'block opacity-100'
          )}
        >
  <NavigationMenuContent
          forceMount
          className={cn(
            'absolute left-0 right-0 top-0 opacity-0 data-[state=closed]:hidden',
            currentNavItem === 'navItem2' && 'block opacity-100'
          )}
        >
//....etc...

Note: the key in my case was the opacity-0 by default because they are all stacked on each other. You could also use visibility hidden / visible.

@jamestrenda
Copy link

My design dictates that I use Accordion on smaller screens and a 3-column grid on larger screens. I thought I could just manipulate the CSS for larger screens, but then I discovered that the content isn't mounted and thus there is nothing to "show" until a trigger mounts the content. This is also bad for SEO, no?

By using forceMount, as some have suggested, I am able to implement the 3-column grid layout on larger screens and that content is now visible in the page source. However, this method introduces problems, as others have again suggested, when sizing back down to smaller screens. In its current state, this seems like a primitive that is most useful for client side apps even though the docs clearly state that they can all be SSR'd.

@jamestrenda
Copy link

My design dictates that I use Accordion on smaller screens and a 3-column grid on larger screens. I thought I could just manipulate the CSS for larger screens, but then I discovered that the content isn't mounted and thus there is nothing to "show" until a trigger mounts the content. This is also bad for SEO, no?

By using forceMount, as some have suggested, I am able to implement the 3-column grid layout on larger screens and that content is now visible in the page source. However, this method introduces problems, as others have again suggested, when sizing back down to smaller screens. In its current state, this seems like a primitive that is most useful for client side apps even though the docs clearly state that they can all be SSR'd.

OK, so I was able to achieve my design by using the forceMount prop and some styling (using tailwind) like so:

(Ignore my primitive aliases...I'm using Astro)

<BaseAccordion type="single" className="w-full lg:grid lg:grid-cols-3 lg:gap-8">
      {data.map((item, i) => {
        return (
          <AccordionItem
            key={i}
            value={`item-${i.toFixed()}`}
            className="[&[data-state=closed]>div]:h-0 lg:[&[data-state=closed]>div]:h-auto"
          >
            <AccordionTrigger className="text-left [&>svg]:h-6 [&>svg]:w-6 md:[&>svg]:h-8 md:[&>svg]:w-8 lg:[&>svg]:hidden lg:cursor-text select-text lg:pointer-events-none">
              {item.title}
            </AccordionTrigger>
            <AccordionContent forceMount>
              <p className="md:text-lg">{item.description}</p>
            </AccordionContent>
          </AccordionItem>
        );
      })}
</BaseAccordion>

¯\(ツ)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Difficulty: Medium This issue is quite difficult Package: react/accordion Package: react/collapsible Package: react/tabs Resolution: Needs Investigation This issue needs more investigation Type: Enhancement Small enhancement to existing primitive/feature
Projects
None yet
Development

No branches or pull requests