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

[AppBar] Add iconStyleLeft prop #3668

Closed
wants to merge 1 commit into from
Closed
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
14 changes: 11 additions & 3 deletions src/app-bar.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import Paper from './paper';
import PropTypes from './utils/prop-types';
import warning from 'warning';

function getStyles(props, state) {
export function getStyles(props, state) {
const {
appBar,
button: {
Expand Down Expand Up @@ -98,6 +98,11 @@ const AppBar = React.createClass({
*/
iconElementRight: React.PropTypes.element,

/**
* Override the inline-styles of the element displayed on the left side of the app bar.
*/
iconStyleLeft: React.PropTypes.object,

/**
* Override the inline-styles of the element displayed on the right side of the app bar.
*/
Expand Down Expand Up @@ -216,6 +221,7 @@ const AppBar = React.createClass({
const {
title,
titleStyle,
iconStyleLeft,
iconStyleRight,
showMenuIconButton,
iconElementLeft,
Expand Down Expand Up @@ -247,6 +253,8 @@ const AppBar = React.createClass({
style: prepareStyles(Object.assign(styles.title, styles.mainElement, titleStyle)),
}, title);

const iconLeftStyle = Object.assign({}, styles.iconButtonStyle, iconStyleLeft);
Copy link
Member

Choose a reason for hiding this comment

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

you shouldn't do this either as it's resulting in calling prepareStyles twice. Please follow my comments as they are 😁


if (showMenuIconButton) {
let iconElementLeftNode = iconElementLeft;

Expand All @@ -260,15 +268,15 @@ const AppBar = React.createClass({
}

menuElementLeft = (
<div style={prepareStyles(Object.assign({}, styles.iconButtonStyle))}>
<div style={prepareStyles(iconLeftStyle)}>
Copy link
Member

Choose a reason for hiding this comment

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

Might this be a regression? Why even change these?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

iconRightStyle will change the style of menuElementRight.
iconLeftStyle should also be similar.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, but this is a breaking change. you shouldn't remove the styles.iconButtonStyle just override it: Object.assign({}, styles.iconButtonStyle, iconLeftStyle)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, I'm sorry. Including a test, and re-commit again at a later date. 😵

{iconElementLeftNode}
</div>
);
} else {
const child = iconClassNameLeft ? '' : <NavigationMenu style={Object.assign({}, styles.iconButtonIconStyle)} />;
menuElementLeft = (
<IconButton
style={styles.iconButtonStyle}
style={iconLeftStyle}
Copy link
Member

Choose a reason for hiding this comment

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

ditto

Copy link
Member

Choose a reason for hiding this comment

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

Object.assign({}, styles.iconButtonStyle, iconLeftStyle)

iconStyle={styles.iconButtonIconStyle}
iconClassName={iconClassNameLeft}
onTouchTap={this._onLeftIconButtonTouchTap}
Expand Down
37 changes: 36 additions & 1 deletion test/unit/AppBar.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import React from 'react';
import sinon from 'sinon';
import {shallow} from 'enzyme';
import {assert} from 'chai';
import AppBar from 'src/app-bar';
import AppBar, {getStyles} from 'src/app-bar';
import IconButton from 'src/icon-button';
import Paper from 'src/paper';
import NavigationClose from 'src/svg-icons/navigation/close';
Expand Down Expand Up @@ -169,4 +169,39 @@ describe('<AppBar />', () => {

assert.equal(wrapper.find(Paper).get(0).props.zDepth, 2, 'should have zDepth to 2');
});

it('menuElementLeft\'s style should be iconButtonStyle', () => {
const wrapper = shallow(
<AppBar />
);

const menuElementLeft = wrapper.find(IconButton).get(0);
const style = menuElementLeft.props.style;
const iconButtonStyle = getStyles(wrapper.props(), wrapper.state()).iconButtonStyle;
assert.deepEqual(style, iconButtonStyle, 'style should be iconButtonStyle');
});

it('if pass iconStyleLeft={marginTop}, change the marginTop only', () => {
const wrapper = shallow(
<AppBar iconStyleLeft={{marginTop: 99}} />
);

const menuElementLeft = wrapper.find(IconButton).get(0);
const style = menuElementLeft.props.style;
const iconButtonStyle = getStyles(wrapper.props(), wrapper.state()).iconButtonStyle;
const expectedStyle = Object.assign({}, iconButtonStyle, {marginTop: 99});
assert.deepEqual(style, expectedStyle, 'should be change style.marginTop only');
});

it('if pass iconElementLeft and iconStyleLeft={marginTop}, change the marginTop only', () => {
const wrapper = shallow(
<AppBar iconElementLeft={<span>foo</span>} iconStyleLeft={{marginTop: 99}} />
);

const menuElementLeft = wrapper.find('div').get(0);
const style = menuElementLeft.props.style;
const iconButtonStyle = getStyles(wrapper.props(), wrapper.state()).iconButtonStyle;
const expectedStyle = Object.assign({}, iconButtonStyle, {marginTop: 99});
assert.deepEqual(style, expectedStyle, 'should be change style.marginTop only');
});
});