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.

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

fix: Hide content highlight on comment drag, update on resize. #2400

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

johnnesky
Copy link
Member

The basics

The details

Resolves

Fixes #2330

Proposed Changes

Temporarily hides the content highlight when a workspace comment drag is initiated, and reveals it with an updated rectangle when the drag is completed. This is consistent with how block drags are handled.

When a workspace comment resize is completed, the content highlight rectangle is updated. (There's currently no event representing the start of a resize drag, but it's better than nothing.)

Reason for Changes

For consistency with moving regular blocks.

Test Coverage

None.

Documentation

N/A

Additional Information

This PR should not be submitted until the an update is published for the core Blockly library containing: google/blockly#8217

@johnnesky johnnesky requested a review from a team as a code owner June 29, 2024 03:21
@johnnesky johnnesky requested review from BeksOmega and removed request for a team June 29, 2024 03:21
@BeksOmega BeksOmega marked this pull request as draft July 1, 2024 16:35
Copy link
Contributor

@BeksOmega BeksOmega left a comment

Choose a reason for hiding this comment

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

Approved, but converted to draft so it doesn't accidentally get merged.

@@ -48,7 +48,7 @@
"typescript": "^5.4.5"
},
"peerDependencies": {
"blockly": "^11.0.0"
"blockly": "^11.1.2"
Copy link
Contributor

Choose a reason for hiding this comment

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

This will need another bump before being merged correct?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure. The current release is v11.1.1, so I picked a higher number, but I don't know whether the next release will be a minor or patch release.

Copy link
Contributor

Choose a reason for hiding this comment

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

I am also not sure! I don't think we've discussed our next release timeline yet. I'll leave this unresolved for now to make sure we check before releasing.

@BeksOmega BeksOmega requested a review from gonfunko August 23, 2024 23:01
@BeksOmega BeksOmega assigned gonfunko and unassigned BeksOmega Aug 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Content-highlight doesn't update for workspace comment move events
3 participants