Skip to content
This repository has been archived by the owner on Nov 16, 2023. It is now read-only.

Updating MessageBar to reuse other components and reduce custom CSS #87

Merged
merged 4 commits into from Nov 8, 2017
Merged

Updating MessageBar to reuse other components and reduce custom CSS #87

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1d444ef
Updating MessageBar to reuse other components and less custom CSS
swese44 Oct 30, 2017
86d61d0
revert readme
swese44 Nov 6, 2017
1115864
revert demos readme
swese44 Nov 6, 2017
4806c95
Merge branch 'master' into messagebar-update
swese44 Nov 7, 2017
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
Binary file modified config/styleguide-visual/reference/MessageBar_with_icon_action_desktop.png
View file
Open in desktop
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 0 additions & 1 deletion package.json
View file
Open in desktop
Expand Up @@ -18,7 +18,6 @@
"@types/prop-types": "=15.5.1",
"@types/react": "=16.0.2",
"@types/react-dom": "=15.5.2",
"autoprefixer": "=6.5.1",
"css-loader": "=0.27.3",
"enzyme": "=2.9.1",
"eslint": "=4.8.0",
Expand Down
1 change: 0 additions & 1 deletion postcss.config.js
View file
Open in desktop
Expand Up @@ -9,6 +9,5 @@ module.exports = () => ({
path: [''],
},
precss: {},
autoprefixer: {},
},
});
70 changes: 38 additions & 32 deletions src/components/Callout/Callout.md
View file
Open in desktop
Expand Up @@ -143,37 +143,43 @@ const HintedPositionCallout = (props) => (
);

<div style={{ height: '400px', display: 'flex', flexDirection: 'column', alignItems: 'center', justifyContent: 'center' }}>
<FixedGridRow>
<FixedGridColumn fixed={true} width={100}>
<HintedPositionCallout hint={DirectionalHint.topRightEdge} align="left" label="Top right" />
</FixedGridColumn>
<FixedGridColumn fixed={true} width={100}>
<HintedPositionCallout hint={DirectionalHint.topCenter} align="center" label="Top center" />
</FixedGridColumn>
<FixedGridColumn fixed={true} width={100}>
<HintedPositionCallout hint={DirectionalHint.topLeftEdge} align="right" label="Top left" />
</FixedGridColumn>
</FixedGridRow>
<FixedGridRow>
<FixedGridColumn fixed={true} width={100}>
<HintedPositionCallout hint={DirectionalHint.leftCenter} align="left" label="Left center" />
</FixedGridColumn>
<FixedGridColumn fixed={true} width={100}>
</FixedGridColumn>
<FixedGridColumn fixed={true} width={100}>
<HintedPositionCallout hint={DirectionalHint.rightCenter} align="right" label="Right center" />
</FixedGridColumn>
</FixedGridRow>
<FixedGridRow>
<FixedGridColumn fixed={true} width={100}>
<HintedPositionCallout hint={DirectionalHint.bottomRightEdge} align="left" label="Bottom right" />
</FixedGridColumn>
<FixedGridColumn fixed={true} width={100}>
<HintedPositionCallout hint={DirectionalHint.bottomCenter} align="center" label="Bottom center" />
</FixedGridColumn>
<FixedGridColumn fixed={true} width={100}>
<HintedPositionCallout hint={DirectionalHint.bottomLeftEdge} align="right" label="Bottom left" />
</FixedGridColumn>
</FixedGridRow>
<div>
<FixedGridRow>
<FixedGridColumn fixed={true} width={100}>
<HintedPositionCallout hint={DirectionalHint.topRightEdge} align="left" label="Top right" />
</FixedGridColumn>
<FixedGridColumn fixed={true} width={100}>
<HintedPositionCallout hint={DirectionalHint.topCenter} align="center" label="Top center" />
</FixedGridColumn>
<FixedGridColumn fixed={true} width={100}>
<HintedPositionCallout hint={DirectionalHint.topLeftEdge} align="right" label="Top left" />
</FixedGridColumn>
</FixedGridRow>
</div>
<div>
<FixedGridRow>
<FixedGridColumn fixed={true} width={100}>
<HintedPositionCallout hint={DirectionalHint.leftCenter} align="left" label="Left center" />
</FixedGridColumn>
<FixedGridColumn fixed={true} width={100}>
</FixedGridColumn>
<FixedGridColumn fixed={true} width={100}>
<HintedPositionCallout hint={DirectionalHint.rightCenter} align="right" label="Right center" />
</FixedGridColumn>
</FixedGridRow>
</div>
<div>
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are the extra divs necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

took a little head-scratching to figure out why the visual diffs were messed up for this story. basically i'd fixed a FixedGridRow bug via flex-basis: 100% because previously it would only fill its horizontal space if its column content pushed it out that far. in your example, your flex container's direct children were also display: flex and now have flex-basis: 100% so they filled the entire vertical space instead of being clustered in the center of the wrapper.

in general it's best to not use flexbox containers as direct children of another flexbox container, you should have another div or something to manage the flex-child layout then nest the next flex container under that

Copy link
Contributor

Choose a reason for hiding this comment

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

Aah I didn't stop to think that FixedGridRow was also display: flex.

<FixedGridRow>
<FixedGridColumn fixed={true} width={100}>
<HintedPositionCallout hint={DirectionalHint.bottomRightEdge} align="left" label="Bottom right" />
</FixedGridColumn>
<FixedGridColumn fixed={true} width={100}>
<HintedPositionCallout hint={DirectionalHint.bottomCenter} align="center" label="Bottom center" />
</FixedGridColumn>
<FixedGridColumn fixed={true} width={100}>
<HintedPositionCallout hint={DirectionalHint.bottomLeftEdge} align="right" label="Bottom left" />
</FixedGridColumn>
</FixedGridRow>
</div>
</div>
```
1 change: 1 addition & 0 deletions src/components/FixedGrid/FixedGrid.css
View file
Open in desktop
Expand Up @@ -3,6 +3,7 @@

.y-fixedGridRow {
display: flex;
flex-basis: 100%;
}

.y-fixedGridRow__bottomSpacing-xxLarge {
Expand Down
29 changes: 6 additions & 23 deletions src/components/MessageBar/MessageBar.css
View file
Open in desktop
@@ -1,36 +1,19 @@
/*! Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. */
@import "src/css/variables/colors.css";
@import "src/css/variables/fonts.css";

.y-message-bar {
display: flex;
background-color: $messageBarBackgroundColor;
border: solid 0 $alertColor__info;
border-radius: 1px;
border-left-width: 4px;
border-left-style: solid;
background-color: $messageBarBackgroundColor;
padding: 7px 12px 8px 12px;
font-size: $fontSize__small;
line-height: $lineHeight__small;
}

.y-message-bar--message {
flex-grow: 1;
color: $messageBarTextColor;
}

.y-message-bar--actions {
padding-left: 32px;
flex-grow: 0;
}

.y-message-bar__type-info {
border-left-color: $messageBarBorderColor__info;
color: $textColor;
padding: 7px 12px 8px;
}

.y-message-bar__type-warning {
border-left-color: $messageBarBorderColor__warning;
border-color: $alertColor__warning;
}

.y-message-bar__type-error {
border-left-color: $messageBarBorderColor__error;
border-color: $alertColor__error;
}
8 changes: 4 additions & 4 deletions src/components/MessageBar/MessageBar.md
View file
Open in desktop
Expand Up @@ -7,7 +7,7 @@ Text that is too long for the context will wrap to two or more lines, but you sh
Informational:

```js { "props": { "data-example": "basic" } }
<MessageBar ariaLabel="A sample informational message">
<MessageBar>
Something you should know about.
</MessageBar>
```
Expand All @@ -17,7 +17,7 @@ Warning:
```js { "props": { "data-example": "with type" } }
const { MessageBarType } = require('.');

<MessageBar ariaLabel="A sample warning message" type={MessageBarType.WARNING}>
<MessageBar type={MessageBarType.WARNING}>
Watch out, something unexpected might happen.
</MessageBar>
```
Expand All @@ -36,7 +36,7 @@ const icon = (
</span>
);

<MessageBar ariaLabel="A sample warning message" actions={icon} type={MessageBarType.WARNING}>
<MessageBar actions={icon} type={MessageBarType.WARNING}>
Watch out, something unexpected might happen.
</MessageBar>
```
Expand All @@ -48,7 +48,7 @@ const { MessageBarType } = require('.');

const link = <FakeLink>Retry</FakeLink>;

<MessageBar ariaLabel="A sample error message" actions={link} type={MessageBarType.ERROR}>
<MessageBar actions={link} type={MessageBarType.ERROR}>
Oh no, something bad happened and we couldn't complete your action!
</MessageBar>
```
10 changes: 5 additions & 5 deletions src/components/MessageBar/MessageBar.test.tsx
View file
Open in desktop
Expand Up @@ -4,12 +4,12 @@ import { shallow, ShallowWrapper } from 'enzyme';
import MessageBar, { MessageBarType, MessageBarProps } from './index';
import FakeLink from '../../components/FakeLink';

describe('<FakeLink />', () => {
describe('<MessageBar />', () => {
let component: ShallowWrapper<MessageBarProps, {}>;

describe('with default options', () => {
beforeEach(() => {
component = shallow(<MessageBar ariaLabel="label">content</MessageBar>);
component = shallow(<MessageBar>content</MessageBar>);
});

it('has its correct base class', () => {
Expand All @@ -24,7 +24,7 @@ describe('<FakeLink />', () => {
describe('with additional className', () => {
beforeEach(() => {
component = shallow(
<MessageBar ariaLabel="label" className="TEST_CLASSNAME">
<MessageBar className="TEST_CLASSNAME">
content
</MessageBar>,
);
Expand All @@ -46,7 +46,7 @@ describe('<FakeLink />', () => {
describe('with a type', () => {
beforeEach(() => {
component = shallow(
<MessageBar ariaLabel="label" type={MessageBarType.WARNING}>
<MessageBar type={MessageBarType.WARNING}>
content
</MessageBar>,
);
Expand All @@ -61,7 +61,7 @@ describe('<FakeLink />', () => {
beforeEach(() => {
const link = <FakeLink>Link</FakeLink>;
component = shallow(
<MessageBar ariaLabel="label" actions={link}>
<MessageBar actions={link}>
content
</MessageBar>,
);
Expand Down
30 changes: 10 additions & 20 deletions src/components/MessageBar/MessageBar.tsx
View file
Open in desktop
Expand Up @@ -3,15 +3,13 @@ import '../../yamui';
import * as React from 'react';
import { NestableBaseComponentProps } from '../../util/BaseComponent/props';
import { MessageBarType } from './enums';
import Block, { TextSize } from '../Block';
import { FixedGridRow, FixedGridColumn, GutterSize } from '../FixedGrid';
import './MessageBar.css';

export { MessageBarType };

export interface MessageBarProps extends NestableBaseComponentProps {
/**
* Additional label that must be provided for screenreaders.
*/
ariaLabel: string;

/**
* Type of message being displayed.
Expand All @@ -35,34 +33,26 @@ export default class MessageBar extends React.PureComponent<MessageBarProps, {}>
};

render() {
const { ariaLabel, children } = this.props;
const { actions, children } = this.props;

return (
<div className={this.getClasses()} aria-label={ariaLabel}>
<div className="y-message-bar--message">{children}</div>
{this.getActions()}
</div>
<Block textSize={TextSize.SMALL} className={this.getClasses()}>
<FixedGridRow gutterSize={GutterSize.XXLARGE}>
<FixedGridColumn>{children}</FixedGridColumn>
{actions && <FixedGridColumn fixed={true}>{actions}</FixedGridColumn>}
</FixedGridRow>
</Block>
);
}

private getClasses() {
const { className, type } = this.props;
const classes = ['y-message-bar', `y-message-bar__type-${type}`];

const classes: string[] = ['y-message-bar', `y-message-bar__type-${type}`];
if (className) {
classes.push(className);
}

return classes.join(' ');
}

private getActions() {
const { actions } = this.props;

if (!actions) {
return null;
}

return <div className="y-message-bar--actions">{actions}</div>;
}
}
86 changes: 47 additions & 39 deletions src/components/MessageBar/__snapshots__/MessageBar.test.tsx.snap
View file
Open in desktop
@@ -1,60 +1,68 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`<FakeLink /> with a type matches its snapshot 1`] = `
<div
aria-label="label"
exports[`<MessageBar /> with a type matches its snapshot 1`] = `
<Block
className="y-message-bar y-message-bar__type-warning"
textSize="small"
>
<div
className="y-message-bar--message"
<FixedGridRow
gutterSize="xxLarge"
>
content
</div>
</div>
<FixedGridColumn>
content
</FixedGridColumn>
</FixedGridRow>
</Block>
`;

exports[`<FakeLink /> with actions matches its snapshot 1`] = `
<div
aria-label="label"
exports[`<MessageBar /> with actions matches its snapshot 1`] = `
<Block
className="y-message-bar y-message-bar__type-info"
textSize="small"
>
<div
className="y-message-bar--message"
<FixedGridRow
gutterSize="xxLarge"
>
content
</div>
<div
className="y-message-bar--actions"
>
<FakeLink>
Link
</FakeLink>
</div>
</div>
<FixedGridColumn>
content
</FixedGridColumn>
<FixedGridColumn
fixed={true}
>
<FakeLink>
Link
</FakeLink>
</FixedGridColumn>
</FixedGridRow>
</Block>
`;

exports[`<FakeLink /> with additional className matches its snapshot 1`] = `
<div
aria-label="label"
exports[`<MessageBar /> with additional className matches its snapshot 1`] = `
<Block
className="y-message-bar y-message-bar__type-info TEST_CLASSNAME"
textSize="small"
>
<div
className="y-message-bar--message"
<FixedGridRow
gutterSize="xxLarge"
>
content
</div>
</div>
<FixedGridColumn>
content
</FixedGridColumn>
</FixedGridRow>
</Block>
`;

exports[`<FakeLink /> with default options matches its snapshot 1`] = `
<div
aria-label="label"
exports[`<MessageBar /> with default options matches its snapshot 1`] = `
<Block
className="y-message-bar y-message-bar__type-info"
textSize="small"
>
<div
className="y-message-bar--message"
<FixedGridRow
gutterSize="xxLarge"
>
content
</div>
</div>
<FixedGridColumn>
content
</FixedGridColumn>
</FixedGridRow>
</Block>
`;