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

#493 - Add virtual scroll for HowtoList #962

Merged
merged 1 commit into from
Jun 1, 2020

Conversation

tudi2d
Copy link
Member

@tudi2d tudi2d commented May 15, 2020

PR Type

  • 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)

PR Checklist

  • - Latest master branch merged
  • - PR title descriptive (can be used in release notes)
  • - Passes Tests

Description

It adds a virtual scroll for the HowtoList to reduce initial rendering and only render the needed Howto´s for every visitor. It will render additional Howto´s while rendering, while removing Howto´s which are not in the viewport.

Therefor I added react-virtualized as discussed in the Issue. First I thought react-window would be good enough, but it does not support a Window scroll feature and there for react-virtualized was my choice here.

Git Issues

Closes #493

- Add react-virtualized package
@tudi2d tudi2d changed the title 493 - Add virtual scroll for HowtoList #493 - Add virtual scroll for HowtoList May 15, 2020
@@ -22,6 +24,11 @@ interface IState {
isLoading: boolean
}

const breakpointsInPX = themes.breakpoints.map(
Copy link
Member Author

@tudi2d tudi2d May 15, 2020

Choose a reason for hiding this comment

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

Here I was not sure how to access the theme properties. Is this ok? Is there an alternative to this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep that's it :)

@cypress
Copy link

cypress bot commented May 15, 2020



Test summary

30 0 0 0


Run details

Project onearmy-community-platform
Status Passed
Commit 296218e ℹ️
Started May 15, 2020 5:47 PM
Ended May 15, 2020 5:54 PM
Duration 06:41 💡
OS Linux Ubuntu Linux - 14.04
Browser Electron 78

View run in Cypress Dashboard ➡️


This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

@BenGamma
Copy link
Contributor

Hey @tudi2d thanks a lot for your work !
This feature was getting very needed as the number of how-tos on kept on growing pretty fast ! Also since we don't have a home page yet, users where landing on this page so this will save quite a lot of bandwidth 🌍
It looks good from my side, I assign chris to be sure he double check since he was the one folling up on the issue.
Thanks again 💪

@chrismclarke
Copy link
Member

Thanks again for the work on this and sorry for my slow response, been a bit swamped for the past week. Will review this weekend and get back to you.

@chrismclarke
Copy link
Member

Apologies again for the slow reply to this. Took a while to understand what was going on (particularly why to divide up chunks instead of using masonry or grid, but then realised for use within the window scroller), but now it makes a lot of sense and works well.

I've tidied up the code a bit, but as I can't add commits on top of your fork pr I'll merge this and then create another pr with the changes.

Thanks again

@chrismclarke chrismclarke merged commit 092d883 into ONEARMY:master Jun 1, 2020
@chrismclarke chrismclarke mentioned this pull request Jun 1, 2020
3 tasks
@chrismclarke
Copy link
Member

@tudi2d - We're starting to setup a bug bounty programme to show appreciation to developers who contribute to the platform. This issue was nominated as one to reward for May, so if you send me your Paypal details I can add you to the list for future payments. Either email me at chris.m.clarke@live.co.uk or find me on PP slack or discord as chrismclarke.

The amounts are pretty small at the moment as we don't haven't much funds for the platform yet (so possibly not time to quit the day-job just yet!), but at least it's a start and hopefully a step in the right direction.

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.

Virtual Scroll Component
3 participants