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

[RFR] BooleanField accessibility #2744

Merged
merged 4 commits into from
Jan 15, 2019
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions docs/Fields.md
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,18 @@ import { BooleanField } from 'react-admin';

![BooleanField](./img/boolean-field.png)

The `BooleanField` also includes an hidden text for accessibility (or to query in end to end tests). By default, it includes the translated label and the translated value, for example `Published: false`.

If you need to override it, you can use the `valueLabelTrue` and `valueLabelFalse` props which both accept a string. Those strings may be translation keys:

```jsx
// Simple texts
<BooleanField source="published" valueLabelTrue="Has been published" valueLabelFalse="Has not been published yet" />

// Translation keys
<BooleanField source="published" valueLabelTrue="myapp.published.true" valueLabelFalse="myapp.published.false" />
```

## `<ChipField>`

Displays a value inside a ["Chip"](http://www.material-ui.com/#/components/chip), which is Material UI's term for a label.
Expand Down
12 changes: 2 additions & 10 deletions packages/ra-core/src/util/FieldTitle.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import pure from 'recompose/pure';
import compose from 'recompose/compose';

import translate from '../i18n/translate';
import getFieldLabelTranslationArgs from './getFieldLabelTranslationArgs';

export const FieldTitle = ({
resource,
Expand All @@ -14,16 +15,7 @@ export const FieldTitle = ({
translate,
}) => (
<span>
{typeof label !== 'undefined'
? translate(label, { _: label })
: typeof source !== 'undefined'
? translate(`resources.${resource}.fields.${source}`, {
_: inflection.transform(source, [
'underscore',
'humanize',
]),
})
: ''}
{translate(...getFieldLabelTranslationArgs({ label, resource, source }))}
{isRequired && ' *'}
</span>
);
Expand Down
29 changes: 29 additions & 0 deletions packages/ra-core/src/util/getFieldLabelTranslationArgs.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import inflection from 'inflection';

/**
* Returns an array of arguments to use with the translate function for the label of a field.
* The label will be the one specified by the label prop or one computed from the resource and source props.
*
* Usage:
* <span>
* {translate(...getFieldLabelTranslationArgs(label, resource, source))}
* </span>
*/
export default options => {
if (!options) {
return [''];
}

const { label, resource, source } = options;

return typeof label !== 'undefined'
? [label, { _: label }]
: typeof source !== 'undefined'
? [`resources.${resource}.fields.${source}`, {
_: inflection.transform(source, [
'underscore',
'humanize',
])
}]
: [''];
};
58 changes: 58 additions & 0 deletions packages/ra-core/src/util/getFieldLabelTranslationArgs.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
import assert from 'assert';
import getFieldLabelTranslationArgs from './getFieldLabelTranslationArgs';

describe('getFieldLabelTranslationArgs', () => {
it('should return empty span by default', () =>
assert.deepEqual(getFieldLabelTranslationArgs(), ['']));

it('should return the label when given', () =>
assert.deepEqual(
getFieldLabelTranslationArgs({
label: 'foo',
resource: 'posts',
source: 'title',
}),
['foo', { _: 'foo' }]
));

it('should return the humanized source when given', () => {
assert.deepEqual(
getFieldLabelTranslationArgs({
resource: 'posts',
source: 'title',
}),
[`resources.posts.fields.title`, { _: 'Title' }]
);

assert.deepEqual(
getFieldLabelTranslationArgs({
resource: 'posts',
source: 'title_with_underscore',
}),
[
`resources.posts.fields.title_with_underscore`,
{ _: 'Title with underscore' },
]
);

assert.deepEqual(
getFieldLabelTranslationArgs({
resource: 'posts',
source: 'titleWithCamelCase',
}),
[
`resources.posts.fields.titleWithCamelCase`,
{ _: 'Title with camel case' },
]
);
});

it('should return the source and resource as translate key', () =>
assert.deepEqual(
getFieldLabelTranslationArgs({
resource: 'posts',
source: 'title',
}),
[`resources.posts.fields.title`, { _: 'Title' }]
));
});
2 changes: 2 additions & 0 deletions packages/ra-core/src/util/index.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import downloadCSV from './downloadCSV';
import FieldTitle from './FieldTitle';
import getFetchedAt from './getFetchedAt';
import getFieldLabelTranslationArgs from './getFieldLabelTranslationArgs';
import HttpError from './HttpError';
import linkToRecord from './linkToRecord';
import removeEmpty from './removeEmpty';
Expand All @@ -15,6 +16,7 @@ export {
downloadCSV,
FieldTitle,
getFetchedAt,
getFieldLabelTranslationArgs,
HttpError,
linkToRecord,
removeEmpty,
Expand Down
64 changes: 60 additions & 4 deletions packages/ra-ui-materialui/src/field/BooleanField.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,31 +5,76 @@ import pure from 'recompose/pure';
import FalseIcon from '@material-ui/icons/Clear';
import TrueIcon from '@material-ui/icons/Done';
import Typography from '@material-ui/core/Typography';
import { createStyles, withStyles } from '@material-ui/core/styles';
import compose from 'recompose/compose';
import { translate } from 'ra-core';

import sanitizeRestProps from './sanitizeRestProps';

export const BooleanField = ({ className, source, record = {}, ...rest }) => {
if (get(record, source) === false) {
const styles = createStyles({
label: {
// Move the text out of the flow of the container.
position: 'absolute',

// Reduce its height and width to just one pixel.
height: 1,
width: 1,

// Hide any overflowing elements or text.
overflow: 'hidden',

// Clip the box to zero pixels.
clip: 'rect(0, 0, 0, 0)',

// Text won't wrap to a second line.
whiteSpace: 'nowrap',
}
});

export const BooleanField = ({
className,
classes,
source,
record = {},
translate,
valueLabelTrue,
valueLabelFalse,
...rest
}) => {
const value = get(record, source);
let ariaLabel = value
? valueLabelTrue
: valueLabelFalse;

if (!ariaLabel) {
ariaLabel = value === false
? translate('ra.boolean.false')
: translate('ra.boolean.true');
}

if (value === false) {
return (
<Typography
component="span"
body1="body1"
className={className}
{...sanitizeRestProps(rest)}
>
<span className={classes.label}>{ariaLabel}</span>
Copy link
Member

Choose a reason for hiding this comment

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

why do we need this span since we have an aria-label?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I must say I'm not sure about the best practices here. I think label elements should only be used with interactive elements such as inputs or buttons. It made sense for me that it was true for aria-label attributes: https://w3c.github.io/using-aria/#label-support

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I removed the aria-label attribute

<FalseIcon />
</Typography>
);
}

if (get(record, source) === true) {
if (value === true) {
return (
<Typography
component="span"
body1="body1"
className={className}
{...sanitizeRestProps(rest)}
>
<span className={classes.label}>{ariaLabel}</span>
<TrueIcon />
</Typography>
);
Expand All @@ -55,9 +100,20 @@ BooleanField.propTypes = {
record: PropTypes.object,
sortBy: PropTypes.string,
source: PropTypes.string.isRequired,
valueLabelTrue: PropTypes.string,
valueLabelFalse: PropTypes.string,
};

BooleanField.defaultProps = {
classes: {},
translate: x => x,
};

const PureBooleanField = pure(BooleanField);
const PureBooleanField = compose(
pure,
withStyles(styles),
translate,
)(BooleanField);

PureBooleanField.defaultProps = {
addLabel: true,
Expand Down
28 changes: 24 additions & 4 deletions packages/ra-ui-materialui/src/field/BooleanField.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,41 @@ import { shallow } from 'enzyme';
import { BooleanField } from './BooleanField';

describe('<BooleanField />', () => {
it('should display tick if value is true', () => {
it('should display tick and truthy text if value is true', () => {
const wrapper = shallow(
<BooleanField record={{ published: true }} source="published" />
<BooleanField record={{ published: true }} source="published" resource="posts" />
);
assert.ok(wrapper.first().is('WithStyles(Typography)'));
assert.equal(wrapper.first().find('pure(Done)').length, 1);
assert.equal(wrapper.first().find('span').text(), 'ra.boolean.true');
});

it('should display cross if value is false', () => {
it('should display tick and custom truthy text if value is true', () => {
const wrapper = shallow(
<BooleanField record={{ published: false }} source="published" />
<BooleanField record={{ published: true }} source="published" resource="posts" valueLabelTrue="Has been published" />
);
assert.ok(wrapper.first().is('WithStyles(Typography)'));
assert.equal(wrapper.first().find('pure(Done)').length, 1);
assert.equal(wrapper.first().find('span').text(), 'Has been published');
});

it('should display cross and falsy text if value is false', () => {
const wrapper = shallow(
<BooleanField record={{ published: false }} source="published" resource="posts" />
);

assert.ok(wrapper.first().is('WithStyles(Typography)'));
assert.equal(wrapper.first().find('pure(Clear)').length, 1);
assert.equal(wrapper.first().find('span').text(), 'ra.boolean.false');
});

it('should display tick and custom falsy text if value is true', () => {
const wrapper = shallow(
<BooleanField record={{ published: false }} source="published" resource="posts" valueLabelFalse="Has not been published yet" />
);
assert.ok(wrapper.first().is('WithStyles(Typography)'));
assert.equal(wrapper.first().find('pure(Clear)').length, 1);
assert.equal(wrapper.first().find('span').text(), 'Has not been published yet');
});

it('should not display anything if value is null', () => {
Expand Down