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

chore(disclosure): update styles, add tests, promoto to beta #1113

Merged
merged 4 commits into from
Jan 28, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 14 additions & 0 deletions .changeset/cold-chefs-cover.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
---
'@twilio-paste/disclosure': major
---

BREAKING CHANGE: style prop is now being blocked by safelySpreadBoxProps, so any additional styles will no longer be rendered.

Style changes:

- Added colorBackgroundBody to DisclosureHeading
- Added colorBackgroundDark to DisclosureHeading on hover
- Added colorBackground to DisclosureHeading when expanded
- Changed the DisclosureHeading icon to ChevronDisclosureExpandedIcon
- Added colorText to icon when hovering on DisclosureHeading
- Icon animates when hovering on DisclosureHeading
5 changes: 5 additions & 0 deletions .changeset/seven-worms-return.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@twilio-paste/website': patch
---

Updated anatomy table on Disclosure doc page to reflect style changes.
5 changes: 4 additions & 1 deletion .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,13 +73,16 @@ module.exports = {
// This rule tells people to do something (import foo = require('foo')) which doesn't work
// with babel compiled typescript.
'@typescript-eslint/no-var-requires': 'off',
// Warn about incorrect type imports
// https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/consistent-type-imports.md
'@typescript-eslint/consistent-type-imports': 'warn',
// PropTypes are useless with typescript
'react/prop-types': 'off',
// ignore dev deps by default, point eslint to all package.json files in the monorepo
'import/no-extraneous-dependencies': [
'error',
{
packageDir: [path.join(__dirname, './'), ...cachedPackages.map(package => package.location)],
packageDir: [path.join(__dirname, './'), ...cachedPackages.map((package) => package.location)],
},
],
'react/jsx-curly-brace-presence': 0,
Expand Down
180 changes: 126 additions & 54 deletions packages/paste-core/components/disclosure/__tests__/disclosure.test.tsx
Original file line number Diff line number Diff line change
@@ -1,29 +1,26 @@
import * as React from 'react';
import {render, screen, fireEvent} from '@testing-library/react';
import {Theme} from '@twilio-paste/theme';
// @ts-ignore typescript doesn't like js imports
import axe from '../../../../../.jest/axe-helper';
import {
Disclosure,
DisclosureContent,
DisclosureHeading,
DisclosureHeadingProps,
DisclosureProps,
DisclosureStateReturn,
useDisclosureState,
} from '../src';
import {Disclosure, DisclosureContent, DisclosureHeading, useDisclosureState} from '../src';
import type {DisclosureHeadingProps, DisclosureProps, DisclosureStateReturn} from '../src';
import {getIconHoverStyles} from '../src/utils';

const MockDisclosure: React.FC<{
visible?: DisclosureProps['visible'];
disabled?: DisclosureHeadingProps['disabled'];
focusable?: DisclosureHeadingProps['focusable'];
}> = ({visible, disabled, focusable}) => {
return (
<Disclosure baseId="disclosure" visible={visible}>
<DisclosureHeading as="h1" variant="heading10" disabled={disabled} focusable={focusable}>
Clickable heading
</DisclosureHeading>
<DisclosureContent data-testid="disclosure">Disclosure content</DisclosureContent>
</Disclosure>
<Theme.Provider theme="default">
<Disclosure baseId="disclosure" visible={visible}>
<DisclosureHeading as="h1" variant="heading10" disabled={disabled} focusable={focusable}>
Clickable heading
</DisclosureHeading>
<DisclosureContent data-testid="disclosure">Disclosure content</DisclosureContent>
</Disclosure>
</Theme.Provider>
);
};

Expand All @@ -42,57 +39,132 @@ const useVisibleDisclosureState = (): DisclosureStateReturn => {
const StateHookMock: React.FC = () => {
const disclosure = useVisibleDisclosureState();
return (
<>
<Theme.Provider theme="default">
<Disclosure variant="contained" state={disclosure}>
<DisclosureHeading as="h2" variant="heading20">
Clickable heading
</DisclosureHeading>
<DisclosureContent>Disclosure content</DisclosureContent>
</Disclosure>
</>
</Theme.Provider>
);
};

describe('Disclosure', () => {
it('should render a disclosure button with aria attributes', () => {
render(<MockDisclosure />);
const renderedDisclosureButton = screen.getByRole('button');
expect(renderedDisclosureButton.getAttribute('aria-expanded')).toEqual('false');
expect(renderedDisclosureButton.getAttribute('aria-controls')).toEqual('disclosure');
expect(renderedDisclosureButton.getAttribute('tabindex')).toEqual('0');
expect(screen.getByTestId('disclosure').id).toEqual('disclosure');
});
it('should render a disclosure open', () => {
render(<MockDisclosure visible />);
const renderedDisclosureButton = screen.getByRole('button');
expect(renderedDisclosureButton.getAttribute('aria-expanded')).toEqual('true');
});
it('should update attributes when clicked', () => {
render(<MockDisclosure />);
const renderedDisclosureButton = screen.getByRole('button');
fireEvent.click(renderedDisclosureButton);
expect(renderedDisclosureButton.getAttribute('aria-expanded')).toEqual('true');
});
it('should render a disabled disclosure', () => {
render(<MockDisclosure disabled />);
const renderedDisclosureButton = screen.getByRole('button');
expect(renderedDisclosureButton.getAttribute('aria-disabled')).toEqual('true');
expect(renderedDisclosureButton.getAttribute('tabindex')).toBeNull();
});
it('should render a disabled but focusable disclosure', () => {
render(<MockDisclosure disabled focusable />);
const renderedDisclosureButton = screen.getByRole('button');
expect(renderedDisclosureButton.getAttribute('aria-disabled')).toEqual('true');
expect(renderedDisclosureButton.getAttribute('tabindex')).toEqual('0');
describe('Unit tests', () => {
it('should return icon hover styles for each heading size', () => {
const mockSpace = {
space10: '10',
space20: '20',
};
expect(getIconHoverStyles(false, 'heading10', false, false, mockSpace)).toMatchObject({
color: 'colorTextIcon',
transform: 'translateX(0) rotate(-90deg)',
transition: 'transform 200ms ease-out',
});
expect(getIconHoverStyles(true, 'heading10', true, false, mockSpace)).toMatchObject({
color: 'colorText',
transform: 'translateX(20) rotate(0deg)',
transition: 'transform 200ms ease-out',
});
expect(getIconHoverStyles(false, 'heading20', false, false, mockSpace)).toMatchObject({
color: 'colorTextIcon',
transform: 'translateX(0) rotate(-90deg)',
transition: 'transform 200ms ease-out',
});
expect(getIconHoverStyles(true, 'heading20', true, false, mockSpace)).toMatchObject({
color: 'colorText',
transform: 'translateX(20) rotate(0deg)',
transition: 'transform 200ms ease-out',
});
expect(getIconHoverStyles(false, 'heading30', false, false, mockSpace)).toMatchObject({
color: 'colorTextIcon',
transform: 'translateX(0) rotate(-90deg)',
transition: 'transform 200ms ease-out',
});
expect(getIconHoverStyles(true, 'heading30', true, false, mockSpace)).toMatchObject({
color: 'colorText',
transform: 'translateX(20) rotate(0deg)',
transition: 'transform 200ms ease-out',
});
expect(getIconHoverStyles(false, 'heading40', false, false, mockSpace)).toMatchObject({
color: 'colorTextIcon',
transform: 'translateX(0) rotate(-90deg)',
transition: 'transform 200ms ease-out',
});
expect(getIconHoverStyles(true, 'heading40', true, false, mockSpace)).toMatchObject({
color: 'colorText',
transform: 'translateX(10) rotate(0deg)',
transition: 'transform 200ms ease-out',
});
expect(getIconHoverStyles(false, 'heading50', false, false, mockSpace)).toMatchObject({
color: 'colorTextIcon',
transform: 'translateX(0) rotate(-90deg)',
transition: 'transform 200ms ease-out',
});
expect(getIconHoverStyles(true, 'heading50', true, false, mockSpace)).toMatchObject({
color: 'colorText',
transform: 'translateX(10) rotate(0deg)',
transition: 'transform 200ms ease-out',
});
expect(getIconHoverStyles(false, 'heading60', false, false, mockSpace)).toMatchObject({
color: 'colorTextIcon',
transform: 'translateX(0) rotate(-90deg)',
transition: 'transform 200ms ease-out',
});
expect(getIconHoverStyles(true, 'heading60', true, false, mockSpace)).toMatchObject({
color: 'colorText',
transform: 'translateX(10) rotate(0deg)',
transition: 'transform 200ms ease-out',
});
expect(getIconHoverStyles(true, 'heading60', true, true, mockSpace)).toMatchObject({
color: 'colorTextIcon',
});
});
});
it('should render a disclosure open and update attributes when clicked using a state hook', () => {
render(<StateHookMock />);
const renderedDisclosureButton = screen.getByRole('button');
expect(renderedDisclosureButton.getAttribute('aria-expanded')).toEqual('true');
fireEvent.click(renderedDisclosureButton);
expect(renderedDisclosureButton.getAttribute('aria-expanded')).toEqual('false');

describe('Render', () => {
it('should render a disclosure button with aria attributes', () => {
render(<MockDisclosure />);
const renderedDisclosureButton = screen.getByRole('button');
expect(renderedDisclosureButton.getAttribute('aria-expanded')).toEqual('false');
expect(renderedDisclosureButton.getAttribute('aria-controls')).toEqual('disclosure');
expect(renderedDisclosureButton.getAttribute('tabindex')).toEqual('0');
expect(screen.getByTestId('disclosure').id).toEqual('disclosure');
});
it('should render a disclosure open', () => {
render(<MockDisclosure visible />);
const renderedDisclosureButton = screen.getByRole('button');
expect(renderedDisclosureButton.getAttribute('aria-expanded')).toEqual('true');
});
it('should update attributes when clicked', () => {
render(<MockDisclosure />);
const renderedDisclosureButton = screen.getByRole('button');
fireEvent.click(renderedDisclosureButton);
expect(renderedDisclosureButton.getAttribute('aria-expanded')).toEqual('true');
});
it('should render a disabled disclosure', () => {
render(<MockDisclosure disabled />);
const renderedDisclosureButton = screen.getByRole('button');
expect(renderedDisclosureButton.getAttribute('aria-disabled')).toEqual('true');
expect(renderedDisclosureButton.getAttribute('tabindex')).toBeNull();
});
it('should render a disabled but focusable disclosure', () => {
render(<MockDisclosure disabled focusable />);
const renderedDisclosureButton = screen.getByRole('button');
expect(renderedDisclosureButton.getAttribute('aria-disabled')).toEqual('true');
expect(renderedDisclosureButton.getAttribute('tabindex')).toEqual('0');
});
it('should render a disclosure open and update attributes when clicked using a state hook', () => {
render(<StateHookMock />);
const renderedDisclosureButton = screen.getByRole('button');
expect(renderedDisclosureButton.getAttribute('aria-expanded')).toEqual('true');
fireEvent.click(renderedDisclosureButton);
expect(renderedDisclosureButton.getAttribute('aria-expanded')).toEqual('false');
});
});
describe('accessibility', () => {

describe('Accessibility', () => {
it('should have no accessibility violations', async () => {
const {container} = render(<MockDisclosure />);
const results = await axe(container);
Expand Down
4 changes: 3 additions & 1 deletion packages/paste-core/components/disclosure/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"name": "@twilio-paste/disclosure",
"version": "0.4.1",
"category": "interaction",
"status": "alpha",
"status": "beta",
"description": "The Disclosure is used to create accessible, hierarchical, and collapsible structure to your pages.",
"author": "Twilio Inc.",
"license": "MIT",
Expand All @@ -25,6 +25,7 @@
"tsc": "tsc"
},
"peerDependencies": {
"@twilio-paste/animation-library": "^0.3.1",
"@twilio-paste/box": "^2.13.1",
"@twilio-paste/card": "^1.5.1",
"@twilio-paste/design-tokens": "^6.5.1",
Expand All @@ -42,6 +43,7 @@
"react-dom": "^16.8.6"
},
"devDependencies": {
"@twilio-paste/animation-library": "^0.3.1",
"@twilio-paste/box": "^2.13.1",
"@twilio-paste/card": "^1.5.1",
"@twilio-paste/design-tokens": "^6.5.1",
Expand Down
38 changes: 6 additions & 32 deletions packages/paste-core/components/disclosure/src/Disclosure.tsx
Original file line number Diff line number Diff line change
@@ -1,37 +1,14 @@
import * as React from 'react';
import * as PropTypes from 'prop-types';
import {
useDisclosurePrimitiveState,
DisclosurePrimitiveInitialState,
DisclosurePrimitveStateReturn,
} from '@twilio-paste/disclosure-primitive';
import {useDisclosurePrimitiveState} from '@twilio-paste/disclosure-primitive';
import {Box} from '@twilio-paste/box';
import {Card} from '@twilio-paste/card';

export type Variants = 'contained' | 'default';

export interface DisclosureContextProps {
disclosure: DisclosurePrimitveStateReturn;
variant: Variants;
}

export const DisclosureContext = React.createContext<DisclosureContextProps>({} as any);

export interface DisclosureStateReturn extends DisclosurePrimitveStateReturn {
[key: string]: any;
}

export type {DisclosurePrimitiveInitialState as DisclosureInitialState};

export interface DisclosureProps extends DisclosurePrimitiveInitialState {
children: NonNullable<React.ReactNode>;
state?: DisclosureStateReturn;
variant?: Variants;
}
import {DisclosureContext} from './DisclosureContext';
import type {DisclosureProps} from './types';
import {DisclosurePropTypes} from './PropTypes';

const Disclosure = React.forwardRef<HTMLDivElement, DisclosureProps>(
({children, variant = 'default', state, ...props}, ref) => {
const disclosure = state || useDisclosurePrimitiveState({...props});
const disclosure = state || useDisclosurePrimitiveState({animated: true, ...props});
const disclosureContext = {
disclosure,
variant,
Expand All @@ -57,10 +34,7 @@ const Disclosure = React.forwardRef<HTMLDivElement, DisclosureProps>(
Disclosure.displayName = 'Disclosure';

if (process.env.NODE_ENV === 'development') {
Disclosure.propTypes = {
children: PropTypes.node.isRequired,
variant: PropTypes.oneOf(['default', 'contained']),
};
Disclosure.propTypes = DisclosurePropTypes;
}
export {Disclosure};

Expand Down