-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Integration branch ux 12.2024 #9765
Conversation
… into feature/uepr-38-add-debug-modal
Feature/uepr 38 add debug modal
All contributors have signed the CLA ✍️ ✅ |
I have read the CLA Document and I hereby sign the CLA |
recheck |
useEffect(() => { | ||
sections.forEach(section => { | ||
new Image().src = section.image; | ||
}); | ||
}, []); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not familiar with this pattern... could you explain what's going on here, just for my education?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea is, that by loading the image once, React caches it and can reuse it later. So the earlier we execute this, the more time we have to load all images we need - in this case, on first render.
I added this at a relatively late point in developing the debug modal since I noticed an issue with the image load time. There seemed to be a noticeable improvement after that (though I did not measure it). Interestingly, I learned about this pattern from Scratch - I noticed it is used in a few places in scratch-www
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like there are some gaps between parts of the C blocks on this one
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should there be an arrowhead on the orange squiggle? If this is as intended then cool :)
id: 'gui.debugModal.addSoundCheckpoints.description1', | ||
defaultMessage: | ||
// eslint-disable-next-line max-len | ||
'Similar to the Slow It Down strategy, you can add different sounds with the “play until done” block at key points to test your sequence.', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extra space in add different
Resolves
What Github issue does this resolve (please include link)?
Proposed Changes
Describe what this Pull Request does
Reason for Changes
Explain why these changes should be made
Test Coverage
Please show how you have added tests to cover your changes
Browser Coverage
Check the OS/browser combinations tested (At least 2)
Mac
Windows
Chromebook
iPad
Android Tablet