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

Add timestamp to AnimationFrame #453

Closed
wants to merge 6 commits into from

Conversation

bbugh
Copy link
Contributor

@bbugh bbugh commented Jun 1, 2021

This adds the expected Number argument from requestAnimationFrame so that consumers can calculate the time step difference, etc.

Fixes #445

@gdotdesign
Copy link
Member

Please update the tests, otherwise it looks good 👍

@Sija Sija added the enhancement New feature or request label Jun 1, 2021
@bbugh
Copy link
Contributor Author

bbugh commented Jun 1, 2021

Ah, I forgot to mention that. Hard to think while holding a squirmy child...

I wasn't sure how to test the timestamps, given that the argument is non-deterministic. In other tools, I'd use a mock or stub tool, or something like timecop to guarantee consistent results.

I looked through the source for some way to mock it and/or make a strict timestamp result, and I couldn't find anything. Do you have a suggestion?

@Sija Sija added the stdlib Standard library related label Jun 2, 2021
@Sija
Copy link
Member

Sija commented Jun 4, 2021

@bbugh I'd change the tests to assert that the passed argument is greater than 0. That should be good enough.

@Sija Sija added this to the 0.13.0 milestone Jun 6, 2021
@bbugh
Copy link
Contributor Author

bbugh commented Jun 6, 2021

There does not seem to be an assertion for greater than, and I don't see one for not equal, or any negation at all. I will make another issue for this.

@bbugh
Copy link
Contributor Author

bbugh commented Jun 6, 2021

Added discussion #458

@gdotdesign gdotdesign requested a review from Sija June 8, 2021 09:48
core/source/Test/Context.mint Show resolved Hide resolved
core/source/Test/Context.mint Show resolved Hide resolved
@bbugh
Copy link
Contributor Author

bbugh commented Jun 8, 2021

@gdotdesign thanks for the updates! It's useful to see how you would write those tests, as the resident Mint expert. ;)

@Sija in case you didn't notice, the changes you've requested were put in by @gdotdesign's commits, so I'm unsure if I should accept the suggestions.

@Sija
Copy link
Member

Sija commented Jun 8, 2021

@bbugh I haven't, sorry :D I still believe though we should split those.

@Sija Sija mentioned this pull request Jun 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request stdlib Standard library related
Development

Successfully merging this pull request may close these issues.

Provider.AnimationFrame does not provide DOMHighResTimeStamp argument
3 participants