Closed LukeWood closed 1 year ago
This pull request is automatically built and testable in CodeSandbox.
To see build info of the built libraries, click here or the icon next to each commit SHA.
Latest deployment of this branch, based on commit 71036a32360f7381ec718e221a160833c82dc6d8:
Sandbox | Source |
---|---|
sandbox | Configuration |
I've updated to include tests!
@LukeWood It looks like tests are failing?
Ah interesting let me look.
is there a quick snippet or guide I can look at to setup my testing locally?
@LukeWood after npm install it should simply be npm test
I can't reproduce this - but I did have a bad test commit a few commits ago.
Could nx be caching that?
@lunarraid thanks for the reviews. I'm not sure why my tests pass locally. I don't understand the testing infrastructure well enough to reason about it.
@LukeWood It says you requested a review but I don't see any changes since my last review. Also I don't think your latest test I commented on was what was failing, the test itself would pass because it wasn't checking if the type passed to the component was the correct type, only if it changed according to the params. My guess is the initial test failed due to the bad push you mentioned as they aren't failing at the moment.
I must have forgotten to push. I will do so when home!
On Thu, Jun 15, 2023 at 7:07 PM Raymond @.***> wrote:
@LukeWood https://github.com/LukeWood It says you requested a review but I don't see any changes since my last review. Also I don't think your latest test I commented on was what was failing, the test itself would pass because it wasn't checking if the type passed to the component was the correct type, only if it changed according to the params. My guess is the initial test failed due to the bad push you mentioned as they aren't failing at the moment.
— Reply to this email directly, view it on GitHub https://github.com/pixijs/pixi-react/pull/441#issuecomment-1593865757, or unsubscribe https://github.com/notifications/unsubscribe-auth/AC5AMRYNQUKBH2PNMHZMAXTXLOPVLANCNFSM6AAAAAAZFUX2YM . You are receiving this because you were mentioned.Message ID: @.***>
-- Sent from mobile, Please excuse any typos.
Thanks @lunarraid !
Hey @LukeWood sorry for the delay, this fix has been published now
🙌 🙌 🙌
Thanks for reviews
On Fri, Sep 1, 2023 at 4:48 AM Zyie @.***> wrote:
Hey @LukeWood https://github.com/LukeWood sorry for the delay, this fix has been published now
— Reply to this email directly, view it on GitHub https://github.com/pixijs/pixi-react/pull/441#issuecomment-1702396622, or unsubscribe https://github.com/notifications/unsubscribe-auth/AC5AMRZWOUD6OQB3NV32CMTXYGONFANCNFSM6AAAAAAZFUX2YM . You are receiving this because you were mentioned.Message ID: @.***>
-- Sent from mobile, Please excuse any typos.
Description of change
Currently, updating the image attribute does not update the actual textures on the animated sprite component. My PR fixes this by checking to see if the images are distinct from the previous iteration, and actually updating them accordingly.
Fixes:
Pre-Merge Checklist
Will do soon, manually testing still - and looking for some feedback
npm run lint
)npm run test
)