feat(action-bar): Refactor ActionBar component

[RayRedGoose]

Summary

• Convert to compound component.
• Remove ChildrenContainer and replace it styles to ActionBarContainer.
• Update documentation
• Add jest component test and visual tests for storybook.

Resolves: #1382

BREAKING CHANGES

Change fixed prop from component to position to set container position (fixed position has been set as default).

[cypress]

---

Test summary

674 • 0 • 1 • 0 • 0

---

Run details

Project	canvas-kit
Status	Passed
Commit	7efe4c7 ℹ️
Started	Jan 7, 2022 11:01 PM
Ended	Jan 7, 2022 11:04 PM
Duration	03:08 💡
OS	Linux Ubuntu - 20.04
Browser	Electron 85

View run in Cypress Dashboard ➡️

---

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

[NicholasBoll]

Overall I'm torn on this refactor. I didn't have comments on specific lines, so I'll review here:

With many conversions to compound components, we've been moving to using layout components which give us a lot of niceties: many style props come for free and defaults are easily overridden.

For example:

Using styled

export interface SomeStyledProps {
  position: ...
}

const SomeStyledComponent = styled('div')<SomeStypedProps & StyledType>({
  display: 'flex',
  ...
  position: 'fixed',
  left: 0,
  right: 0,
  top: 0,
  ...
  ({position}) => ({position})
})

const SomeComponent = createComponent({
  Component(props: SomeComponentProps, ref, Element) {
    return <SomeStyledComponent ref={ref} as={Element} {...props} />
  }
})

Layout components

interface SomeComponentProps extends FlexProps {}

const SomeComponent = createComponent({
  Component(props: SomeComponentProps, ref, Element) {
    return (
      <Flex
        ref={ref}
        as={Element}
        position="fixed"
        top={0}
        left={0}
        right={0}
        {...props}
      />
    )
  }
})

We don't export prop types anymore with createComponent. The reason is the component is polymorphic, meaning exporting props aren't as useful as it used to be. Instead we use ExtractProps from the common module:

interface Props {
  foo: string
}

const Component = createComponent('div')({
  Component(props: Props) {
    return <div />
  }
})

type Props1 = Props // Props
type Props2 = ExtractProps<typeof Component> // Props & React.HTMLAttributes<HTMLDivElement>
type Props3 = ExtractProps<typeof Component, never> // Props
type Props4 = ExtractProps<typeof Component, 'aside'> // Props & React.HTMLAttributes<HTMLElement>

If someone wants to extend Component and use the same props and get the HTML attributes that come with the element, they can use ExtractProps to get the interface. If they use as, they can add the second argument:

const MyComponent = (props: ExtractProps<typeof Component, 'aside'>) => {
  return <Component as="aside" {...props} />
}

Also, fixed used to default to false... Should we maintain that? If so, the codemod is incomplete. It should change fixed={true} to position="fixed".

If ActionBar was actually a HStack element, the flex + gap would be handled and wouldn't need to be in the styled component.

[alanbsmith]

Two questions:

• Will this support default imports? import ActionBar from '@workday/canvas-kit-react/action-bar
• Will this support imports from @workday/canvas-kit-react?

[RayRedGoose]

• No. There is no default import from action-bar files.
• Yes. It supports imports from main directory.

[alanbsmith]

Just one small copy nit. The rest looks great! Thanks!!

[alanbsmith]

WIll the codemod also update any default imports to named imports? E.g.

import ActionBar from '@workday/canvas-kit-react/action-bar';

becomes this:

import { ActionBar } from '@workday/canvas-kit-react/action-bar';

[RayRedGoose]

Thanks for catching it! I've updated existing codemod to add a script that changes default import to named or renamed import from action-bar package. Now codemod should cover this problem.

[RayRedGoose]

This PR will be on hold by tomorrow. @alanbsmith found a change that is not covered by codemod but It's reasonable to have opportunity to change default import to named using codemod.

[NicholasBoll]

This is a visual regression test that isn't showing the buttons. Is this intentional? https://www.chromatic.com/test?appId=5d854c26ba934e0020f5e98a&id=61d8c63dd73a51003ae4758a

Changes were implemented