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

Feature/react window #46

Merged
merged 6 commits into from Jan 8, 2019
Merged

Feature/react window #46

merged 6 commits into from Jan 8, 2019

Conversation

P0lip
Copy link
Contributor

@P0lip P0lip commented Dec 30, 2018

Good news is Firefox supports CSS Scrollbars as of 64 version so we should have fairly consistent scrollbar styling for everywhere using plain CSS. The only subtle difference is lack of border-radius.

Copy link
Contributor

@marbemac marbemac left a comment

Choose a reason for hiding this comment

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

One inline question, and then is this the expected perf for re-rendering?

jan-01-2019 17-26-08

src/ScrollList.tsx Show resolved Hide resolved
@lottamus
Copy link
Contributor

lottamus commented Jan 4, 2019

One inline question, and then is this the expected perf for re-rendering?

Should we be memoizing the row before passing it into react-window?

They have an example of doing this in their docs using their areEqual function: https://react-window.now.sh/#/examples/list/memoized-list-items

image

Seems to work:

kapture 2019-01-04 at 13 07 28

@P0lip
Copy link
Contributor Author

P0lip commented Jan 4, 2019

Yeah, I use that memoization technique in Tree-List already.

@lottamus
Copy link
Contributor

lottamus commented Jan 4, 2019

Yeah, I use that memoization technique in Tree-List already.

Should we add it to the storybook too? Or should our ScrollList just do this for you by default?

@P0lip
Copy link
Contributor Author

P0lip commented Jan 4, 2019

Sure, will include it.

@marbemac marbemac dismissed their stale review January 5, 2019 18:57

will leave chris to give final review

@marbemac marbemac requested review from marbemac and removed request for marbemac January 5, 2019 18:57
@marbemac
Copy link
Contributor

marbemac commented Jan 5, 2019

Removed my review - leaving @lottamus to give final review.

@lottamus lottamus merged commit 99a8f06 into master Jan 8, 2019
@lottamus lottamus deleted the feature/react-window branch January 8, 2019 20:13
@marbemac
Copy link
Contributor

marbemac commented Jan 8, 2019

🎉 This PR is included in version 1.26.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants