Bug fixes: Null guard in repositionBlocks and _setupPaletteMenu#6446
Bug fixes: Null guard in repositionBlocks and _setupPaletteMenu#6446Shiv0087 wants to merge 1 commit intosugarlabs:masterfrom
Conversation
…vent crash on resize/scroll
|
✅ All Jest tests passed! This PR is ready to merge. |
There was a problem hiding this comment.
Pull request overview
This PR addresses two runtime TypeErrors triggered during scroll/resize by adding defensive guards before accessing properties that may not be initialized yet.
Changes:
- Added early-return guard in
repositionBlocks()whenactivity.blocks.blockListis unavailable. - Wrapped
this.boundary.hide()in a null check to prevent callinghide()onundefined.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // FIX: guard against blocks not being initialized yet | ||
| if (!activity || !activity.blocks || !activity.blocks.blockList) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
The closing brace indentation is inconsistent (line 813 has extra leading whitespace), which makes the control flow harder to read and easy to mis-edit. Please align the braces/indentation with the surrounding style (i.e., } aligned with the if).
| } | |
| } |
| //Array for storing individual dragGroups (the chunks of code linked together which are not connected) | ||
| const dragGroups = []; | ||
|
|
||
|
|
||
| // Identifying individual dragGroups | ||
| Object.values(activity.blocks.blockList).forEach(block => { |
There was a problem hiding this comment.
This comment states there is an array for storing dragGroups, but the dragGroups declaration was removed in this hunk. Either restore the declaration (if it’s still used later in repositionBlocks) or update/remove the comment to avoid misleading future edits.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| function repositionBlocks(activity) { | ||
| // FIX: guard against blocks not being initialized yet | ||
| if (!activity || !activity.blocks || !activity.blocks.blockList) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
The new null-guard block has inconsistent indentation/bracing compared to the surrounding file formatting (e.g., extra leading spaces and a misaligned closing brace). CI runs Prettier on changed JS files, so this will likely fail the formatting check; please reformat this block to match the project’s 4-space indentation (.editorconfig) / Prettier settings.
| ); | ||
| this.boundary.hide(); | ||
| // ✅ FIX: guard against boundary not being initialized | ||
| if (this.boundary) { | ||
| this.boundary.hide(); | ||
| } |
There was a problem hiding this comment.
This newly added this.boundary guard is misindented (extra leading spaces on the comment/if/body). Since the repo enforces Prettier formatting in CI for changed JS files, please reformat this section to match the surrounding indentation.
Type: Bug
Bug fixes: Null guard in repositionBlocks and _setupPaletteMenu
Problem
Two uncaught TypeErrors occur during scroll and resize:
Fix
Added null checks before accessing:
Testing
No TypeErrors observed in console.