-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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 basic ListView component #87
Conversation
Update from original master
@@ -0,0 +1,194 @@ | |||
import invariant from 'fbjs/lib/invariant' | |||
|
|||
export default class ListViewDataSource { |
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.
why isn't this class copy-pasted from react-native?
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.
That particular class was build using flow types incorporated into the code. I (for some reason..) thought that was not supported by the react-native-web toolchain (I had done some spot-checks and could not find any flow enabled code), so instead made a port of it (the code is nearly identical though).
I've just checked and the original code compiles just fine and works as expected. I agree we should use that. I only had to change the require's as the top of the file to use:
var invariant = require('fbjs/lib/invariant');
var isEmpty = require('fbjs/lib/isEmpty');
var warning = require('fbjs/lib/warning');
Furthermore, the class throws a ton of linting errors because semi-colons are not allowed. I think it would be nice if we could drop code from 'react-native' in without having to remove all semi-colons (or change the require's).
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.
react native is changing their fbjs requires to work like standard npm requires. you can add /* eslint disable */
to the top of the file as is done in other modules mirroring RN
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.
Alright cool, I'll do that. It's probably gonna be tomorrow before I have time for this.
Hi, I've updated ListViewDataSource to the copy/pasted version of the original react-native code. |
I am interested by this ListView. Anything missing @necolas? |
For the record, I'm also working a new virtualized (infinite) scrollview implementation for javascript, supporting (sticky) section-headers, insert/remove/move animations, bottom alignment, etc.. I'll post progress here when I more to show. |
Why you don't work on react-virtualized, which seems like a pretty solid start? They have an open PR for the v7 which will remove some legacy things bvaughn/react-virtualized#206 |
@MoOx Cause I'm working on a different feature-set, I have a set of specific goals in my mind. It's not specifically aimed at react-native-web, but when done we could have a look whether it is a match. In terms of features I'm recreating what can be seen in this demo here: |
I'm interested by this ListView component. I tried merging the PR branch and the master branch and it worked like a charm for me after fixing a small conflict, which is great, compared to the current master which throws an error on Can't it be merged as it is, and the missing performance-related features added later? |
I love this project, thanks everyone for your contributions! I am having difficulties getting the ListView to compile though. I manually created the files in this PR in my project and get the following error when running it: Module parse failed: /Users/.../node_modules/react-native-web/dist/components/ListView/index.js Unexpected character '@' (10:0) it is pointing to this line: I am sorry if this is not the place for this, and thanks again for all your work. EDIT: I realize I am missing all of the babel translated code at the top of the file, now to figure out how to get that in there... |
@mcampsall you need a babel plugin in your stack that will handle decorators as they are not in es2015 (and not in es2016). See https://babeljs.io/docs/plugins/transform-decorators/ |
Thanks @MoOx, that would likely work if I had added the files to the project correctly. What would the proper way to add this PR to my project? |
+1 to getting this in - I'd like to use this in the main React Native docs for ListView. @necolas are there any missing features / tests that you'd want before merging this? |
Thanks! I've merged this to unblock the work on the RN docs (install 0.0.28). We can iterate on ListView from here. |
Thanks man I owe you one high five 😃 |
@MoOx how to use react-virtualized on web? |
Just consume it via npm. Default main of the package.json leads to a commonjs build. You should not have to do anything special. If you need more help, you should read the docs or contact the maintainer of this package, not here :) |
hi, how to install this pull request through npm? |
ListView has been remove from React Native, so it has been from React Native for web. |
This commit adds a basic ListView component implementation. It fully supports the DataSource api which is exposed by ListView.DataSource. This allows for running your react-native ListView code using react-native-web.
Things that are supported:
[X] ListView.DataSource (fully supported)
[X] SectionHeaders (
ListView.props.renderSectionHeader
&ListView.DataSource.cloneWithRowsAndSections
)[X] Header (
ListView.props.renderHeader
)[X] Footer (
ListView.props.renderFooter
)[X] Separators (
ListView.props.renderSeparator
)[X] renderScrollComponent (
ListView.props.renderScrollComponent
)Things are are not (yet) supported:
[ ] Infinite scrolling (only render content that is within a certain visible view)
[ ] Only re-render changed rows
The following props are defined but not (yet) used anywhere: