Refactor/workspace layout controller#7768
Merged
walterbender merged 4 commits intoJul 9, 2026
Merged
Conversation
…roller Move the Home button workspace layout responsibilities (_findBlocks, repositionBlocks, setHomeContainers, _isFirstHomeClick toggle) out of activity.js into js/activity/workspace-layout-controller.js, following the same controller-extraction pattern used for GridController, SearchController and SelectionController. activity.js now instantiates the controller and keeps thin delegation stubs so existing callers are unaffected. Signed-off-by: Vanshika <pahalvanshikaa@gmail.com>
Cover setup/wiring, findBlocks, the row/column _findBlocks toggle, repositionBlocks breakpoint handling, resize delegation, setHomeContainers, turtle reset behavior, and edge cases such as empty, single-block and large workspaces. Signed-off-by: Vanshika <pahalvanshikaa@gmail.com>
…helpfulWheelDiv
document.getElementById("helpfulWheelDiv") can return null; accessing
.style on it would throw. The pre-extraction code had the same gap,
so guard it now that the code is being touched.
Signed-off-by: Vanshika <pahalvanshikaa@gmail.com>
…ce test duplication Use controller.method.bind(controller) instead of arrow-function wrappers for the activity-level delegation stubs, and add a setupActivity() test helper to collapse the repeated makeActivity()+setupWorkspaceLayoutController() pairing across the suite. Also correct a misleading comment (the DOM lookup isn't cached, it's just done once). Signed-off-by: Vanshika <pahalvanshikaa@gmail.com>
vanshika2720
requested review from
Ashutoshx7,
omsuneri and
walterbender
as code owners
Member
|
Looks good but do you know what is going on with the test failures? |
Contributor
Author
|
@walterbender current master is carrying two Jest regressions introduced by the recently merged test coverage PRs #7646 – test/pitchstaircase-widget-coverage: pitchstaircase.test.js fails with TypeError: Cannot set properties of undefined (setting 'backgroundColor') and a failing replaceChildren expectation. #7647 – test/musickeyboard-widget-coverage: musickeyboard.test.js fails in _sortLayout, expecting ["do", "sol", "hertz"] but receiving the full unsorted pitch array. That's why the test is failing. |
github-project-automation
Bot
moved this from Todo
to Done
in @MusicBlocks Issues and PRs
rakshityadav1868
pushed a commit
to rakshityadav1868/musicblocks
that referenced
this pull request
Jul 9, 2026
* refactor(activity): extract workspace layout into WorkspaceLayoutController
Move the Home button workspace layout responsibilities (_findBlocks,
repositionBlocks, setHomeContainers, _isFirstHomeClick toggle) out of
activity.js into js/activity/workspace-layout-controller.js, following
the same controller-extraction pattern used for GridController,
SearchController and SelectionController. activity.js now instantiates
the controller and keeps thin delegation stubs so existing callers are
unaffected.
Signed-off-by: Vanshika <pahalvanshikaa@gmail.com>
* test(workspace-layout-controller): add behavioral test coverage
Cover setup/wiring, findBlocks, the row/column _findBlocks toggle,
repositionBlocks breakpoint handling, resize delegation,
setHomeContainers, turtle reset behavior, and edge cases such as
empty, single-block and large workspaces.
Signed-off-by: Vanshika <pahalvanshikaa@gmail.com>
* fix(workspace-layout-controller): guard findBlocks against a missing helpfulWheelDiv
document.getElementById("helpfulWheelDiv") can return null; accessing
.style on it would throw. The pre-extraction code had the same gap,
so guard it now that the code is being touched.
Signed-off-by: Vanshika <pahalvanshikaa@gmail.com>
* refactor(workspace-layout-controller): bind delegation stubs and reduce test duplication
Use controller.method.bind(controller) instead of arrow-function
wrappers for the activity-level delegation stubs, and add a
setupActivity() test helper to collapse the repeated
makeActivity()+setupWorkspaceLayoutController() pairing across the
suite. Also correct a misleading comment (the DOM lookup isn't
cached, it's just done once).
Signed-off-by: Vanshika <pahalvanshikaa@gmail.com>
---------
Signed-off-by: Vanshika <pahalvanshikaa@gmail.com>
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
2 participants
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.
Summary
This PR extracts the workspace layout and Home button functionality from
activity.jsinto a dedicatedWorkspaceLayoutController, following the existing controller architecture used throughout the project. The refactor preserves existing behavior while improving modularity and maintainability.Changes
js/activity/workspace-layout-controller.jsWorkspaceLayoutControllerandsetupWorkspaceLayoutController(activity)activity.jswith delegation stubsloader.jsExtracted Functionality
findBlocks_findBlockssetHomeContainersrepositionBlocks_handleRepositionBlocksOnResizeThe controller also manages the
_isFirstHomeClickstate used for toggling the Home button layout behavior.Tests
Added:
js/activity/__tests__/workspace-layout-controller.test.jsCoverage includes:
Also updated:
js/__tests__/activity_toolbar_integration.test.jsto mock
setupWorkspaceLayoutController.Verification