Perf: Optimize event listeners, cloning operations, and trash view rendering#6121
Open
parthdagia05 wants to merge 1 commit intosugarlabs:masterfrom
Open
Perf: Optimize event listeners, cloning operations, and trash view rendering#6121parthdagia05 wants to merge 1 commit intosugarlabs:masterfrom
parthdagia05 wants to merge 1 commit intosugarlabs:masterfrom
Conversation
- Prevent duplicate click listeners in languagebox.js while keeping test compatibility - Replace JSON.parse(JSON.stringify(...)) with Object.assign for cheaper cloning in musickeyboard.js - Move handleClickOutsideTrashView() outside loop to avoid redundant event handler setup - Fix padding value in statistics.js to match expected shorthand All tests pass (3930/3930). No application logic changes.
Contributor
|
✅ All Jest tests passed! This PR is ready to merge. |
Contributor
Author
|
@walterbender @omsuneri this PR is ready for review. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Category
Summary
This PR introduces several small performance improvements across the codebase while maintaining full compatibility with the existing test suite.
The optimizations reduce unnecessary event listener accumulation, avoid expensive deep cloning operations inside loops, and eliminate redundant event handler setup in frequently executed UI rendering code.
All changes are implementation-level improvements and do not modify application behavior.
Changes
1. Prevent Event Listener Accumulation
File:
languagebox.jsRepeated calls to
hide()previously attached duplicate click listeners to.language-linkelements, causing unnecessary event handling and potential memory growth.The implementation now ensures listeners are attached only once while remaining compatible with the mocked DOM used in tests.
2. Replace Expensive Deep Clone Operations
File:
musickeyboard.jsInstances of:
were replaced with:
This avoids unnecessary serialization and reduces CPU and GC overhead when cloning objects inside loops.
3. Remove Redundant Event Handler Setup
File:
activity.jshandleClickOutsideTrashView(trashView)was previously called inside aforEachloop, causing repeated remove/add cycles of the same event handler.The call has been moved outside the loop so the handler is registered only once.
4. Fix Padding Value for Test Compatibility
File:
statistics.jsUpdated:
to
to match the expected shorthand value used in the test suite.
Test Results
All tests pass successfully:
Impact
These optimizations:
Resulting in improved runtime efficiency without changing user-facing behavior.