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

Show a message if loading own comments is taking >10s #3041

Merged

Conversation

keikari
Copy link
Contributor

@keikari keikari commented Jan 17, 2024

Fixes:

User complained about loading own comments "hanging". It took ~30s on their account.

Now loading long would look like this:

large-comment-history.mp4

@keikari
Copy link
Contributor Author

keikari commented Jan 17, 2024

Just realized that useEffect may be unnecessary here. Will try without it.

EDIT: Works just the same without it.

@keikari keikari force-pushed the large-commment-history-loading branch from 2b4ca3b to d6f9be3 Compare January 17, 2024 15:35
@infinite-persistence
Copy link
Collaborator

  • We aren't supposed to make side-effects inside a render function, so my suggestion is to use the previous version. That would be the norm.
  • I think that effect version should be fine -- feel free to go ahead with this 👍

2 minor issues if want to improve:

  • The timer might not be cleared if the page is moved away while fetching (usually, an effect's return statement is used to clean up)
  • The ref could be avoidable if doCommentListOwn is await-able (can be tweaked to make it so).

The following is GPT's suggestion (but again, your previous one is fine):

React.useEffect(() => {
  let timeout;

  const fetchData = async () => {
    setIsLoadingLong(false);

    if (page !== 0 && activeChannelId) {
      timeout = setTimeout(() => setIsLoadingLong(true), 10000);
      
      try {
        await doCommentListOwn(activeChannelId, page, COMMENT_PAGE_SIZE_TOP_LEVEL);
      } catch (error) {
        // Handle any errors here
      } finally {
        clearTimeout(timeout); // Clear the timeout if the function finishes or fails
      }
    }
  };

  fetchData();
  return () => clearTimeout(timeout); // Ensure the timeout is cleared when the component unmounts
}, [page]); // eslint-disable-line react-hooks/exhaustive-deps

@keikari keikari force-pushed the large-commment-history-loading branch from d6f9be3 to 83c40ad Compare January 18, 2024 06:30
@keikari
Copy link
Contributor Author

keikari commented Jan 18, 2024

Changed to the GPT's solution. Thanks!

@tzarebczan
Copy link
Contributor

Thanks guys!

@tzarebczan tzarebczan merged commit dbfb376 into OdyseeTeam:master Jan 18, 2024
2 checks passed
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.

3 participants