Skip to content
This repository has been archived by the owner on Jan 10, 2025. It is now read-only.

✨ Added debounce and throttle #829

Closed
wants to merge 4 commits into from
Closed

Conversation

AndrewMusgrave
Copy link
Member

@AndrewMusgrave AndrewMusgrave commented Aug 8, 2019

Description

Add debounce and throttle to function enhancers from polaris-rails and the tests πŸ™‡β€β™‚οΈ and typescriptify it. We never return a value this version of debounce, I'll create an issue for it to be addressed later.

Type of change

  • Patch: Bug/ Documentation fix (non-breaking change which fixes an issue or adds documentation)
  • Minor: New feature (non-breaking change which adds functionality)
  • Major: Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

  • I have added a changelog entry, prefixed by the type of change noted above
  • I have prefixed my pull request title with the corresponding emoji from this guide

@AndrewMusgrave AndrewMusgrave force-pushed the add-debounce-throttle branch from be07489 to 4cafbed Compare August 8, 2019 15:51
@AndrewMusgrave AndrewMusgrave changed the title Added debounce and throttle ✨ Added debounce and throttle Aug 8, 2019
@AndrewMusgrave AndrewMusgrave changed the title ✨ Added debounce and throttle Added debounce and throttle Aug 8, 2019
@AndrewMusgrave AndrewMusgrave changed the title Added debounce and throttle ✨ Added debounce and throttle Aug 8, 2019
@AndrewMusgrave AndrewMusgrave requested a review from michenly August 8, 2019 15:56
@AndrewMusgrave AndrewMusgrave marked this pull request as ready for review September 5, 2019 19:28
@AndrewMusgrave
Copy link
Member Author

cc/ @michenly

@AndrewMusgrave AndrewMusgrave requested review from michenly and removed request for michenly October 21, 2019 16:38
@lemonmade
Copy link
Member

@michenly can you please review this when you get a chance?

Copy link
Contributor

@marutypes marutypes left a comment

Choose a reason for hiding this comment

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

Overall this looks great, readable code and well tested.

The main thing this lacks for me right now is documentation, I'd like to see some added to the README, and I think it might be a good case for adding a simple TSDoc style comment so editors can pickup some context to display about what the options do (maxWait in particular is somewhat unintuitive)

maxWait?: number;
}

export default function debounce<F extends (...args: any[]) => void>(
Copy link
Contributor

Choose a reason for hiding this comment

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

A TSDoc comment might be nice here to get some tooltip explanations of what the options here do. We do some of that in Quilt libraries such as react-form and I think here it could be helpful.

@marutypes
Copy link
Contributor

PS @AndrewMusgrave sorry for the long wait

@AndrewMusgrave
Copy link
Member Author

Thanks for the review @TheMallen πŸ™Œ I'll add some docs at the beginning of next week πŸ˜„

@stale
Copy link

stale bot commented Dec 25, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale Stale issue that hasn't received any attention in a while label Dec 25, 2020
Base automatically changed from master to main February 18, 2021 19:25
@stale stale bot removed the stale Stale issue that hasn't received any attention in a while label Feb 18, 2021
@AndrewMusgrave AndrewMusgrave requested a review from a team June 29, 2021 16:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants