-
Notifications
You must be signed in to change notification settings - Fork 14.4k
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
Added controls for Table Viz #1253
Conversation
c82f500
to
38753a3
Compare
changeMetrics(metricsOpts) { | ||
this.props.actions.setMetrics(metricsOpts); | ||
changeMetrics(metrics) { | ||
this.props.actions.setMetrics(metrics); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change the variable names here to avoid confusion: Opts variables are for dropdown menu values, non-Opts values are for selected ones.
@@ -43,7 +43,7 @@ describe('reducers', () => { | |||
}); | |||
|
|||
it('should return new state with time stamp', () => { | |||
const newState = exploreReducer(initialState, actions.setTimeStamp(1)); | |||
const newState = exploreReducer(initialState, actions.setTimeStampFormat(1)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change function name here to match with the variable name in store
</div> | ||
); | ||
|
||
export const VIZ_CONTROL_MAPPING = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The viz-control panel mapping is stored here
vizType: null, | ||
}; | ||
|
||
class ControlPanelsContainer extends React.Component { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Getting linting error here saying component should be written as a pure function, but in stateless components 'this.props' can't be accessed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typically if you write a pure function you do so like:
function ControlPanelsContainer(props) { ... }
and so you access props
the same way you would acces this.props
if it were a class.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ohhh I see...Thanks Chris!!
looks good! 💯 |
looks like you need to rebase master to fix the conflicts in views.py |
9a4d3e1
to
2a6e30b
Compare
Solved and merged! |
This reverts commit bb10f9e1fefce597a4ab87580d78f712f2c7cd77.
This reverts commit bb10f9e1fefce597a4ab87580d78f712f2c7cd77.
This reverts commit bb10f9e1fefce597a4ab87580d78f712f2c7cd77.
This reverts commit bb10f9e1fefce597a4ab87580d78f712f2c7cd77.
Done:
@ascott
Todo: