Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat!: change gestures to look at selected when dragging #7991

Merged
merged 4 commits into from
Apr 4, 2024
Merged

feat!: change gestures to look at selected when dragging #7991

merged 4 commits into from
Apr 4, 2024

Conversation

BeksOmega
Copy link
Collaborator

@BeksOmega BeksOmega commented Apr 2, 2024 •
edited
Loading

The basics

The details

Resolves

Fixes N/A

Proposed Changes + reasons

Makes it so the gesture class looks at whatever is selected when it starts a drag. This will allow the multi select plugin to provide its own draggable for when multiple things are selected.

Test Coverage

Manually tested and everything seems to be working! But this could definitely use some hammering.

  • Dragging blocks
  • Dragging child blocks
  • Dragging the workspace
  • Dragging flyouts
  • Dragging blocks from the flyout
  • Dragging on child blocks in the flyout
  • Dragging blocks in mutators

Documentation

N/A

Additional Information

N/A

Breaking changes / To fix

This reverses the responsibility of the Blockly.common.setSelected and the someElement.select methods. If you were calling select or unselect you should instead call Blockly.common.setSelected(some element) and Blockly.common.setSelected(null) respectively.

@github-actions github-actions bot added PR: feature Adds a feature and removed PR: feature Adds a feature labels Apr 2, 2024
@BeksOmega BeksOmega changed the title feat: change gestures to look at selected when dragging feat!: change gestures to look at selected when dragging Apr 2, 2024
@github-actions github-actions bot added breaking change Used to mark a PR or issue that changes our public APIs. PR: feature Adds a feature and removed PR: feature Adds a feature labels Apr 2, 2024
@BeksOmega BeksOmega mentioned this pull request Apr 2, 2024
Merged
1 task
BeksOmega
BeksOmega commented Apr 2, 2024
View reviewed changes
core/block_svg.ts Outdated
@@ -241,37 +241,19 @@ export class BlockSvg
return this.style.colourTertiary;
}

// TODO: Before merging, is it better to just remove these or to leave them
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually we have to leave these so that the BlockSvg conforms to the ISelectable interface.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you have to keep it, please add a description of the breaking change to the PR description (as I understanding, that block.select() and block.unselect() no longer fire events or update the workspace's selected value--they just update the block itself--and that devs need to call common.select to get the old behaviour.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm gonna go through and update the PR descriptions in bulk before the release.

@BeksOmega BeksOmega marked this pull request as ready for review April 2, 2024 22:54
@BeksOmega BeksOmega requested a review from a team as a code owner April 2, 2024 22:54
rachel-fenichel
rachel-fenichel reviewed Apr 3, 2024
View reviewed changes
core/block_svg.ts Outdated
@@ -241,37 +241,19 @@ export class BlockSvg
return this.style.colourTertiary;
}

// TODO: Before merging, is it better to just remove these or to leave them
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you have to keep it, please add a description of the breaking change to the PR description (as I understanding, that block.select() and block.unselect() no longer fire events or update the workspace's selected value--they just update the block itself--and that devs need to call common.select to get the old behaviour.

core/block_svg.ts Outdated Show resolved Hide resolved
core/comments/rendered_workspace_comment.ts Outdated Show resolved Hide resolved
core/dragging/block_drag_strategy.ts Show resolved Hide resolved
core/gesture.ts Outdated Show resolved Hide resolved
tests/mocha/comment_deserialization_test.js Outdated Show resolved Hide resolved
@BeksOmega BeksOmega merged commit ed403d0 into google:rc/v11.0.0 Apr 4, 2024
6 checks passed
johnnesky pushed a commit to johnnesky/blockly that referenced this pull request Apr 13, 2024
@BeksOmega @johnnesky
feat!: change gestures to look at selected when dragging (google#7991)
ac17298 
* feat: change gestures to look at selected when dragging

* chore: fix tests

* chore: format

* chore: PR comments
@github-actions github-actions bot added breaking change Used to mark a PR or issue that changes our public APIs. PR: feature Adds a feature and removed PR: feature Adds a feature breaking change Used to mark a PR or issue that changes our public APIs. labels Apr 19, 2024
@maribethb maribethb mentioned this pull request May 2, 2024
Merged
1 task
This was referenced Nov 13, 2024
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rachel-fenichel rachel-fenichel rachel-fenichel approved these changes

Assignees

@rachel-fenichel rachel-fenichel

Labels
breaking change Used to mark a PR or issue that changes our public APIs. PR: feature Adds a feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@BeksOmega @rachel-fenichel