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.

Sign up for GitHub

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

Jump to bottom

Move a few components to es6 with mixin decorator #2439

Merged
merged 1 commit into from
Dec 8, 2015
Merged

Move a few components to es6 with mixin decorator #2439

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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 package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
"babelify": "^6.1.3",
"browserify-istanbul": "^0.2.1",
"chai": "^3.2.0",
"core-decorators": "^0.9.1",
"eslint": "^1.8.0",
"eslint-loader": "^1.0.0",
"eslint-plugin-react": "^3.2.2",
Expand Down
71 changes: 35 additions & 36 deletions src/app-bar.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,27 +7,34 @@ import DefaultRawTheme from './styles/raw-themes/light-raw-theme';
import ThemeManager from './styles/theme-manager';
import Paper from './paper';
import PropTypes from './utils/prop-types';
import {autobind, mixin} from 'core-decorators';

const AppBar = React.createClass({
@mixin(StylePropable)
export default class AppBar extends React.Component {
constructor(props, context) {
super(props, context);

mixins: [StylePropable],
this.state = {
muiTheme: this.context.muiTheme ? this.context.muiTheme : ThemeManager.getMuiTheme(DefaultRawTheme),
};
}

contextTypes: {
//for passing default theme context to children
static childContextTypes = {
muiTheme: React.PropTypes.object,
},
}

//for passing default theme context to children
childContextTypes: {
static contextTypes = {
muiTheme: React.PropTypes.object,
},
}

getChildContext() {
return {
muiTheme: this.state.muiTheme,
};
},
static defaultProps = {
showMenuIconButton: true,
title: '',
zDepth: 1,
}

propTypes: {
static propTypes = {
/**
* Can be used to render a tab inside an app bar for instance.
*/
Expand Down Expand Up @@ -107,28 +114,20 @@ const AppBar = React.createClass({
* The shadow of the app bar is also dependent on this property.
*/
zDepth: PropTypes.zDepth,
},
}

getInitialState() {
getChildContext() {
return {
muiTheme: this.context.muiTheme ? this.context.muiTheme : ThemeManager.getMuiTheme(DefaultRawTheme),
muiTheme: this.state.muiTheme,
};
},
}

//to update theme inside state whenever a new theme is passed down
//from the parent / owner using context
componentWillReceiveProps(nextProps, nextContext) {
let newMuiTheme = nextContext.muiTheme ? nextContext.muiTheme : this.state.muiTheme;
this.setState({muiTheme: newMuiTheme});
},

getDefaultProps() {
return {
showMenuIconButton: true,
title: '',
zDepth: 1,
};
},
}

componentDidMount() {
if (process.env.NODE_ENV !== 'production') {
Expand All @@ -146,7 +145,7 @@ const AppBar = React.createClass({
);
}
}
},
}

getStyles() {
let spacing = this.state.muiTheme.rawTheme.spacing;
Expand Down Expand Up @@ -199,7 +198,7 @@ const AppBar = React.createClass({
};

return styles;
},
}

render() {
let {
Expand Down Expand Up @@ -314,26 +313,26 @@ const AppBar = React.createClass({
{children}
</Paper>
);
},
}

@autobind
Copy link
Member

Choose a reason for hiding this comment

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

Please use arrow functions instead of autobind.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Instead of the arrow functions I could alternatively just leave react-codemod alone; it automatically puts this bindings in the constructor. Which would you prefer?

Copy link
Member

Choose a reason for hiding this comment

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

Having a look at https://github.com/callemall/material-ui/pull/1734/files, I would say, let's let react-codemod puts this.

Copy link
Member

Choose a reason for hiding this comment

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

I'm sorry but I completely disagree! Doing that makes the components extremely hard to maintain! I had one hell of bad time with leftnav's doc page component. This is not a good pattern. and using arrow would make it look like this:

_foo = () => {
  // ... 
};

It's not so bad, is it?

Copy link
Member

Choose a reason for hiding this comment

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

@subjectix react-router seems to follow this convention too (https://github.com/rackt/react-router/blob/master/examples/huge-apps/components/GlobalNav.js).
I don't know why they and facebook do so, but I feel like we can trust them.

_onLeftIconButtonTouchTap(event) {
if (this.props.onLeftIconButtonTouchTap) {
this.props.onLeftIconButtonTouchTap(event);
}
},
}

@autobind
_onRightIconButtonTouchTap(event) {
if (this.props.onRightIconButtonTouchTap) {
this.props.onRightIconButtonTouchTap(event);
}
},
}

@autobind
_onTitleTouchTap(event) {
if (this.props.onTitleTouchTap) {
this.props.onTitleTouchTap(event);
}
},

});

export default AppBar;
}
}
46 changes: 22 additions & 24 deletions src/app-canvas.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,42 +2,43 @@ import React from 'react';
import StylePropable from './mixins/style-propable';
import DefaultRawTheme from './styles/raw-themes/light-raw-theme';
import ThemeManager from './styles/theme-manager';
import {mixin} from 'core-decorators';

const AppCanvas = React.createClass({
@mixin(StylePropable)
export default class AppCanvas extends React.Component {
constructor(props, context) {
super(props, context);

mixins: [StylePropable],
this.state = {
muiTheme: context.muiTheme ? context.muiTheme : ThemeManager.getMuiTheme(DefaultRawTheme),
};
}

contextTypes: {
//for passing default theme context to children
static childContextTypes = {
muiTheme: React.PropTypes.object,
},

propTypes: {
children: React.PropTypes.node,
},
}

//for passing default theme context to children
childContextTypes: {
static contextTypes = {
muiTheme: React.PropTypes.object,
},
}

static propTypes = {
children: React.PropTypes.node,
}

getChildContext() {
return {
muiTheme: this.state.muiTheme,
};
},

getInitialState() {
return {
muiTheme: this.context.muiTheme ? this.context.muiTheme : ThemeManager.getMuiTheme(DefaultRawTheme),
};
},
}

//to update theme inside state whenever a new theme is passed down
//from the parent / owner using context
componentWillReceiveProps(nextProps, nextContext) {
let newMuiTheme = nextContext.muiTheme ? nextContext.muiTheme : this.state.muiTheme;
this.setState({muiTheme: newMuiTheme});
},
}

render() {
let styles = {
Expand Down Expand Up @@ -69,8 +70,5 @@ const AppCanvas = React.createClass({
{newChildren}
</div>
);
},

});

export default AppCanvas;
}
}
57 changes: 27 additions & 30 deletions src/avatar.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,27 +3,34 @@ import StylePropable from './mixins/style-propable';
import Colors from './styles/colors';
import DefaultRawTheme from './styles/raw-themes/light-raw-theme';
import ThemeManager from './styles/theme-manager';
import {mixin} from 'core-decorators';

const Avatar = React.createClass({
@mixin(StylePropable)
export default class Avatar extends React.Component {
constructor(props, context) {
super(props, context);

mixins: [StylePropable],
this.state = {
muiTheme: context.muiTheme ? context.muiTheme : ThemeManager.getMuiTheme(DefaultRawTheme),
};
}

contextTypes: {
//for passing default theme context to children
static childContextTypes = {
muiTheme: React.PropTypes.object,
},
}

//for passing default theme context to children
childContextTypes: {
static contextTypes = {
muiTheme: React.PropTypes.object,
},
}

getChildContext() {
return {
muiTheme: this.state.muiTheme,
};
},
static defaultProps = {
backgroundColor: Colors.grey400,
color: Colors.white,
size: 40,
}

propTypes: {
static propTypes = {
/**
* The backgroundColor of the avatar. Does not apply to image avatars.
*/
Expand Down Expand Up @@ -63,28 +70,20 @@ const Avatar = React.createClass({
* Override the inline-styles of the root element.
*/
style: React.PropTypes.object,
},
}

getInitialState() {
getChildContext() {
return {
muiTheme: this.context.muiTheme ? this.context.muiTheme : ThemeManager.getMuiTheme(DefaultRawTheme),
muiTheme: this.state.muiTheme,
};
},
}

//to update theme inside state whenever a new theme is passed down
//from the parent / owner using context
componentWillReceiveProps(nextProps, nextContext) {
let newMuiTheme = nextContext.muiTheme ? nextContext.muiTheme : this.state.muiTheme;
this.setState({muiTheme: newMuiTheme});
},

getDefaultProps() {
return {
backgroundColor: Colors.grey400,
color: Colors.white,
size: 40,
};
},
}

render() {
let {
Expand Down Expand Up @@ -156,7 +155,5 @@ const Avatar = React.createClass({
</div>
);
}
},
});

export default Avatar;
}
}