Skip to content

Fixing useAsyncList loadMore skip logic #1829

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

Merged
merged 6 commits into from
Apr 22, 2021
Merged

Fixing useAsyncList loadMore skip logic #1829

merged 6 commits into from
Apr 22, 2021

Conversation

LFDanLu
Copy link
Member

@LFDanLu LFDanLu commented Apr 22, 2021

Closes

✅ Pull Request Checklist:

  • Included link to corresponding React Spectrum GitHub Issue.
  • Added/updated unit tests and storybook for this change (for new code or code which already has tests).
  • Filled out test instructions.
  • Updated documentation (if it already exists for this component).
  • Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

🧢 Your Project:

bug occured in combobox where selecting an option and then quickly opening the menu again caused the list to have duplicate entries. This was because we now rely on a loadingState ref that would update to "error" since the loadMore from the first opening of the menu was canceled by the filtering caused by the option selection, allowing loadMore to trigger again instead of being skipped when opening the menu again.Also moved the settimeout (for simulated long fetch times) before the fetch so requests would get propely canceled, otherwise the fetch could finish early
@adobe-bot
Copy link

Build successful! 🎉

track if any loadMore or filtering operations are happening via Set of abortControllers and skip if so
@adobe-bot
Copy link

Build successful! 🎉

@LFDanLu LFDanLu changed the title (WIP) Fixing useAsyncList loadMore skip logic Fixing useAsyncList loadMore skip logic Apr 22, 2021
@LFDanLu LFDanLu marked this pull request as ready for review April 22, 2021 17:49
@adobe-bot
Copy link

Build successful! 🎉

@adobe-bot
Copy link

Build successful! 🎉

@@ -230,6 +234,12 @@ function reducer<T, C>(data: AsyncListState<T, C>, action: Action<T, C>): AsyncL
items: action.type === 'loading' ? [] : data.items,
abortController: action.abortController
};
case 'loadingMore':
// If already loading more and another loading more is triggered, abort the new load more.
Copy link
Member

Choose a reason for hiding this comment

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

Can you just add a little bit more about "the cursor hasn't been updated so they are duplicate requests"?

@adobe-bot
Copy link

Build successful! 🎉

@devongovett devongovett merged commit da3dc02 into main Apr 22, 2021
@devongovett devongovett deleted the fix_useasynclist branch April 22, 2021 21:52
majornista pushed a commit to majornista/react-spectrum-v3 that referenced this pull request May 19, 2021
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.

4 participants