Skip to content

Conversation

@vera-liu
Copy link
Contributor

@vera-liu vera-liu commented Oct 4, 2016

Done:

  • Added NotGroupBy and Options in control for Table Viz
    screen shot 2016-10-04 at 4 24 22 pm

@ascott
Todo:

  • tests

@vera-liu vera-liu force-pushed the vliu_table_controls branch 3 times, most recently from c82f500 to 38753a3 Compare October 5, 2016 00:57
changeMetrics(metricsOpts) {
this.props.actions.setMetrics(metricsOpts);
changeMetrics(metrics) {
this.props.actions.setMetrics(metrics);
Copy link
Contributor Author

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));
Copy link
Contributor Author

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 = {
Copy link
Contributor Author

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 {
Copy link
Contributor Author

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

Copy link
Contributor

@williaster williaster Oct 5, 2016

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.

Copy link
Contributor Author

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!!

@ascott
Copy link

ascott commented Oct 5, 2016

looks good! 💯

@ascott
Copy link

ascott commented Oct 5, 2016

looks like you need to rebase master to fix the conflicts in views.py

@vera-liu vera-liu force-pushed the vliu_table_controls branch from 9a4d3e1 to 2a6e30b Compare October 5, 2016 21:27
@vera-liu vera-liu merged commit 66b498d into apache:master Oct 5, 2016
@vera-liu
Copy link
Contributor Author

vera-liu commented Oct 5, 2016

Solved and merged!

@vera-liu vera-liu deleted the vliu_table_controls branch November 1, 2016 19:05
zhaoyongjie pushed a commit to zhaoyongjie/incubator-superset that referenced this pull request Nov 17, 2021
This reverts commit bb10f9e1fefce597a4ab87580d78f712f2c7cd77.
zhaoyongjie pushed a commit to zhaoyongjie/incubator-superset that referenced this pull request Nov 24, 2021
This reverts commit bb10f9e1fefce597a4ab87580d78f712f2c7cd77.
zhaoyongjie pushed a commit to zhaoyongjie/incubator-superset that referenced this pull request Nov 25, 2021
This reverts commit bb10f9e1fefce597a4ab87580d78f712f2c7cd77.
zhaoyongjie pushed a commit to zhaoyongjie/incubator-superset that referenced this pull request Nov 26, 2021
This reverts commit bb10f9e1fefce597a4ab87580d78f712f2c7cd77.
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.12.0 First shipped in 0.12.0 labels Feb 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.12.0 First shipped in 0.12.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants