google / blockly

The web-based visual programming editor.
https://developers.google.com/blockly/
Apache License 2.0
12.37k stars 3.69k forks source link

RenderedWorkspaceComments setDeleteStyle not working #8413

Closed changminbark closed 3 weeks ago

changminbark commented 1 month ago

Check for duplicates

Description

Currently, the visual style of the cursor to show that whatever comment it is dragging is going to be deleted when hovering over the trash can is not showing up.

Multiselect Plugin context: It only works for blocks even though I am calling the setDeleteStyle(true) for each of the subdraggables. I also checked whether the comment was deletable, which was true, so I am not sure why the visual style is not being updated. image

Beka has already told me that this is a CSS bug.

Reproduction steps

Stack trace

No response

Screenshots

No response

Browsers

No response

cpcallen commented 1 month ago

Hey @BeksOmega: can you add a description of what needs to be fixed here and then remove the triage label? Thanks!

srivastavaarpit977 commented 1 month ago

Hey @BeksOmega : I want to work on this issue...could you please assign this issue to me !!

johnnesky commented 1 month ago

Hi, I was also looking into this issue today. I narrowed it down to: https://github.com/google/blockly/commit/e4b734c0db537032a36ae907ba7af20bb5bb41f7

Deleting cursor: grab; from this style fixes the above issue in my local testing. A parent element has the .blocklyDraggable CSS class which provides the same cursor, but the parent element does a better job of switching the cursor (to a closed fist when the mouse is pressed, and a hand with a red X when hovering over the trashcan).

However, I don't know what problem this commit was trying to solve, and it's not clear from looking at the commit description why it was necessary.

johnnesky commented 1 month ago

Aha I think I figured it out. The style added 2 months ago in https://github.com/google/blockly/commit/e4b734c0db537032a36ae907ba7af20bb5bb41f7 was necessary at the time because there wasn't any other style at the time that provided the drag cursor. However, https://github.com/google/blockly/commit/4cdca28fe5c92ce883cc597d517d01531bf8770e was added 2 weeks ago and provides the drag cursor for the parent element along with the necessary variations, making the previous commit obsolete.

johnnesky commented 1 month ago

@srivastavaarpit977, I've assigned the issue to you. It looks like the solution is straightforward, see my comments. We just need to revert https://github.com/google/blockly/commit/e4b734c0db537032a36ae907ba7af20bb5bb41f7