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

Adding direction to overscanIndicesGetter #629

Merged
merged 4 commits into from
Mar 24, 2017
Merged

Adding direction to overscanIndicesGetter #629

merged 4 commits into from
Mar 24, 2017

Conversation

offsky
Copy link
Contributor

@offsky offsky commented Mar 24, 2017

Fixes #627 by adding direction to the overscanIndicesGetter

@bvaughn
Copy link
Owner

bvaughn commented Mar 24, 2017

Mind updating the docs as well to list the new parameter? 😄

@offsky
Copy link
Contributor Author

offsky commented Mar 24, 2017

Like that?

@bvaughn
Copy link
Owner

bvaughn commented Mar 24, 2017

Thanks, but not quite. Update the section below that the hyperlink refers to (#overscanindicesgetter)

@offsky
Copy link
Contributor Author

offsky commented Mar 24, 2017

Im sorry, I dont understand. Im happy to make a change if you can elaborate.

@bvaughn
Copy link
Owner

bvaughn commented Mar 24, 2017

I was just clarifying that- by enabling horizontal scrolling- you're using List in a way it's not intended to be used. It's great that everything works (excepting this). I just wanted that clarification.

The reference I made about width being 100% is here.

@offsky
Copy link
Contributor Author

offsky commented Mar 24, 2017

Brian, I think we got our conversations mixed. This pull request is for my upgrade to the overscanIndicesGetter. Your last comment seems like it was meant for issue #630. Im happy to update this pull request with better documentation if you can elaborate on what changes you want me to make.

@bvaughn
Copy link
Owner

bvaughn commented Mar 24, 2017

lol yes sorry. I was responding via email client and I assumed your response was to my most recent comment on the issue. Sorry!

Go look at the Grid props docs. Specifically, the overscanIndicesGetter property doesn't defines its interface inline b'c it's hard to read. Instead it links to a separate section. I was requesting you put the interface/params list there. This is similar to how other props (eg cellRenderer) are done. 😄

@bvaughn
Copy link
Owner

bvaughn commented Mar 24, 2017

👍 Thanks

@bvaughn bvaughn merged commit c77b523 into bvaughn:master Mar 24, 2017
@bvaughn
Copy link
Owner

bvaughn commented Mar 27, 2017

9.4.0 was just released with this feature. Thank you for your contribution! I listed you in the 9.4.0 release notes.

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