Removes styler code #1250
Removes styler code #1250
Conversation
static_src/components/panel_row.jsx
Outdated
| import style from 'cloudgov-style/css/cloudgov-style.css'; | ||
|
|
||
| import createStyler from '../util/create_styler'; | ||
| import classnames from 'classnames'; |
There was a problem hiding this comment.
I think the accepted convention is to import classNames from 'classnames'; (i.e. camelCase classNames).
static_src/components/panel_row.jsx
Outdated
| const styleClass = props.styleClass && `panel-row-${this.props.styleClass}`; | ||
| const mainClass = props.styleClass !== 'boxed' && 'panel-row'; | ||
| const { props } = this; | ||
| const classes = classnames({ |
There was a problem hiding this comment.
Were you also meant to pass this.props.className to this?
| render() { | ||
| const descClass = this.props.description && 'panel-documentation-desc'; | ||
|
|
||
| return ( | ||
| <div className={ this.styler('panel-documentation', descClass) }> | ||
| <div className={ `panel-documentation ${descClass}` }> |
There was a problem hiding this comment.
Not sure if you want to use classNames here to remove the tmp var?
<div className={classNames('panel-documentation', { 'panel-documentation-desc': this.props.description })}>| render() { | ||
| const alignClass = `panel-actions-${this.props.align}`; | ||
| return ( | ||
| <span className={ this.styler('panel-actions', alignClass) }> | ||
| <span className={ `panel-actions ${alignClass}` }> |
There was a problem hiding this comment.
Maybe remove the alignClass tmp var and just put panel-actions-${this.props.align} straight in the string.
| <div className={ this.styler('notification', statusClass, 'test-notification') }> | ||
| <div className={ this.styler('notification-wrap') }> | ||
| <p className={ this.styler('notification-message', 'test-notification-message') }> | ||
| <div className={ `notification ${statusClass} test-notification` }> |
There was a problem hiding this comment.
Maybe put statusClass contents straight in string and remove tmp var?
| render() { | ||
| const props = this.props; | ||
| const alignClass = `elastic_line-item-${props.align}`; | ||
|
|
||
| return ( | ||
| <div className={ this.styler('elastic_line-item', alignClass) }> | ||
| <div className={ `elastic_line-item ${alignClass}`}> |
| this.state = {}; | ||
| this.styler = createStyler(style); | ||
| } | ||
|
|
||
| render() { | ||
| const props = this.props; |
There was a problem hiding this comment.
Not really related, but if you're here, might want to remove props tmp var - not really conventional to pull out this.props into a var.
| render() { | ||
| const props = this.props; | ||
| const statusClass = `count_status-${props.health.toLowerCase()}`; | ||
|
|
||
| return ( | ||
| <div className={ this.styler('count_status', statusClass) }> | ||
| <div className={ this.styler('count_status-icon') }> | ||
| <div className={`count_status ${statusClass}`}> |
| { title } | ||
| </h4> | ||
| </header> | ||
| ); | ||
| } | ||
|
|
||
| return ( | ||
| <div className={ [this.props.className, this.styler('complex_list')].join(' ') }> | ||
| <div className={ [this.props.className, 'complex_list'].join(' ') }> |
There was a problem hiding this comment.
Use classNames to do the joining? Also because this.props.className might be falsey so this could render undefined complex_list right?
| @@ -56,7 +48,7 @@ export default class AppQuicklook extends React.Component { | |||
| const statusClass = !isHealthyApp(app) && 'status-error'; | |||
|
|
|||
| return ( | |||
| <a className={ this.styler(statusClass) } href={ this.appHref() }> | |||
| <a className={ statusClass } href={ this.appHref() }> | |||
There was a problem hiding this comment.
Use classNames inline? Makes the semantics of !isHealthyApp(app) && 'status-error' clearer:
<a className={classNames({ 'status-error': !isHealthyApp(app) })} />
|
Yep, I was planning on going back in and using the |
el-mapache
force-pushed
the
ab-remove-styler
branch
from
8add0ce to
935af3a
Compare
|
LGTM ⭐️ |
I think that any place we used the styler we can just leverage the
classnamespackages directly.That package also provides the de-dupe functionality that the styler added, and now we don't have to include the styler object in every single component.