Search code examples
reactjstypescriptnext.jsreact-hookstailwind-css

React nested Accordion Parent not updating height


I am trying to build the mobile version of my homepage, My nested accordion "Projects" seems to have a bug where it doesn't show the correct height of the bottom projects section on the first open.

To open that, you first click on the projects text, then it lists out the projects and then you click on the project toggle the Project Card.

Screenshot of visual bug

(Updated) I believe this is happening because my parent Accordion is not re-updating its height when the child Accordion opens.

Do you know a good way of doing this? Or if needed should I restructure my components in a way that makes this possible? The difficulty is the fact that Accordion accepts children, and I reuse Accordion inside it so it's rather confusing. I know I can potentially use a callback function to trigger the parent but not quite sure how to approach this.

Homepage.tsx


import { Accordion } from "@/components/atoms/Accordion"
import { AccordionGroup } from "@/components/atoms/AccordionGroup"
import { AccordionSlideOut } from "@/components/atoms/AccordionSlideOut"
import { Blog } from "@/components/compositions/Blog"
import { Contact } from "@/components/compositions/Contact"
import { Portfolio } from "@/components/compositions/Portfolio"
import { PuyanWei } from "@/components/compositions/PuyanWei"
import { Resumé } from "@/components/compositions/Resumé"
import { Socials } from "@/components/compositions/Socials"
import { Component } from "@/shared/types"

interface HomepageProps extends Component {}

export function Homepage({ className = "", testId = "homepage" }: HomepageProps) {
  return (
    <main className={`grid grid-cols-12 pt-24 ${className}`} data-testid={testId}>
      <section className="col-span-10 col-start-2">
        <AccordionGroup>
          <Accordion title="Puyan Wei">
            <PuyanWei />
          </Accordion>
          <Accordion className="lg:hidden" title="Portfolio">
            <Portfolio />
          </Accordion>
          <AccordionSlideOut className="hidden lg:flex" title="Portfolio">
            <Portfolio />
          </AccordionSlideOut>
          <Accordion title="Resumé">
            <Resumé />
          </Accordion>
          <Accordion title="Contact">
            <Contact />
          </Accordion>
          <Accordion title="Blog">
            <Blog />
          </Accordion>
          <Accordion title="Socials">
            <Socials />
          </Accordion>
        </AccordionGroup>
      </section>
    </main>
  )
}

Portfolio.tsx

import { Accordion } from "@/components/atoms/Accordion"
import { AccordionGroup } from "@/components/atoms/AccordionGroup"
import { ProjectCard } from "@/components/molecules/ProjectCard"
import { projects } from "@/shared/consts"
import { Component } from "@/shared/types"

interface PortfolioProps extends Component {}

export function Portfolio({ className = "", testId = "portfolio" }: PortfolioProps) {
  return (
    <AccordionGroup className={`overflow-hidden ${className}`} testId={testId}>
      {projects.map((project, index) => (
        <Accordion title={project.title} key={`${index}-${project}`} headingSize="h2">
          <ProjectCard project={project} />
        </Accordion>
      ))}
    </AccordionGroup>
  )
}

AccordionGroup.tsx - The purpose of AccordionGroup is to only allow one child Accordion to be open at one time. If an Accordion is not in AccordionGroup it can open and close independently.


"use client"

import React, { Children, ReactElement, cloneElement, isValidElement, useState } from "react"
import { AccordionProps } from "@/components/atoms/Accordion"
import { Component } from "@/shared/types"

interface AccordionGroupProps extends Component {
  children: ReactElement<AccordionProps>[]
}

export function AccordionGroup({
  children,
  className = "",
  testId = "accordion-group",
}: AccordionGroupProps) {
  const [activeAccordion, setActiveAccordion] = useState<number | null>(null)

  function handleAccordionToggle(index: number) {
    setActiveAccordion((prevIndex) => (prevIndex === index ? null : index))
  }

  return (
    <div className={className} data-testid={testId}>
      {Children.map(children, (child, index) =>
        isValidElement(child)
          ? cloneElement(child, {
              onClick: () => handleAccordionToggle(index),
              isActive: activeAccordion === index,
              children: child.props.children,
              title: child.props.title,
            })
          : child
      )}
    </div>
  )
}

Accordion.tsx


"use client"
import { Component } from "@/shared/types"
import React, { MutableRefObject, ReactNode, RefObject, useEffect, useRef, useState } from "react"
import { Heading } from "@/components/atoms/Heading"

export interface AccordionProps extends Component {
  title: string
  children: ReactNode
  isActive?: boolean
  onClick?: () => void
  headingSize?: "h1" | "h2"
}

export function Accordion({
  className = "",
  title,
  children,
  isActive,
  onClick,
  headingSize = "h1",
  testId = "Accordion",
}: AccordionProps) {
  const [isOpen, setIsOpen] = useState(false)
  const [height, setHeight] = useState("0px")
  const contentHeight = useRef(null) as MutableRefObject<HTMLElement | null>

  useEffect(() => {
    if (isActive === undefined) return
    isActive ? setHeight(`${contentHeight.current?.scrollHeight}px`) : setHeight("0px")
  }, [isActive])

  function handleToggle() {
    if (!contentHeight?.current) return
    setIsOpen((prevState) => !prevState)
    setHeight(isOpen ? "0px" : `${contentHeight.current.scrollHeight}px`)
    if (onClick) onClick()
  }
  return (
    <div className={`w-full text-lg font-medium text-left focus:outline-none ${className}`}>
      <button onClick={handleToggle} data-testid={testId}>
        <Heading
          className="flex items-center justify-between"
          color={isActive ? "text-blue-200" : "text-white"}
          level={headingSize}
        >
          {title}
        </Heading>
      </button>
      <div
        className={`overflow-hidden transition-max-height duration-250 ease-in-out`}
        ref={contentHeight as RefObject<HTMLDivElement>}
        style={{ maxHeight: height }}
      >
        <div className="pt-2 pb-4">{children}</div>
      </div>
    </div>
  )
}

ProjectCard.tsx


import Image from "next/image"
import { Card } from "@/components/atoms/Card"
import { Children, Component, Project } from "@/shared/types"
import { Subheading } from "@/components/atoms/Subheading"
import { Tag } from "@/components/atoms/Tag"
import { Text } from "@/components/atoms/Text"

interface ProjectCardProps extends Component {
  project: Project
}

export function ProjectCard({
  className = "",
  testId = "project-card",
  project,
}: ProjectCardProps) {
  const {
    title,
    description,
    coverImage: { src, alt, height, width },
    tags,
  } = project
  return (
    <Card className={`flex min-h-[300px] ${className}`} data-testid={testId}>
      <div className="w-1/2">
        <CoverImage className="relative w-full h-full mb-4 -mx-6-mt-6">
          <Image
            className="absolute inset-0 object-cover object-center w-full h-full rounded-l-md"
            src={src}
            alt={alt}
            width={parseInt(width)}
            height={parseInt(height)}
            loading="eager"
          />
        </CoverImage>
      </div>
      <div className="w-1/2 p-4 px-8 text-left">
        <Subheading className="text-3xl font-bold" color="text-black">
          {title}
        </Subheading>
        <Tags className="flex flex-wrap pb-2">
          {tags.map((tag, index) => (
            <Tag className="mt-2 mr-2" key={`${index}-${tag}`} text={tag} />
          ))}
        </Tags>
        <Text color="text-black" className="text-sm">
          {description}
        </Text>
      </div>
    </Card>
  )
}

function CoverImage({ children, className }: Children) {
  return <div className={className}>{children}</div>
}
function Tags({ children, className }: Children) {
  return <div className={className}>{children}</div>
}

Any help would be appreciated, thanks!


Solution

  • You know, t is a bit challenging with this implementation here, because when you want to update the height of the grandparent Accordion from its child one, you can't really know from there, which corresponding grandparent Accordion you want to update unless you pass props to the grandparent Accordion and also pass props to the in-between component (Portfolio for example, i.e child Accordion's parent) so it can propagate them to its child Accordion.
    by doing that, we can make the grandparent and child Accordions communicate somehow.
    Maybe this is not the best solution you can find, but sadly, I cannot think of a better one.


    So to recap: the idea is to create a state in the top level to hold heights that refer to each parent Accordion, so it is an array where the length is set "manually" which makes it somehow ugly, but if you have to use an array of data to display your components dynamically, then it is not a problem we will discover that later, also we will see the limitation of the workaround.


    Workaround:

    Now we'll go with the simplest and most straightforward fix that works for what is included in the question.

    As mentioned, first we create the state in the HomePage component:

    const [heights, setHeights] = useState(Array(7).fill("0px")); // you have 7 parent Accordions
    

    After creating the array state at the top level, Now, to each Accordion component, we pass the state setter function setHeights, the index indexx, and also the corresponding height heightParent if it is a parent Accordion

    <AccordionGroup>
      <Accordion title="Puyan Wei" heightParent={heights[0]} setHeights={setHeights} indexx="0">
          <PuyanWei setHeights={setHeights} indexx="0" />
      </Accordion>
      <Accordion title="Portfolio" heightParent={heights[1]} setHeights={setHeights} indexx="1">
          <Portfolio setHeights={setHeights} indexx="1" />
      </Accordion>
      //...
      <Accordion title="Socials" heightParent={heights[6]} setHeights={setHeights} indexx="6">
          <Socials setHeights={setHeights} indexx="6" />
      </Accordion> 
    </AccordionGroup>
    

    Note: indexx props passed to parent and indexx props passed to the in-between component (Portfolio) they should have the same value indicating the corresponding index, this is in fact, the key of the solution.
    it is named 'indexx' with two 'x' to avoid conflict later.

    Then, From the in-between component you pass those received props to the child Accordion:

    export function Portfolio({
      className = "",
      testId = "portfolio",
      indexx,
      setHeight,
    }: PortfolioProps) {
      // update props interface
      return (
        <AccordionGroup className={`overflow-hidden ${className}`} testId={testId}>
          {projects.map((project, index) => (
            <Accordion
              title={project.title}
              key={`${index}-${project}`}
              headingSize="h2"
              indexx={indexx}
              setHeight={setHeight}
            >
              <ProjectCard project={project} />
            </Accordion>
          ))}
        </AccordionGroup>
      );
    }
    

    Now from your child Accordion component, you are able to update the height of the corresponding Accordion parent in the HomePage state, getting advantage of the passed indexx props, so when we update the child height we also update the parent height

    function handleToggle() {
      if (!contentHeight?.current) return;
      setIsOpen((prevState) => !prevState);
      let newHeight = isOpen ? "0px" : `${contentHeight.current.scrollHeight}px`;
      if (!heightParent) { // there is no need to update the state when it is a parent Accordion
        setHeight(newHeight);
      }
      setHeights((prev) =>
        prev.map((h, index) => (index == indexx ? newHeight : h))
      );
    }
    

    Finally, when you specify the height for one Accordion you can check if it receives heightParent as props so we know it is a parent one, by this, we let the Accordion component uses heightParent as maxHeight and not its own state height if it is a parent one, that's the reason behind ignoring updating the height state when it is a parent Accordion that get opened, therefore, we have to change the way maxHeight property is set, now it should depend on the nature of the Accordion:

    style={{ maxHeight: `${heightParent? heightParent : height }` }}
    

    If you want to make the parent Accordion just uses its state height as maxHeight and just keep your code as it is so it makes more sense

    style={{ maxHeight: height }}
    

    you can still do this, by adding a useEffect in the Accordion component and making sure its runs only when the received heightParent props is updated and also defined, we do that to be sure the code there is only running when it is a parent accordion that should update its height state:

    useEffect(()=>{
     if(heightParent){
       setHeight(heightParent)
     }
    },[heightParent])
    

    As said, this makes more sense and it is most beautiful too, but I still prefer the first one since it is more straightforward and also saves one additional render.


    Working with data dynamically:

    If we have data stored in an array and we want to display our components based on that we do this instead:

    const data = [...]
    const [heights, setHeights] = useState(data.map(_ => "0px")); 
    //...
    <section className="col-span-10 col-start-2">
     <AccordionGroup>
      {data.map(element, index => {
         <Accordion key={index} title={element.title} heightParent={heights[index]} setHeights={setHeights} indexx={index} >
           {element.for === "portfolio" ? <Portfolio setHeights={setHeights} indexx={index} /> : <PuyanWei setHeights={setHeights} indexx={index} /> }  // just an example
         </Accordion>
      })
      }
     </AccordionGroup>
    </section>
    

    You can notice here we have to specify a key in the parent accordion so we can use it instead of indexx but you know the key property is special and we don't want to mess with it anyways, hope you get the idea


    Limitation:

    It is obvious this solution works only one level, so if the child Accordion gets itself a child Accordion, then you have to wrap your head around it again, but likely if I understood what you are doing, you will not face that, since with your implementation the child Accordion is supposed to show items, but who knows maybe one day you'll need to make it return another child Accordion, and that's why I consider my suggestion a workaround rather than an optimal solution.


    Like I said, it might not be the best fix, but honestly, especially with this implementation, I don't think such a multi-level working solution exists, please prove me wrong, I'm following the post.