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

A11y design enhancements #6563

Merged
merged 23 commits into from
Apr 25, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
4e043eb
made global checkbox responsive
CodeByAlex Apr 12, 2019
4cb4de8
modified empty text for each accessibility status
CodeByAlex Apr 12, 2019
bdbd153
added badge to make the severity display more accessible
CodeByAlex Apr 12, 2019
d1e8ccc
removed passes from props and used type to determine the badge needed
CodeByAlex Apr 12, 2019
321ba10
fixed spacing issues between checkbox and element path
CodeByAlex Apr 12, 2019
a28dd42
a11y: added padding next to rule text to allow for more space below c…
CodeByAlex Apr 12, 2019
e9320b7
modified styling of a11y severity badges
CodeByAlex Apr 16, 2019
827898e
added critical badge color
CodeByAlex Apr 18, 2019
0782de0
added react resize-me to allow for automatic flowing of the global ch…
CodeByAlex Apr 18, 2019
6c84cbb
Used resize-me to auto flow the badges when the width of the addon pa…
CodeByAlex Apr 18, 2019
f1521ef
made the badge slimmer for the a11y rules
CodeByAlex Apr 18, 2019
de96c0f
fixed ts issues
CodeByAlex Apr 18, 2019
3bdf885
added a constant for Item width in the the a11y rules
CodeByAlex Apr 19, 2019
8095c23
Merge branch 'next' into a11y-design-enhancements
CodeByAlex Apr 19, 2019
c74bd00
updated storyshots and badge options
CodeByAlex Apr 19, 2019
bfe6df8
ran yarn bootstrap --reset --core and added changes
CodeByAlex Apr 20, 2019
c245294
updated snapshots
CodeByAlex Apr 20, 2019
c453929
Capitalized colorblindness emulation text
CodeByAlex Apr 23, 2019
e5550aa
Capitalized mono for color blindness emulation
CodeByAlex Apr 23, 2019
b80728c
Merge branch 'next' into a11y-design-enhancements
CodeByAlex Apr 24, 2019
0a7146d
inserted the es lint disable comments originally added by ndelangen w…
CodeByAlex Apr 24, 2019
71791b5
modified color blindness logic to use lowercase version of visual tit…
CodeByAlex Apr 25, 2019
4426529
modified color blindness logic to set the first character of title to…
CodeByAlex Apr 25, 2019
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
1 change: 1 addition & 0 deletions addons/a11y/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
"memoizerific": "^1.11.3",
"react": "^16.8.4",
"react-redux": "^7.0.2",
"react-sizeme": "^2.5.2",
"redux": "^4.0.1",
"util-deprecate": "^1.0.2"
},
Expand Down
9 changes: 3 additions & 6 deletions addons/a11y/src/components/A11YPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -175,10 +175,9 @@ export class A11YPanel extends Component<A11YPanelProps, A11YPanelState> {
label: <Violations>{violations.length} Violations</Violations>,
panel: (
<Report
passes={false}
items={violations}
type={RuleType.VIOLATION}
empty="No a11y violations found."
empty="No accessibility violations found."
/>
),
items: violations,
Expand All @@ -188,10 +187,9 @@ export class A11YPanel extends Component<A11YPanelProps, A11YPanelState> {
label: <Passes>{passes.length} Passes</Passes>,
panel: (
<Report
passes
items={passes}
type={RuleType.PASS}
empty="No a11y check passed."
empty="No accessibility checks passed."
/>
),
items: passes,
Expand All @@ -201,10 +199,9 @@ export class A11YPanel extends Component<A11YPanelProps, A11YPanelState> {
label: <Incomplete>{incomplete.length} Incomplete</Incomplete>,
panel: (
<Report
passes={false}
items={incomplete}
type={RuleType.INCOMPLETION}
empty="No a11y incomplete found."
empty="No accessibility checks incomplete."
/>
),
items: incomplete,
Expand Down
2 changes: 1 addition & 1 deletion addons/a11y/src/components/ColorBlindness.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ export class ColorBlindness extends Component<ColorBlindnessProps, ColorBlindnes
'mono',
].map(i => ({
id: i,
Copy link
Member

Choose a reason for hiding this comment

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

is this id used anywhere?

Copy link
Member

Choose a reason for hiding this comment

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

suggest:

id: i.toLowerCase()
...
  this.setFilter(this.id)

Copy link
Member Author

@CodeByAlex CodeByAlex Apr 25, 2019

Choose a reason for hiding this comment

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

after I modified the array of types, its back to being lowercase by default and only the first char is capitalized in the title. Not sure if the id is being used but I can check.

UPDATE: the Id is used in the TooltipLinkList: key={id || (title as string)}

title: i,
title: i.charAt(0).toUpperCase() + i.slice(1),
onClick: () => {
this.setFilter(i);
},
Expand Down
15 changes: 7 additions & 8 deletions addons/a11y/src/components/Report/Elements.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,25 +14,25 @@ const Item = styled.li({
const ItemTitle = styled.span(({ theme }) => ({
borderBottom: `1px solid ${theme.appBorderColor}`,
width: '100%',
display: 'inline-block',
display: 'flex',
paddingBottom: '6px',
marginBottom: '6px',
justifyContent: 'space-between',
}));

const HighlightToggleElement = styled.span({
fontWeight: 'normal',
float: 'right',
alignSelf: 'center',
paddingRight: '15px',
input: { margin: 0 },
});

interface ElementProps {
element: NodeResult;
passes: boolean;
type: RuleType;
}

const Element: FunctionComponent<ElementProps> = ({ element, passes, type }) => {
const Element: FunctionComponent<ElementProps> = ({ element, type }) => {
const { any, all, none } = element;
const rules = [...any, ...all, ...none];
const highlightToggleId = `${type}-${element.target[0]}`;
Expand All @@ -51,21 +51,20 @@ const Element: FunctionComponent<ElementProps> = ({ element, passes, type }) =>
/>
</HighlightToggleElement>
</ItemTitle>
<Rules rules={rules} passes={passes} />
<Rules rules={rules} />
</Item>
);
};

interface ElementsProps {
elements: NodeResult[];
passes: boolean;
type: RuleType;
}

export const Elements: FunctionComponent<ElementsProps> = ({ elements, passes, type }) => (
export const Elements: FunctionComponent<ElementsProps> = ({ elements, type }) => (
<ol>
{elements.map((element, index) => (
<Element passes={passes} element={element} key={index} type={type} />
<Element element={element} key={index} type={type} />
))}
</ol>
);
9 changes: 4 additions & 5 deletions addons/a11y/src/components/Report/Item.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import HighlightToggle from './HighlightToggle';

const Wrapper = styled.div(({ theme }) => ({
display: 'flex',
width: '100%',
borderBottom: `1px solid ${theme.appBorderColor}`,
'&:hover': {
background: theme.background.hoverable,
Expand Down Expand Up @@ -49,14 +50,12 @@ const HighlightToggleElement = styled.span({
fontWeight: 'normal',
float: 'right',
marginRight: '15px',
marginTop: '10px',

alignSelf: 'center',
input: { margin: 0 },
});

interface ItemProps {
item: Result;
passes: boolean;
type: RuleType;
}

Expand All @@ -75,7 +74,7 @@ export class Item extends Component<ItemProps, ItemState> {
}));

render() {
const { item, passes, type } = this.props;
const { item, type } = this.props;
const { open } = this.state;
const highlightToggleId = `${type}-${item.id}`;

Expand Down Expand Up @@ -104,7 +103,7 @@ export class Item extends Component<ItemProps, ItemState> {
{open ? (
<Fragment>
<Info item={item} key="info" />
<Elements elements={item.nodes} passes={passes} type={type} key="elements" />
<Elements elements={item.nodes} type={type} key="elements" />
<Tags tags={item.tags} key="tags" />
</Fragment>
) : null}
Expand Down
87 changes: 68 additions & 19 deletions addons/a11y/src/components/Report/Rules.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import React, { FunctionComponent } from 'react';
import { styled } from '@storybook/theming';

import { Icons } from '@storybook/components';
import { Badge, Icons } from '@storybook/components';
import { CheckResult } from 'axe-core';
import { RuleType } from '../A11YPanel';
import { SizeMe } from 'react-sizeme';
Copy link
Member

@ndelangen ndelangen Apr 19, 2019

Choose a reason for hiding this comment

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

We could cut back on the number of dependencies by using https://github.com/storybooks/storybook/blob/next/lib/ui/src/app.js#L4

import ResizeDetector from 'react-resize-detector';

Copy link
Member Author

Choose a reason for hiding this comment

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

That's a good point. I hadn't spotted that in the repo. It looks like it also has the "refreshMode" feature so it fits the needs. Ill convert what I have to use react-resize-detector. Thanks!

Copy link
Member Author

@CodeByAlex CodeByAlex Apr 19, 2019

Choose a reason for hiding this comment

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

So I tried the react-resize-detector and there is a delay between when you start an action and the resizing occurs. Think we should switch back to ResizeMe where this did not occur. react-sizeme does an initial, invisible render of the component to measure its size. Then, when the component renders, the size is known and the component can render properly from the beginning.
glitchy-response

Resize me:
smooth-response

Copy link
Member Author

Choose a reason for hiding this comment

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

I tried switching out the react-resize-detector for resize me but for that use case, react-resize-detector was the better library

Copy link
Member Author

Choose a reason for hiding this comment

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

@ndelangen what do you think about keeping both?


const impactColors = {
minor: '#f1c40f',
Expand All @@ -15,18 +16,33 @@ const impactColors = {
const List = styled.div({
display: 'flex',
flexDirection: 'column',
padding: '4px',
paddingBottom: '4px',
paddingRight: '4px',
paddingTop: '4px',
fontWeight: '400',
} as any);

const Item = styled.div({
display: 'flex',
flexDirection: 'row',
marginBottom: '6px',
const Item = styled.div(({ elementWidth }: { elementWidth: number }) => {
const maxWidthBeforeBreak = 407;
return {
flexDirection: elementWidth > maxWidthBeforeBreak ? 'row' : 'inherit',
marginBottom: elementWidth > maxWidthBeforeBreak ? '6px' : '12px',
display: elementWidth > maxWidthBeforeBreak ? 'flex' : 'block',
};
});

const StyledBadge = styled(Badge)(({ status }: { status: string }) => ({
padding: '2px 8px',
marginBottom: '3px',
minWidth: '65px',
maxWidth: 'fit-content',
width: '100%',
textAlign: 'center',
}));

const Message = styled.div({
paddingLeft: '6px',
paddingRight: '23px',
});

const Status = styled.div(({ passes, impact }: { passes: boolean; impact: string }) => ({
Expand All @@ -40,30 +56,63 @@ const Status = styled.div(({ passes, impact }: { passes: boolean; impact: string
},
}));

export enum ImpactValue {
MINOR = 'minor',
MODERATE = 'moderate',
SERIOUS = 'serious',
CRITICAL = 'critical',
}

interface RuleProps {
rule: CheckResult;
passes: boolean;
}

const Rule: FunctionComponent<RuleProps> = ({ rule, passes }) => (
<Item>
<Status passes={passes || undefined} impact={rule.impact}>
{passes ? <Icons icon="check" /> : <Icons icon="cross" />}
</Status>
<Message>{rule.message}</Message>
</Item>
);
const formatSeverityText = (severity: string) => {
return severity
.charAt(0)
.toUpperCase()
.concat(severity.slice(1));
};

const Rule: FunctionComponent<RuleProps> = ({ rule }) => {
let badgeType: any = null;
switch (rule.impact) {
case ImpactValue.CRITICAL:
badgeType = 'critical';
break;
case ImpactValue.SERIOUS:
badgeType = 'negative';
break;
case ImpactValue.MODERATE:
badgeType = 'warning';
break;
case ImpactValue.MINOR:
badgeType = 'neutral';
break;
default:
break;
}
Armanio marked this conversation as resolved.
Show resolved Hide resolved
return (
<SizeMe refreshMode="debounce">
{({ size }: { size: any }) => (
<Item elementWidth={size.width}>
<StyledBadge status={badgeType}>{formatSeverityText(rule.impact)}</StyledBadge>
<Message>{rule.message}</Message>
</Item>
)}
</SizeMe>
);
};

interface RulesProps {
rules: CheckResult[];
passes: boolean;
}

export const Rules: FunctionComponent<RulesProps> = ({ rules, passes }) => {
export const Rules: FunctionComponent<RulesProps> = ({ rules }) => {
return (
<List>
{rules.map((rule, index) => (
<Rule passes={passes} rule={rule} key={index} />
<Rule rule={rule} key={index} />
))}
</List>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ exports[`HighlightToggle component should match snapshot 1`] = `
"app": "#F6F9FC",
"bar": "#FFFFFF",
"content": "#FFFFFF",
"critical": "#FF4400",
"gridCellSize": 10,
"hoverable": "rgba(0,0,0,.05)",
"negative": "#FEDED2",
Expand Down Expand Up @@ -256,6 +257,7 @@ exports[`HighlightToggle component should match snapshot 1`] = `
"color": Object {
"ancillary": "#22a699",
"border": "rgba(0,0,0,.1)",
"critical": "#FFFFFF",
"dark": "#666666",
"darker": "#444444",
"darkest": "#333333",
Expand Down
5 changes: 2 additions & 3 deletions addons/a11y/src/components/Report/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,13 @@ import { RuleType } from '../A11YPanel';
export interface ReportProps {
items: Result[];
empty: string;
passes: boolean;
type: RuleType;
}

export const Report: FunctionComponent<ReportProps> = ({ items, empty, type, passes }) => (
export const Report: FunctionComponent<ReportProps> = ({ items, empty, type }) => (
<Fragment>
{items && items.length ? (
items.map(item => <Item passes={passes} item={item} key={`${type}:${item.id}`} type={type} />)
items.map(item => <Item item={item} key={`${type}:${item.id}`} type={type} />)
) : (
<Placeholder key="placeholder">{empty}</Placeholder>
)}
Expand Down
Loading