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: Accordion component #189

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions packages/wethegit-components-cli/src/registry-index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,14 @@ const ICON: Registry = {
"https://wethegit.github.io/component-library/?path=/docs/components-icon-readme--overview",
}

const ACCORDION: Registry = {
name: "Accordion",
category: "component",
localDependencies: [ICON, TEXT],
docsUrl:
"https://wethegit.github.io/component-library/?path=/docs/components-accordion-readme--overview",
}

const MODAL: Registry = {
name: "modal",
category: "component",
Expand Down Expand Up @@ -189,6 +197,7 @@ const BACK_TO_TOP: Registry = {

/* INDEX */
export const REGISTRY_INDEX: RegistryIndex = {
[ACCORDION.name]: ACCORDION,
[BACK_TO_TOP.name]: BACK_TO_TOP,
[BREAKPOINT_SNIPE.name]: BREAKPOINT_SNIPE,
[FLEX.name]: FLEX,
Expand Down
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to clean this up a bit.

  • Use more classes
  • Avoid unnecessary nesting that increases specificity
  • Use a separate class to modify state instead of &.active

Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
@mixin reduced-motion {
animation: none !important;
scroll-behavior: auto !important;
transition: none !important;
}

.accordionPanel {
--light-grey: #ebedf0;
--grey: #475f7b;
--white: #fff;

background-color: var(--white);
border: 1px solid var(--light-grey);

& * {
margin: 0;
}

button {
align-items: center;
display: flex;
flex-direction: row;
flex-wrap: nowrap;
justify-content: space-between;
padding: 0.5rem 1rem;
width: 100%;

.accordionTitle {
color: var(--grey);
font-size: 14px;
font-weight: 500;
margin: 0;
}

.accordionIcon {
color: transparent;
padding: 2px 0 0 2px;
transition: 0.35s;
}

&.active {
background-color: var(--light-grey);

.accordionIcon {
transform: rotate(180deg);
transform-origin: center;
}

.accordionTitle {
color: var(--grey);
}
}
}

.accordionBody {
background-color: var(--white);
font-size: 14px;
font-weight: 400;
line-height: 24px;

p {
color: var(--grey);
margin: 0;
padding: 1rem;

+ p {
padding-top: 0;
}
}

&.collapse {
height: 0;
overflow: hidden;
position: relative;
transition: height 0.35s ease;
}
}
}

a,
a:hover,
a:focus {
outline: none;
text-decoration: none;
transition: 0.5s;
}

@media (prefers-reduced-motion) {
.accordionBody.collapse,
.accordionIcon,
button.active .accordionIcon {
@include reduced-motion;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import type { Meta, StoryObj } from "@storybook/react"

import { Accordion } from "."

const meta: Meta<typeof Accordion> = {
title: "components/accordion",
component: Accordion,
}

export default meta

type Story = StoryObj<typeof Accordion>

export const Default: Story = {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import { useState } from "react"

import styles from "./accordion.module.scss"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The styles for the component itself should be imported last to help with CSS loading order.

import AccordionItem, { CardsData } from "./component/AccordionItem"
import cards from "./content.js"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think content should be passed to the component as a prop and not shipped with it, otherwise you can't reuse it.


export interface AccordionProps
extends Omit<React.HTMLAttributes<HTMLDivElement>, "aria-label"> {
"aria-label": string
toggle: () => void
className?: string
}

export function Accordion({}: AccordionProps) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't look like you are using any of the props from AccordionProps, the object is empty

const [active, setActive] = useState<number | null>(null)
const icon = "chevron" // Define your icon name here
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably be a prop.


const handleToggle = (index: number) => {
active === index ? setActive(null) : setActive(index)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whenever using the current state to determine a new use, please use a function
https://react.dev/reference/react/useState#updating-state-based-on-the-previous-state

}

return (
<div className={styles.accordionGroup}>
{cards.map((card: CardsData, index: number) => {
return (
<AccordionItem
key={index}
index={index}
active={active}
handleToggle={handleToggle}
card={card}
icon={icon}
/>
)
})}
</div>
)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please maintain a standard and lowercase the file name.

Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
import React, { useRef } from "react"

import { Icon, IconDefs, IconSymbol, Tag, Text } from "@local/components"

import styles from "../accordion.module.scss"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use a single stylesheet per component.


const DEFAULT_TAG = "h3" as const

export function GenericSection({ ...props }) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure this is being used.

const { as = DEFAULT_TAG, ...rest } = props

return <Tag as={as} {...rest} />
}

export interface CardsData {
title: string
text: string | object
}

interface Props {
handleToggle: (index: number) => void
active: number | null
card: CardsData
icon: string
index: number
}

const AccordionItem: React.FC<Props> = ({ index, active, handleToggle, card, icon }) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Follow standards, use a function. No need for React.FC

const contentEl = useRef<HTMLDivElement>(null)
const { title, text } = card

if (!card) return null
Copy link
Contributor

@marlonmarcello marlonmarcello Apr 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are de-structuring the card variable on the line above but doing the check after. Doesn't make much sense and I think the line above will throw if card is anything other than an object.


return (
<div
className={styles.accordionPanel}
aria-expanded={active === index ? "true" : undefined}
>
<Tag as={DEFAULT_TAG} id={`accordionHeading${index}`}>
<button
className={`${active === index ? styles.active : ""}`}
onClick={() => handleToggle(index)}
tabIndex={0}
aria-controls={`accordionBody${index}`}
>
<span className={styles.accordionTitle}>{title}</span>

<IconDefs>
<IconSymbol id={icon} size={27}>
<path
d="M6 9L12 15L18 9"
stroke="#000000"
strokeWidth="1.2"
strokeLinecap="round"
strokeLinejoin="round"
></path>
</IconSymbol>
</IconDefs>

<Icon id={icon} className={styles.accordionIcon} />
</button>
</Tag>

<Tag
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be just a div no need to use Tag as it won't be replaced by a dynamic value.

ref={contentEl}
className={`${styles.collapse} ${styles.accordionBody}`}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use the classnames helper to concatenate class names

id={`accordionBody${index}`}
aria-labelledby={`accordionHeading${index}`}
role="region"
style={
active === index
? { height: contentEl.current?.scrollHeight || 200 }
: { height: "0px" }
}
>
{typeof text === "string" && (
<Text variant="body-smaller" dangerouslySetInnerHTML={{ __html: text }} />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid dangerouslySetInnerHTML

)}

{typeof text === "object" &&
Object.values(text).map((p, i) => (
<Text
key={i}
variant="body-smaller"
dangerouslySetInnerHTML={{ __html: p.toString() }}
/>
))}
Comment on lines +80 to +87
Copy link
Contributor

@marlonmarcello marlonmarcello Apr 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mind explaining this a bit? Why an object instead of an array? Why the p.toString()? Seems like all you need really is an array of strings.

</Tag>
</div>
)
}

export default AccordionItem
33 changes: 33 additions & 0 deletions packages/wethegit-components/src/components/accordion/content.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
const cards = [
{
title: "Jean-Luc Picard",
text: "Jean-Luc Picard is a fictional character in the Star Trek franchise, most often seen as the captain of the Federation starship USS Enterprise (NCC-1701-D). Played by Patrick Stewart, Picard has appeared in the television series Star Trek: The Next Generation (TNG) and the premiere episode of Star Trek: Deep Space Nine, as well as the feature films Star Trek Generations (1994), Star Trek: First Contact (1996), Star Trek: Insurrection (1998), and Star Trek: Nemesis (2002). He is also featured as the central character in the show Star Trek: Picard (2020–2023).",
},
{
title: "William Riker",
text: `William Thomas "Will" Riker is a fictional character in the Star Trek universe appearing primarily as a main character in Star Trek: The Next Generation, portrayed by Jonathan Frakes. Throughout the series and its accompanying films, he is the Enterprise's first officer, and briefly captain, until he accepts command of the USS Titan at the end of Star Trek: Nemesis. He is the husband of Deanna Troi.`,
},
{
title: "Data",
text: `Data was found by Starfleet in 2338. He was the sole survivor on Omicron Theta in the rubble of a colony left after an attack from the Crystalline Entity. He is a synthetic life form with artificial intelligence, designed and built by Doctor Noonien Soong in his own likeness (likewise portrayed by Spiner).
<br /><br />
Data is a self-aware, sapient, sentient and anatomically fully functional male android who serves as the second officer and chief operations officer aboard the Federation starship USS Enterprise-D and later the USS Enterprise-E.`,
},
{
title: "Worf, son of Mogh",
text: {
paragraph1: `Worf, <i>son of Mogh</i> is a fictional character in the Star Trek franchise, portrayed by actor Michael Dorn. He appears in the television series Star Trek: The Next Generation (TNG), seasons four through seven of Star Trek: Deep Space Nine (DS9) and the third and final season of Star Trek: Picard, as well as the feature films.
<ul>
<li>Star Trek Generations (1994)</li>
<li>Star Trek: First Contact (1996)</li>
<li>Star Trek: Insurrection (1998)</li>
<li>Star Trek: Nemesis (2002)</li>
</ul>`,
paragraph2:
"Worf has appeared in more Star Trek franchise episodes than any other character. Worf is the best character in the Star Trek universe.",
paragraph3: "Worf has much honour",
},
},
]

export default cards
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export { Accordion } from "./accordion"
export type { AccordionProps } from "./accordion"
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import { Meta } from "@storybook/blocks"

<Meta title="components/accordion/Readme" />

# Accordion

`<Accordion>` provides a componentized solution to adding an accordion to your project.

## Install

```bash
npx @wethegit/components-cli add accordion
```

Then add the required styles to your global stylesheet:

```css filename="global.scss"
@import "<path-to>/components/accordion/styles/accordion";
```

## Usage example

```jsx
import { Accordion } from "@local/components/accordion"

function YourComponent() {
return <Accordion>
}
```

## Accessibility

### Reduced motion

This component leverages the `@media (prefers-reduced-motion)` css media feature to dynamically change the duration of the open/close effect, based on whether the user has the reduced motion preference enabled on their system. If they do, the expand/collapse happens instanly rather than being tweened.

## Customize

To customize the accordion styles, you can override the variables inside the `accordion.module.scss` file.
1 change: 1 addition & 0 deletions packages/wethegit-components/src/components/index.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
export * from "./accordion"
export * from "./back-to-top"
export * from "./breakpoint-snipe"
export * from "./flex"
Expand Down
Loading