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

Connection UI cleanups & tests for prior PR #4020

Merged
merged 3 commits into from
Jan 4, 2017
Merged

Conversation

jacogr
Copy link
Contributor

@jacogr jacogr commented Jan 3, 2017

@jacogr jacogr added A0-pleasereview 🤓 Pull request needs code review. M7-ui labels Jan 3, 2017
}

export default connect(
mapStateToProps,
mapDispatchToProps
null
Copy link
Contributor

Choose a reason for hiding this comment

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

This line (null) can just be omitted.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I prefer to be explicit, hence putting that in there.

@@ -17,7 +17,6 @@
import React, { Component, PropTypes } from 'react';
import { FormattedMessage } from 'react-intl';
import { connect } from 'react-redux';
import { bindActionCreators } from 'redux';
import ActionCompareArrows from 'material-ui/svg-icons/action/compare-arrows';
import ActionDashboard from 'material-ui/svg-icons/action/dashboard';
import HardwareDesktopMac from 'material-ui/svg-icons/hardware/desktop-mac';
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we use the new ui/Icons exports there while touching this file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, probably, yes. However I left those in there on purpose since the icons they are actually non-button icons where the rest are all meant for buttons.

@ngotchac ngotchac added A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jan 4, 2017
@jacogr jacogr merged commit cc8e200 into master Jan 4, 2017
@jacogr jacogr deleted the jg-connection-check branch January 4, 2017 14:15
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants