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

Fixes: 1595 - [next] Infinite Scrolling of Posts is broken (All of our infinite scrolls were broken) #1614

Merged
merged 4 commits into from Jan 27, 2021

Conversation

PedroFonsecaDEV
Copy link
Contributor

@PedroFonsecaDEV PedroFonsecaDEV commented Jan 27, 2021

Issue This PR Addresses

Fixes #1595

Type of Change

  • Bugfix: Change which fixes an issue
  • New Feature: Change which adds functionality
  • Documentation Update: Change which improves documentation
  • UI: Change which improves UI

Description

After landed the PR: #1573 created a bug inside our LoadAutoScroll's useEffect both for our Gatsby and Next. This PR will fix the warning issue and fix all of our infinite scroll problems for Gatsby and Next (homepage and search page).

Checklist

  • [] Quality: This PR builds and passes our npm test and works locally
  • Tests: This PR includes thorough tests or an explanation of why it does not
  • Screenshots: This PR includes screenshots or GIFs of the changes made or an explanation of why it does not (if applicable)
  • Documentation: This PR includes updated/added documentation to user exposed functionality or configuration variables are added/changed or an explanation of why it does not(if applicable)

eslint-disable-next-line react-hooks/exhaustive-deps
LINE 45
  File: LoadAutoScroll.tsx
    eslint-disable-next-line react-hooks/exhaustive-deps
    LINE 48
File: LoadAutoScroll.tsx
@tonyvugithub
Copy link
Contributor

@PedroFonsecaDEV : Seem to be a quick fix, good job!

Copy link
Contributor

@chrispinkney chrispinkney left a comment

Choose a reason for hiding this comment

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

LGTM

Nextjs automation moved this from In progress to Reviewer approved Jan 27, 2021
@tonyvugithub tonyvugithub merged commit 67ed296 into Seneca-CDOT:master Jan 27, 2021
Nextjs automation moved this from Reviewer approved to Done Jan 27, 2021
@@ -44,12 +44,13 @@ function LoadAutoScroll({ onScroll }: Scroll) {
return () => {
observer.unobserve(buttonRefCopy as HTMLButtonElement);
};
}, [$buttonRef, onScroll]);
// eslint-disable-next-line react-hooks/exhaustive-deps
Copy link
Contributor

Choose a reason for hiding this comment

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

We should file a follow-up to figure out why our eslint doesn't like [] since I'd say it's a valid use case to not define deps, but want a one-time effect.

DukeManh pushed a commit to DukeManh/telescope that referenced this pull request Feb 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: nextjs Nextjs related issues
Projects
No open projects
Nextjs
  
Done
Development

Successfully merging this pull request may close these issues.

[next] Infinite Scrolling of Posts is broken
4 participants