You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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
The setMultiSelectedIds API uses string flags and pushes IDs directly, risking duplicates and unclear reset logic. Consider using a clearer signature (e.g., accept arrays or separate add/remove) and deduplicating entries.
/** * Set the multi-selected block IDs * @param id - id of the block to add to the multi-selected list, or "clear" to reset the list */setMultiSelectedIds(id: string){if(id==='clear'){this._store.multiselectedIds=[];}else{this._store.multiselectedIds.push(id);}}
The handleMultipleSelection logic resets selected when multiple IDs are chosen, which may lead to inconsistent UI states. Verify the flow between selected and multiselectedIds, and ensure proper toggling and clearing.
consthandleMultipleSelection=(event)=>{if(!designer.hovered||designer.hovered===designer.selected){return;}constid=getNearestBlock(event.targetasElement);// prevent events for elements until selectedevent.stopPropagation();event.preventDefault();if(designer.multiselectedIds.includes(id)){return;// Do nothing if the id is already selected}designer.setSelected(id);designer.setMultiSelectedIds(id);if(designer.multiselectedIds.length>1){designer.setSelected('');}};
The multi-block drag-and-drop logic in handleDocumentMouseUp is complex and duplicates code paths. Ensure correct order when moving blocks, handle failures gracefully, and consider refactoring to reduce duplication and improve maintainability.
if(designer.multiselectedIds.length>1){// Handle multiple block movementsletlastSiblingId=placeholderAction.id;// Start with the placeholder IDdesigner.multiselectedIds.forEach((id)=>{constsw=state.getBlock(placeholderAction.id);if(placeholderAction.type==='before'||placeholderAction.type==='after'){constsiblingWidget=state.getBlock(lastSiblingId);// Use the last sibling IDif(siblingWidget.parent){constparent=state.getBlock(sw.parent.id);if(parent.widget==='iteration'){if(parent.slots.children.children.length){notification.add({color: 'error',message:
'Please delete block within iterator before adding another child',});designer.deactivateDrag();return;}}state.dispatch({message: ActionMessages.MOVE_BLOCK,payload: {
id,position: {parent: siblingWidget.parent.id,slot: siblingWidget.parent.slot,sibling:
placeholderAction.type==='before'
? siblingWidget.id
: lastSiblingId,type: placeholderAction.type,},},});// Update the lastSiblingId to the current blocklastSiblingId=id;}}elseif(placeholderAction.type==='replace'){if(sw.widget!=='iteration'){state.dispatch({message: ActionMessages.SET_BLOCK_DATA,payload: {id: placeholderAction.id,path: 'child',value: state.getBlock(id),},});}if(sw.widget!=='iteration'){state.dispatch({message: ActionMessages.MOVE_BLOCK,payload: {
id,position: {parent: placeholderAction.id,slot: placeholderAction.slot,},},});}}});
When iterating multiple selected IDs, each block’s own reference should be used instead of always using placeholderAction.id. Update the lookup to use lastSiblingId (or id as appropriate) so that the correct block is fetched for movement calculations.
Why: The loop incorrectly always fetches the same block via placeholderAction.id; using lastSiblingId fixes subsequent sibling lookups and ensures correct move calculations.
Medium
General
Show alias per block
The variableName only applies to a single selected block and will be empty in multi-select mode. Retrieve the alias per block inside the map, e.g. state.getAlias(id), so each mask shows its correct label.
Why: In multi-select mode, variableName remains the alias of the last single selection; using state.getAlias(id) ensures each mask displays the correct label.
Medium
Include initial selected block
When starting a multi-selection, the original selected block should also be included. If multiselectedIds is empty, first push the current designer.selected before adding the new id. This ensures the first and subsequent blocks are grouped.
Why: Without adding the original selected to multiselectedIds, the first block is never grouped, so pushing it when the array is empty fixes complete multi-selection behavior.
Medium
Avoid mutating ID array
Mutating the array in place can break reactivity and allow duplicates. Replace push with array spreading to create a new array on each update.
if (id === 'clear') {
this._store.multiselectedIds = [];
} else {
- this._store.multiselectedIds.push(id);+ this._store.multiselectedIds = [...this._store.multiselectedIds, id];
}
Suggestion importance[1-10]: 5
__
Why: Replacing in-place push with array spreading creates a new array reference for better reactivity; however, it doesn’t inherently prevent duplicate IDs.
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.
Now the user can able to select multiple blocks.