From 1cd18ac9f4181c7675e87d43eda69910c14c6d7b Mon Sep 17 00:00:00 2001 From: infiinte-persistence Date: Thu, 18 Jun 2020 11:18:25 +0200 Subject: [PATCH] Inf-scroll: (1) Reset to top (2) Fix "page vs. results" being out of sync. --- Issues: (1) #-2669: Page does not restore to top when navigating new tags. (2) "Encountered children with the same key" error (duplicate entries shown). https://github.com/lbryio/lbry-desktop/issues/4367#issuecomment-645449206 --- Changes: (1) Ignore the history if it's a new query (i.e. explicitly clicked). The BACK history will still behave as normal (doesn't reset to top). (2) Previously, the `page` variable will continue to increment as you scroll and stay within the page (e.g. Trending vs New, or clicked another Tag). As you move between queries, we hit a scenario where `page` is significantly under or over the latest retrieved `claimSearchResult.length`. This messes up the rest of the code. Fix by correcting the value of `page` according to the current `claimSearchResult.length` when necessary. --- ui/component/claimListDiscover/view.jsx | 37 ++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/ui/component/claimListDiscover/view.jsx b/ui/component/claimListDiscover/view.jsx index 18b477807..4c427e050 100644 --- a/ui/component/claimListDiscover/view.jsx +++ b/ui/component/claimListDiscover/view.jsx @@ -296,13 +296,32 @@ function ClaimListDiscover(props: Props) { const claimSearchCacheQuery = createNormalizedClaimSearchKey(options); const claimSearchResult = claimSearchByQuery[claimSearchCacheQuery]; + const [prevOptions, setPrevOptions] = useState(options); + + if (!isJustScrollingToNewPage(prevOptions, options)) { + setPrevOptions(options); + + if (didNavigateForward) { + // --- Reset to top. + window.scrollTo(0, 0); // Prevents onScrollBottom() from re-hitting while waiting for doClaimQuery(): + options.page = 1; + setPage(options.page); + } else { + // --- Update 'page' based on retrieved 'claimSearchResult'. + options.page = Math.ceil(claimSearchResult.length / CS.PAGE_SIZE); + if (options.page !== page) { + setPage(options.page); + } + } + } + const shouldPerformSearch = claimSearchResult === undefined || didNavigateForward || (!loading && claimSearchResult && claimSearchResult.length && - claimSearchResult.length < CS.PAGE_SIZE * page && + claimSearchResult.length < CS.PAGE_SIZE * options.page && claimSearchResult.length % CS.PAGE_SIZE === 0); // Don't use the query from createNormalizedClaimSearchKey for the effect since that doesn't include page & release_time @@ -337,6 +356,22 @@ function ClaimListDiscover(props: Props) { ); + // Returns true if the change in 'options' indicate that we are simply scrolling + // down to a new page; false otherwise. + function isJustScrollingToNewPage(prevOptions, options) { + // Compare every field except for 'page' and 'release_time'. + // There might be better ways to achieve this. + let tmpPrevOptions = { ...prevOptions }; + tmpPrevOptions.page = -1; + tmpPrevOptions.release_time = ''; + + let tmpOptions = { ...options }; + tmpOptions.page = -1; + tmpOptions.release_time = ''; + + return JSON.stringify(tmpOptions) === JSON.stringify(tmpPrevOptions); + } + function handleChange(change) { const url = buildUrl(change); setPage(1);