[explore-v2] make control panel sections and fields more dense #1954

Merged
merged 3 commits into from Jan 12, 2017

Projects

None yet

2 participants

@ascott
Collaborator
ascott commented Jan 11, 2017 edited

fixes: #1941

before:
screenshot 2017-01-11 14 33 26

after:
screenshot 2017-01-11 14 52 17

plz review @airbnb/superset-reviewers @elibrumbaugh

ascott added some commits Jan 11, 2017
@ascott ascott make control panel sections and fields more dense
98175f0
@ascott ascott remove Panel
1d49ba7
- <Panel header={this.renderHeader()}>
- {this.props.children}
- </Panel>
+ <div className="panel panel-default control-panel-section">
@mistercrunch
mistercrunch Jan 12, 2017 Contributor

Wouldn't <Panel className="control-panel-section"> accomplish the same?

@ascott
ascott Jan 12, 2017 Collaborator

you're right. i didn't realize this component took a className as a prop. 👍

@@ -25,3 +25,8 @@
margin-top: 0px;
margin-right: 3px;
}
+
+.control-panel-section {
@mistercrunch
mistercrunch Jan 12, 2017 Contributor

I don't know much about css classing best practices but I was wondering why introducing a new class as opposed to using existing ones, as in this selector: .control-panel-section .panel (of course assuming that we'd create a class for control-panel-section in the parent)

@ascott
ascott Jan 12, 2017 Collaborator

if there was already a parent class, we could reference it the way you are describing, but since there is not i decided to make it more specific and make a new class for these control panel sections. there's not a big difference in approaches.

@ascott ascott use <Panel> with className prop
fb2547e
@mistercrunch
Contributor

LGTM

@ascott ascott merged commit fc74fbe into airbnb:master Jan 12, 2017

4 checks passed

code-quality/landscape Code quality remained the same
Details
codeclimate no new or fixed issues
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 79.779%
Details
@ascott ascott deleted the ascott:alanna-dense-controls branch Jan 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment