Skip to content

Conversation

nimeratus
Copy link

Resolves

What Github issue does this resolve (please include link)?

There is a PR that solves this issue in scratch-gui but I think this way it can be solved with much less changes (out of the 42 added rows 22 is just changing white space for eslint, only the other 20 rows are actual changes)

Proposed Changes

Describe what this Pull Request does

  1. Resize the dragging bounds with the same CSS transform that resizes the monitors, instead of changing the width/height CSS properties
  2. Remove the <Box> that used to do the CSS transform
  3. Set the scale property on the <Draggable>
  4. Prevent the monitor from going to fractional coordinates even if scale is a fraction
    • set the grid property on the <Draggable>
    • round the coordinates in the handleDragEnd event handler

Reason for Changes

Explain why these changes should be made

  1. This way CSS word wrapping and react-draggable will let the monitors go to the edges of the stage
  2. It doesn't do the transform anymore and I don't think it did anything else
  3. This way react-draggable knows the speed at which it should move the monitors
  4. The existing code expects the monitor coordinates to be integers here and there might be other places where it expects integers
    • round coordinates in CSS transform so the monitor appears to move to the position where Scratch will think it is
    • the <Draggable> doesn't round the coordinates for the event handler

Test Coverage

Please show how you have added tests to cover your changes

I've run npm test and it accepted it (I think) (it didn't say "error" like when it didn't like the identation)

I've tried it out in these browsers and it seems to work

  • Windows 10, Chrome 139
  • Android, Chrome 138

I've temporarily added this to handleDragEnd for testing and it didn't cause any popups:

if(this.element.style.transform!==`translate(${Math.round(x)}px, ${Math.round(y)}px)`) {
    alert(`monitor position doesn't match CSS: [${x},${y}] - ${this.element.style.transform}`);
}

Use the CSS transform to resize the dragging bounds,
instead of changing the width/height properties.
This way CSS word-wrapping and react-draggable will
let the monitors go to the edges of the stage.
Forward the scale info through <Monitor> and <MonitorComponent> to <Draggable>.
This way the monitor will move at the same speed as the cursor.
because the existing code seems to expect monitor coordinates to be integers,
but now that `scale` isn't 1, they could become fractions

Set `grid` on <Draggable> - This way it will round the coordinates in the CSS transform
Round coordinates in handleDragEnd
@nimeratus nimeratus requested a review from a team as a code owner September 11, 2025 08:37
Copy link


Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just posting a Pull Request Comment same as the below format.


I have read the CLA Document and I hereby sign the CLA


You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot.

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.

1 participant