Skip to content

Commit

Permalink
refactor(accordion): improve hooks and props handling
Browse files Browse the repository at this point in the history
  • Loading branch information
segunadebayo committed Feb 21, 2020
1 parent f1f5e66 commit 871f047
Show file tree
Hide file tree
Showing 33 changed files with 572 additions and 863 deletions.
3 changes: 2 additions & 1 deletion .eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
"@typescript-eslint/array-callback-return": "off",
"@typescript-eslint/array-type": "off",
"react-hooks/rules-of-hooks": "error",
"react-hooks/exhaustive-deps": "warn"
"react-hooks/exhaustive-deps": "warn",
"@typescript-eslint/member-delimiter-style": "off"
}
}
7 changes: 2 additions & 5 deletions .prettierrc
Original file line number Diff line number Diff line change
@@ -1,14 +1,11 @@
{
"arrowParens": "avoid",
"bracketSpacing": true,
"jsxBracketSameLine": false,
"jsxSingleQuote": false,
"printWidth": 80,
"proseWrap": "always",
"requirePragma": false,
"semi": true,
"semi": false,
"singleQuote": false,
"tabWidth": 2,
"trailingComma": "all",
"useTabs": false
"trailingComma": "all"
}
69 changes: 57 additions & 12 deletions CODE_REVIEW_GUIDELINE.MD
Original file line number Diff line number Diff line change
@@ -1,26 +1,71 @@
# Code Review Guidelines

The goal of this document is to outline all the requirements for any Chakra
components. At Chakra, we want the entire code base to be beginner friendly,
easy to understand and high quality.
The goal of this document is to outline all the requirements for any Chakra components. At Chakra, we want the entire
code base to be beginner friendly, easy to understand and high quality.

At the end of the day, most of these documented knowledge will be used directlyon the docs website.

## Components Architecture

Every component package must implement the following:

### Components API
- docs/
- [component].api.md
- [component].aria.md
- readme.md: This is the main website docs
- stories/
- basic.stories.tsx
- advanced.stories.tsx
- src/
- [component].hook.tsx (optional): Any amazing hook that's local to this component.
- [component].utils.tsx (optional): Any utility that's local to this component
- [component].tsx: Base component that consumes the hook.
- test/
- utils.test.tsx
- hooks.test.tsx
- component.test.tsx
- accessbility.test.tsx
- readme.md: A basic guideline for this package, installation guide, contribution guide, and sample examples.

### Docs / Components API

- filename: `[component].api.md`
- purpose: document your ideas for this component's API and usage examples. You
can also link to existing libraries written in either jQuery, React, Vue,
vanilla JavaScript.
- purpose: document your ideas for this component's API and usage examples. You can also link to existing libraries
written in either jQuery, React, Vue, vanilla JavaScript.

### Aria Guidelines
### Docs / Aria Guidelines

- filename: `[component].aria.md`
- purpose: This is where you write out the accessibility requirements for this
- purpose: This is where you write out the accessibility requirements and html structure for this component. You can
also link to existing accessibility guidelines or website.

> Ensure you exclude all the `*.notes.md, *.aria.md`, in tsconfig.json
## Code Guidelines

- Write small functions, and compose them: Resist the urge to write a complex function or abstraction in your components
or hooks.

- Test those small functions in isolation and ensure they work as expected

- TypeScript: Hook props should be named as `[hook]HookProps` and it's return type should be `[hook]HookReturn`

- TypeScript: Component prop types should be named with `[compoonent]Props`

- Ensure you update the component dependencies, anytime you import a chakra component

## Hooks

- If a hook is for a component with multiple parts, try to export all the required props for those component parts from
the hook.

- If the "remaining" props for a hook-consuming component is going to be spread unto a component, then return the
"remaining" props from the hook to prevent unnecessary destructuring

- Add a `data-chakra-*` signature to all components so it's easier to spot chakra components anywhere online :)

## Resources

components, and link to any existing
### TypeScript:

> Ensure you exclude all the *notes.md, *aria.md, files from the compiler
> (tsconfig.json)
- https://github.com/labs42io/clean-code-typescript
6 changes: 6 additions & 0 deletions jest.config.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
module.exports = {
setupFilesAfterEnv: ["@testing-library/jest-dom/extend-expect"],
snapshotSerializers: ["jest-emotion"],
moduleNameMapper: {
"@chakra-ui/utils": "<rootDir>/packages/utils",
"@chakra-ui/hooks": "<rootDir>/packages/hooks",
"@chakra-ui/system": "<rootDir>/packages/system",
},
transformIgnorePatterns: ["^.+\\.js$"],
};
4 changes: 2 additions & 2 deletions packages/Accordion/__tests__/Accordion.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ test("Button renders correctly", () => {
});

test("It opens the accordion panel", () => {
const { getByTestId, debug } = render(
const { getByTestId } = render(
<BaseAccordion defaultIndex={0}>
<BaseAccordionItem>
<BaseAccordionButton data-testid="button">
Expand All @@ -33,5 +33,5 @@ test("It opens the accordion panel", () => {
);

const button = getByTestId("button");
expect(button).toHaveAttribute("aria-expanded", "false");
expect(button).toHaveAttribute("aria-expanded", "true");
});
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ exports[`Button renders correctly 1`] = `
</button>
<div
aria-labelledby="accordion-header-1"
context="[object Object]"
data-chakra-accordion-panel=""
hidden=""
id="accordion-panel-1"
Expand Down
Empty file.
Empty file.
Empty file.
29 changes: 19 additions & 10 deletions packages/Accordion/package.json
Original file line number Diff line number Diff line change
@@ -1,12 +1,19 @@
{
"name": "@chakra-ui/accordion",
"version": "0.0.1",
"description": "description",
"description": "A simple and accessible accordion component for React & Chakra UI",
"keywords": [
"theme",
"theming",
"ui mode",
"ui"
"react",
"component",
"accordion",
"chakra",
"chakra ui",
"collapse",
"accessible",
"accessibility",
"wai-aria",
"aria",
"a11y"
],
"author": "Segun Adebayo <sage@adebayosegun.com>",
"homepage": "https://github.com/chakra-ui/chakra-ui#readme",
Expand Down Expand Up @@ -47,12 +54,14 @@
"tslib": "1.10.0",
"typescript": "3.7.4"
},
"dependencies": {},
"dependencies": {
"@chakra-ui/descendant": "0.0.1",
"@chakra-ui/hooks": "0.0.1",
"@chakra-ui/utils": "0.0.1",
"@chakra-ui/system": "0.0.1"
},
"peerDependencies": {
"@emotion/core": "10.x",
"@emotion/styled": "10.x",
"emotion-theming": "10.x",
"react": "16.x",
"react-dom": "16.x"
}
}
}
102 changes: 45 additions & 57 deletions packages/Accordion/src/Accordion.base.tsx
Original file line number Diff line number Diff line change
@@ -1,82 +1,70 @@
import { PropsOf } from "@chakra-ui/system";
import { Omit } from "@chakra-ui/utils";
import { createContext } from "@chakra-ui/utils/src";
import * as React from "react";
import { PropsOf } from "@chakra-ui/system"
import { Omit } from "@chakra-ui/utils"
import { createContext } from "@chakra-ui/utils/src"
import * as React from "react"
import {
AccordionHookProps,
AccordionItemHookProps,
useAccordion,
useAccordionItem,
useAccordionItemButton,
useAccordionItemPanel,
} from "./Accordion.hook";
AccordionHookReturn,
AccordionItemHookReturn,
} from "./Accordion.hook"

type AccordionContext = Omit<ReturnType<typeof useAccordion>, "children">;
const [AccordionProvider, useAccordionContext] = createContext<
type AccordionContext = Omit<AccordionHookReturn, "children" | "htmlProps">
const [AccordionCtxProvider, useAccordionCtx] = createContext<
AccordionContext
>();
>()

export function BaseAccordion({
children,
defaultIndex,
...props
}: AccordionHookProps & PropsOf<"div">) {
const { children: enhancedChildren, ...context } = useAccordion({
children,
defaultIndex,
});
export type BaseAccordionProps = AccordionHookProps &
Omit<PropsOf<"div">, "onChange">

export function BaseAccordion(props: BaseAccordionProps) {
const { children, htmlProps, ...context } = useAccordion(props)
return (
<AccordionProvider value={context}>
<div data-chakra-accordion="" {...props} children={enhancedChildren} />
</AccordionProvider>
);
<AccordionCtxProvider value={context}>
<div data-chakra-accordion="" {...htmlProps}>
{children}
</div>
</AccordionCtxProvider>
)
}

type AccordionItemContext = Omit<AccordionItemHookReturn, "htmlProps">

const [AccordionItemProvider, useAccordionItemContext] = createContext<
ReturnType<typeof useAccordionItem>
>();
AccordionItemContext
>()

export function useAccordionItemState() {
const { isOpen, isDisabled, onClose } = useAccordionItemContext();
return { isOpen, isDisabled, onClose };
const { isOpen, onClose, onOpen } = useAccordionItemContext()
return { isOpen, onClose, onOpen }
}

export function BaseAccordionItem(
props: PropsOf<"div"> & AccordionItemHookProps,
) {
const {
isFocusable,
isDisabled,
onChange,
isOpen,
defaultIsOpen,
...htmlProps
} = props;

const context = useAccordionContext();

const itemContext = useAccordionItem({
context,
isFocusable,
isDisabled,
onChange,
});
export type BaseAccordionItemProps = PropsOf<"div"> &
Omit<AccordionItemHookProps, "context">

export function BaseAccordionItem(props: BaseAccordionItemProps) {
const accordionContext = useAccordionCtx()
const { htmlProps, ...context } = useAccordionItem({
...props,
context: accordionContext,
})
return (
<AccordionItemProvider value={itemContext}>
<AccordionItemProvider value={context}>
<div data-chakra-accordion-item="" {...htmlProps} />
</AccordionItemProvider>
);
)
}

export function BaseAccordionButton(props: PropsOf<"button">) {
const context = useAccordionItemContext();
const buttonProps = useAccordionItemButton({ context });
return <button data-chakra-accordion-button="" {...props} {...buttonProps} />;
export type BaseAccordionButtonProps = PropsOf<"button">
export function BaseAccordionButton(props: BaseAccordionButtonProps) {
const { getButtonProps } = useAccordionItemContext()
return <button data-chakra-accordion-button="" {...getButtonProps(props)} />
}

export function BaseAccordionPanel(props: PropsOf<"div">) {
const context = useAccordionItemContext();
const panelProps = useAccordionItemPanel({ context });
return <div data-chakra-accordion-panel="" {...props} {...panelProps} />;
export type BaseAccordionPanelProps = PropsOf<"div">
export function BaseAccordionPanel(props: BaseAccordionPanelProps) {
const { getPanelProps } = useAccordionItemContext()
return <div data-chakra-accordion-panel="" {...getPanelProps(props)} />
}
Loading

0 comments on commit 871f047

Please sign in to comment.