-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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.
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
feat: merge copy paste work into develop #7379
Conversation
* feat: implement basic IPaster interface * feat: add pasters for blocks and workspace comments
* chore: rename CopyData to ICopyData * fix: ICopyable data structures * fix: switch clipboard over to use new copy data interfaces * chore: rename saveInfo to somthing more descriptive
* chore: rename module-local variables to not conflict * feat: make ICopyable generic and update clipboard APIs * chore: switch over more things to use generic ICopyables * chore: fix shortcut items using copy paste * chore: add test for interface between clipboard and pasters * chore: export isCopyable * chore: format * chore: fixup PR comments * chore: add deprecation tags
* fix: add logic for bumping pasted blocks * chore: add tests for bumping pasted blocks to the correct location * fix: add logic for bumping pasted comments * chore: add tests for bumping pasted comments
…ated APIs (#7352) * chore: remove references to clipboard.copy in shortcuts * chore: remove references to clipboard.copy in context menus * chore: fix tests * chore: format * fix: PR comments
* @param block The block to move to an unambiguous location. | ||
* @internal | ||
*/ | ||
export function moveBlockToNotConflict(block: BlockSvg) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was already reviewed in another PR, so this doesn't have to block merging, I'm just curious. does bumpNeighbors
not work for this?
This feels like really similar logic to what's happening in bumpNeighbors
but you're not taking advantage of some of the methods we already have like we have a connection.neighbours
method that takes in a snap radius, etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bumpNeighbors
always moves whichever block has the inferior connection :/ We want to make sure we're only moving the block that's being pasted.
Wrt code reuse: we're using connection.closest
instead of connection.neighbours
. They both take in a snap radius and do similar things.
The basics
npm run format
andnpm run lint
The details
Resolves
Fixes N/A
Proposed Changes
Merges the work that was being done on a feature branch into develop.
Reason for Changes
It's better to work off develop if I can so I don't need to worry about changes in develop geting out of sync with changes in my branch.
Additional info
Planning to merge-commit this so that the release notes include info about all of the previous PRs.