Skip to content
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

New examples for responsive resize, custom styling, and tooltips #59

Merged
merged 7 commits into from
Oct 10, 2016

Conversation

wcjordan
Copy link
Member

@wcjordan wcjordan commented Oct 9, 2016

Description

New examples for responsive resize, custom styling, and tooltips based on feature requests / suggestions in issues.
Also cleans up and standardizes examples and switches the to using pure components for performance.

Motivation and Context

This helps onboard new users, as it provides examples for common requests. It resolves commonly opened issues without a need for changes integrated directly into the library.

How Has This Been Tested?

Built the examples and tested them locally on my laptop.

Screenshots (if appropriate):

screen shot 2016-10-09 at 4 14 41 pm

![tooltip](https://cloud.githubusercontent.com/assets/1034455/19223385/44c3555e-8e3c-11e6-979b-c60eca27a0c4.jpg) ## Types of changes
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.

header={<Cell>First Name</Cell>}
cell={<TextCell data={dataList} col="firstName" />}
Copy link
Member Author

Choose a reason for hiding this comment

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

moved these to columnKey which I believe is more inline with best practices

this._callback = callback;
}

getDataVersion() {
Copy link
Member Author

Choose a reason for hiding this comment

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

Introduced a version field similar to what we do on LiveDesign so I could make the renderers pure for this example and get better performance

@@ -44,7 +28,7 @@ class ReorderExample extends React.Component {
super(props);

this.state = {
dataList: new FakeObjectDataListStore(1000000),
dataList: new FakeObjectDataListStore(10000),
Copy link
Member Author

@wcjordan wcjordan Oct 9, 2016

Choose a reason for hiding this comment

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

I did this because this example is really jank with dev React (but fine with prod React). I can revert though if desired.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we revert and use production react?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ya already using production react, but will switch this number back. I had just noticed the jankiness while dev'ing

@@ -27,7 +26,7 @@ class SortHeaderCell extends React.Component {
}

render() {
var {sortDir, children, ...props} = this.props;
var {onSortChange, sortDir, children, ...props} = this.props;
Copy link
Member Author

Choose a reason for hiding this comment

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

const React = require('react');
const {StyleSheet, css} = require('aphrodite');

class StylingExample extends React.Component {
Copy link
Member Author

@wcjordan wcjordan Oct 9, 2016

Choose a reason for hiding this comment

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

Hopefully this example helps with issues like #51 and #47

I found some limitations though. The 3 wraps on the default cell make styling it hard. I couldn't style the border of FixedDataTableCellDefault. Also when the table width > total column width, I couldn't figure out how to style the extra header padding.

@@ -1,74 +0,0 @@
/**
Copy link
Member Author

Choose a reason for hiding this comment

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

I deleted all the old examples folder. It was unused.

@@ -8,6 +8,7 @@
"react-dom": ">=0.14.0 || ^0.14.0-beta3"
},
"devDependencies": {
Copy link
Member Author

Choose a reason for hiding this comment

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

Does it make sense for example dependencies to be tracked as devDependencies? Should React be tracked as a devDependency as well?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not 100% sure. Doesn't NPM install these when someone uses these libs? Seems unnecessary if thats the case

Copy link
Member Author

Choose a reason for hiding this comment

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

I ran an experiment and did not see these installed when installing FDT from another repo.

It doesn't install any devDependencies transitively. See http://stackoverflow.com/questions/18875674/whats-the-difference-between-dependencies-devdependencies-and-peerdependencies

@@ -22,6 +22,7 @@ var ExampleHeader = require('./ExampleHeader');
var ExamplesWrapper = require('./ExamplesWrapper');
var React = require('react');
var Constants = require('../Constants');
const Dimensions = require('react-dimensions');
Copy link
Member Author

Choose a reason for hiding this comment

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

Switched the examples page to use react-dimensions as well to make life easier.

One downside is react-dimensions now spits out this warning on all the examples pages
digidem/react-dimensions#35

@@ -6,6 +6,7 @@

Copy link
Member Author

Choose a reason for hiding this comment

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

needed for the change to react-dimensions for responsive styling of the examples page

* Below that entry point the user is welcome to consume or
* pass the prop through at their discretion.
*/
rowIndex: PropTypes.number
Copy link
Member Author

Choose a reason for hiding this comment

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

fixes another issue with unknown propTypes in our examples

};
module.exports.TextCell = TextCell;


Copy link
Contributor

Choose a reason for hiding this comment

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

nit: extra line

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed, thanks!

Copy link
Member Author

Choose a reason for hiding this comment

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

writing too much python lately...

};
module.exports.LinkCell = LinkCell;

module.exports.PagedCell = ({data, ...props}) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we move this to be after PendingCell and move the exports to a new line, to follow the others

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, will do

@KamranAsif
Copy link
Contributor

LGTM

@wcjordan wcjordan merged commit 53df937 into schrodinger:master Oct 10, 2016
@wcjordan wcjordan deleted the new_examples branch October 10, 2016 22:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants