Skip to content

Conversation

@BeksOmega
Copy link
Contributor

@BeksOmega BeksOmega commented Aug 11, 2023

The basics

  • I branched from develop
  • My pull request is against develop
  • My code follows the style guide
  • I ran npm run format and npm 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.

* 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
@BeksOmega BeksOmega requested a review from maribethb August 11, 2023 16:40
@BeksOmega BeksOmega requested a review from a team as a code owner August 11, 2023 16:40
@github-actions github-actions bot added PR: feature Adds a feature and removed PR: feature Adds a feature labels Aug 11, 2023
* @param block The block to move to an unambiguous location.
* @internal
*/
export function moveBlockToNotConflict(block: BlockSvg) {
Copy link
Contributor

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.

Copy link
Contributor Author

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.

@BeksOmega BeksOmega merged commit 1a41891 into develop Aug 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PR: feature Adds a feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants