-
Notifications
You must be signed in to change notification settings - Fork 290
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
Conversation
header={<Cell>First Name</Cell>} | ||
cell={<TextCell data={dataList} col="firstName" />} |
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.
moved these to columnKey which I believe is more inline with best practices
this._callback = callback; | ||
} | ||
|
||
getDataVersion() { |
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.
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), |
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.
I did this because this example is really jank with dev React (but fine with prod React). I can revert though if desired.
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.
Can we revert and use production react?
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.
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; |
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.
addresses facebookarchive/fixed-data-table#453
const React = require('react'); | ||
const {StyleSheet, css} = require('aphrodite'); | ||
|
||
class StylingExample 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.
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 @@ | |||
/** |
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.
I deleted all the old examples folder. It was unused.
@@ -8,6 +8,7 @@ | |||
"react-dom": ">=0.14.0 || ^0.14.0-beta3" | |||
}, | |||
"devDependencies": { |
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.
Does it make sense for example dependencies to be tracked as devDependencies? Should React be tracked as a devDependency as well?
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.
I'm not 100% sure. Doesn't NPM install these when someone uses these libs? Seems unnecessary if thats the case
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.
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'); |
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.
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 @@ | |||
|
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.
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 |
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.
fixes another issue with unknown propTypes in our examples
}; | ||
module.exports.TextCell = TextCell; | ||
|
||
|
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.
nit: extra line
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.
fixed, thanks!
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.
writing too much python lately...
}; | ||
module.exports.LinkCell = LinkCell; | ||
|
||
module.exports.PagedCell = ({data, ...props}) => { |
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.
Can we move this to be after PendingCell and move the exports to a new line, to follow the others
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.
Sure, will do
LGTM |
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):
![tooltip](https://cloud.githubusercontent.com/assets/1034455/19223385/44c3555e-8e3c-11e6-979b-c60eca27a0c4.jpg) ## Types of changes
Checklist: