Skip to content

Commit

Permalink
✨ Replace Accordion ReachUI component with ChakraUI (#1378)
Browse files Browse the repository at this point in the history
  • Loading branch information
millianapia committed Dec 8, 2022
1 parent 2c4e277 commit 6f528ad
Show file tree
Hide file tree
Showing 4 changed files with 171 additions and 70 deletions.
28 changes: 21 additions & 7 deletions web/components/src/Accordion/Accordion.tsx
Original file line number Diff line number Diff line change
@@ -1,15 +1,29 @@
import { forwardRef } from 'react'

import { Accordion as RAccordion, AccordionProps as RAccordionProps } from '@reach/accordion'
import { Accordion as CAccordion } from '@chakra-ui/react'
import { Flags } from '../../../common/helpers/datasetHelpers'

export type AccordionProps = RAccordionProps & {
id: string
}
export type CAccordionProps = {
id: string
children?: React.ReactNode
}

export const Accordion = forwardRef<HTMLDivElement, AccordionProps>(function Accordion({ id, children, ...rest }, ref) {
return (
<RAccordion ref={ref} {...rest} id={id}>
{children}
</RAccordion>
)
})
export const Accordion = Flags.IS_DEV
? forwardRef<HTMLDivElement, CAccordionProps>(function Accordion({ id, children }, ref) {
return (
<CAccordion ref={ref} allowMultiple id={id}>
{children}
</CAccordion>
)
})
: forwardRef<HTMLDivElement, AccordionProps>(function Accordion({ id, children, ...rest }, ref) {
return (
<RAccordion ref={ref} {...rest} id={id}>
{children}
</RAccordion>
)
})
106 changes: 77 additions & 29 deletions web/components/src/Accordion/Header.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import {
useAccordionItemContext,
} from '@reach/accordion'

import { Flags } from '../../../common/helpers/datasetHelpers'
import { AccordionButton as CAccordionButton, useAccordionItemState } from '@chakra-ui/react'
import { outlineTemplate, Tokens } from '@utils'

const { outline } = Tokens
Expand All @@ -16,6 +18,23 @@ export type AccordionHeaderProps = {
headingLevel?: 'h2' | 'h3' | 'h4' | 'h5'
} & RAccordionButtonProps

export type CAccordionHeaderProps = {
headingLevel?: 'h2' | 'h3' | 'h4' | 'h5'
children: React.ReactNode
}

const StyledCAccordionButton = styled(CAccordionButton)`
display: flex;
align-items: center;
width: 100%;
background: transparent;
padding: var(--space-medium) 0;
border: none;
cursor: pointer;
&[data-focus-visible-added]:focus {
${outlineTemplate(outline)}
}
`
const StyledRAccordionButton = styled(RAccordionButton)`
display: flex;
align-items: center;
Expand Down Expand Up @@ -72,32 +91,61 @@ const StyledTypography = styled(Typography)<{ isExpanded?: boolean }>`
}}
`

export const Header = forwardRef<HTMLButtonElement, AccordionHeaderProps>(function Header(
{ headingLevel = 'h3', children, ...rest },
ref,
) {
const context = useAccordionItemContext()
const isExpanded = context.isExpanded
const iconSize = 24
return (
<StyledHeader forwardedAs={headingLevel}>
<StyledRAccordionButton ref={ref} {...rest}>
{/* Let's do it in the easiest way by just swapping the icons and see how that works */}
{isExpanded ? (
<StyledIcon>
<OutlineIcon size={iconSize} data={remove_outlined} />
<FilledIcon size={iconSize} data={remove} />
</StyledIcon>
) : (
<StyledIcon>
<OutlineIcon size={iconSize} data={add_circle_outlined} />
<FilledIcon size={iconSize} data={add_circle_filled} />
</StyledIcon>
)}
<StyledTypography isExpanded={isExpanded} forwardedAs="span">
{children}
</StyledTypography>
</StyledRAccordionButton>
</StyledHeader>
)
})
export const Header = Flags.IS_DEV
? forwardRef<HTMLButtonElement, CAccordionHeaderProps>(function Header(
{ headingLevel = 'h3', children, ...rest },
ref,
) {
const iconSize = 24
const { isOpen } = useAccordionItemState()

return (
<StyledHeader forwardedAs={headingLevel}>
<StyledCAccordionButton ref={ref} {...rest}>
{isOpen ? (
<StyledIcon>
<OutlineIcon size={iconSize} data={remove_outlined} />
<FilledIcon size={iconSize} data={remove} />
</StyledIcon>
) : (
<StyledIcon>
<OutlineIcon size={iconSize} data={add_circle_outlined} />
<FilledIcon size={iconSize} data={add_circle_filled} />
</StyledIcon>
)}
<StyledTypography isExpanded={isOpen} forwardedAs="span">
{children}
</StyledTypography>
</StyledCAccordionButton>
</StyledHeader>
)
})
: forwardRef<HTMLButtonElement, AccordionHeaderProps>(function Header(
{ headingLevel = 'h3', children, ...rest },
ref,
) {
const context = useAccordionItemContext()
const isExpanded = context.isExpanded
const iconSize = 24
return (
<StyledHeader forwardedAs={headingLevel}>
<StyledRAccordionButton ref={ref} {...rest}>
{/* Let's do it in the easiest way by just swapping the icons and see how that works */}
{isExpanded ? (
<StyledIcon>
<OutlineIcon size={iconSize} data={remove_outlined} />
<FilledIcon size={iconSize} data={remove} />
</StyledIcon>
) : (
<StyledIcon>
<OutlineIcon size={iconSize} data={add_circle_outlined} />
<FilledIcon size={iconSize} data={add_circle_filled} />
</StyledIcon>
)}
<StyledTypography isExpanded={isExpanded} forwardedAs="span">
{children}
</StyledTypography>
</StyledRAccordionButton>
</StyledHeader>
)
})
30 changes: 24 additions & 6 deletions web/components/src/Accordion/Item.tsx
Original file line number Diff line number Diff line change
@@ -1,18 +1,36 @@
import styled from 'styled-components'
import { AccordionItem as RAccordionItem, AccordionItemProps as RAccordionItemProps } from '@reach/accordion'
import { AccordionItem as CAccordionItem } from '@chakra-ui/react'
import { Flags } from '../../../common/helpers/datasetHelpers'

const StyledItem = styled(RAccordionItem)`
border-bottom: 1px solid var(--grey-40);
`
const CStyledItem = styled(CAccordionItem)`
border-bottom: 1px solid var(--grey-40);
`

export type AccordionItemProps = RAccordionItemProps & {
id: number
}

export const Item = ({ id, children, ...rest }: AccordionItemProps) => {
return (
<StyledItem {...rest} index={id}>
{children}
</StyledItem>
)
export type CAccordionItemProps = {
id: number
children: React.ReactNode

This comment has been minimized.

Copy link
@fernandolucchesi

fernandolucchesi Dec 21, 2022

Contributor

there is an extra space after "ReactNode" 😄 which makes me guess your prettier is not yet correctly setup

}

export const Item = Flags.IS_DEV
? ({ id, children, ...rest }: CAccordionItemProps) => {
return (
<CStyledItem {...rest} index={id}>
{children}
</CStyledItem>
)
}
: ({ id, children, ...rest }: AccordionItemProps) => {
return (
<StyledItem {...rest} index={id}>
{children}
</StyledItem>
)
}
77 changes: 49 additions & 28 deletions web/components/src/Accordion/Panel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,18 @@ import {
useAccordionItemContext,
} from '@reach/accordion'
import styled from 'styled-components'
import { AccordionPanel as CAccordionPanel } from '@chakra-ui/react'
import { Flags } from '../../../common/helpers/datasetHelpers'

export type AccordionPanelProps = {
animate?: boolean
} & RAccordionPanelProps

export type CAccordionPanelProps = {
children?: React.ReactNode
animate?: boolean
}

const AnimatedAccordionPanel = animated(RAccordionPanel)

const StyledPanel = styled.div`
Expand All @@ -27,32 +34,46 @@ const ContentWithBorder = styled.div`
padding-left: calc(var(--space-xLarge) / 2);
`

export const Panel = forwardRef<HTMLDivElement, AccordionPanelProps>(function Panel(
{ animate = true, children, ...rest },
forwardedRef,
) {
const { isExpanded } = useAccordionItemContext()
const { ref, height } = useDivHeight()
const prefersReducedMotion = usePrefersReducedMotion()
const animation = useSpring({
opacity: isExpanded ? 1 : 0,
height: isExpanded ? height : 0,
overflow: 'hidden',
config: { duration: 150 },
immediate: prefersReducedMotion || !animate,
})
export const Panel = Flags.IS_DEV
? forwardRef<HTMLDivElement, CAccordionPanelProps>(function Panel(
{ children, animate = true, ...rest },

This comment has been minimized.

Copy link
@fernandolucchesi

fernandolucchesi Dec 21, 2022

Contributor

animate is not being used

This comment has been minimized.

Copy link
@fernandolucchesi

fernandolucchesi Dec 21, 2022

Contributor

I checked the old component and looks like the animation wasn't working there either, possibly a bug? I'd expect the hovering transition to be smooth but not exactly sure

This comment has been minimized.

Copy link
@millianapia

millianapia Dec 21, 2022

Author Contributor

i added the animated prop there because it is passed as a prop when used in the project now, but when the reach component is removed, this prop can be removed as well. I added a screenshot of how it looks like when the prop is removed. The reason why it is not needed now, is because the component already comes with a built in animation.

Screenshot 2022-12-21 at 16 14 28

forwardedRef,
) {
const { ref } = useDivHeight()
return (
<CAccordionPanel ref={forwardedRef} {...rest}>
<StyledPanel ref={ref}>
<ContentWithBorder>{children}</ContentWithBorder>
</StyledPanel>
</CAccordionPanel>
)
})
: forwardRef<HTMLDivElement, AccordionPanelProps>(function Panel(
{ animate = true, children, ...rest },
forwardedRef,
) {
const { isExpanded } = useAccordionItemContext()
const { ref, height } = useDivHeight()
const prefersReducedMotion = usePrefersReducedMotion()
const animation = useSpring({
opacity: isExpanded ? 1 : 0,
height: isExpanded ? height : 0,
overflow: 'hidden',
config: { duration: 150 },
immediate: prefersReducedMotion || !animate,
})

return (
<AnimatedAccordionPanel
style={animation}
hidden={false}
aria-hidden={!isExpanded || undefined}
ref={forwardedRef}
{...rest}
>
<StyledPanel ref={ref}>
<ContentWithBorder>{children}</ContentWithBorder>
</StyledPanel>
</AnimatedAccordionPanel>
)
})
return (
<AnimatedAccordionPanel
style={animation}
hidden={false}
aria-hidden={!isExpanded || undefined}
ref={forwardedRef}
{...rest}
>
<StyledPanel ref={ref}>
<ContentWithBorder>{children}</ContentWithBorder>
</StyledPanel>
</AnimatedAccordionPanel>
)
})

1 comment on commit 6f528ad

@fernandolucchesi
Copy link
Contributor

Choose a reason for hiding this comment

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

New accordion has different margins from old one:
image
vs
image

Please sign in to comment.