Skip to content
This repository has been archived by the owner on Sep 14, 2021. It is now read-only.

Commit

Permalink
feat(Highlight): Fix undefined class assigned to element (#572)
Browse files Browse the repository at this point in the history
Due to the `[styles[tone]]: true` defaulting tone to `TONE.NEUTRAL`, by default we always get `undefined` class being assigned to the element.

This is caused by the fact that `.neutral` class does not exist in the stylesheet.

Tone is NOT a required prop from the consumer's perspective and also defaulting is a) doing nothing, b) introducing incorrect behaviour of the component.
  • Loading branch information
kosanna authored Nov 1, 2018
1 parent 139e95b commit 1640d89
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 4 deletions.
2 changes: 1 addition & 1 deletion react/Highlight/Highlight.demo.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export default {
component: Text,
initialProps: {
headline: true,
children: renderChildren({ tone: TONE.NEUTRAL })
children: renderChildren({})
},
options: [
{
Expand Down
6 changes: 3 additions & 3 deletions react/Highlight/Highlight.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,19 @@ type Props = {
children: React$Node,
secondary?: boolean,
className?: string,
tone?: typeof TONE.CRITICAL | typeof TONE.NEUTRAL
tone?: typeof TONE.CRITICAL
};

export default function Highlight(props: Props) {
const { children, secondary, tone = TONE.NEUTRAL, className, ...restProps } = props;
const { children, secondary, tone, className, ...restProps } = props;

return (
<mark
{...restProps}
className={classnames({
[styles.root]: true,
[styles.secondaryText]: secondary,
[styles[tone]]: true,
[styles[tone]]: tone,
...(className ? { [className]: className } : {})
})}>
{children}
Expand Down
22 changes: 22 additions & 0 deletions react/Highlight/Highlight.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import React from 'react';
import { shallow } from 'enzyme';

import Highlight from './Highlight';

describe('Highlight:', () => {
it('should render with defaults', () => {
expect(shallow(<Highlight>text</Highlight>)).toMatchSnapshot();
});

it('should render with className', () => {
expect(shallow(<Highlight className="foo">text</Highlight>)).toMatchSnapshot();
});

it('should render with tone', () => {
expect(shallow(<Highlight tone="critical" className="foo">text</Highlight>)).toMatchSnapshot();
});

it('should render as secondary', () => {
expect(shallow(<Highlight secondary tone="critical" className="foo">text</Highlight>)).toMatchSnapshot();
});
});
33 changes: 33 additions & 0 deletions react/Highlight/__snapshots__/Highlight.test.js.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`Highlight: should render as secondary 1`] = `
<mark
className="Highlight__root Highlight__secondaryText Highlight__critical foo"
>
text
</mark>
`;

exports[`Highlight: should render with className 1`] = `
<mark
className="Highlight__root foo"
>
text
</mark>
`;

exports[`Highlight: should render with defaults 1`] = `
<mark
className="Highlight__root"
>
text
</mark>
`;

exports[`Highlight: should render with tone 1`] = `
<mark
className="Highlight__root Highlight__critical foo"
>
text
</mark>
`;

0 comments on commit 1640d89

Please sign in to comment.