Skip to content

Fix dragging monitors on small stage #314

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.

Sign up for GitHub

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

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open

Fix dragging monitors on small stage #314

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
078452d
fix: dragging bounds
nimeratus Sep 7, 2025
de58733
refactor: delete the <Box> that used to do the CSS transform
nimeratus Sep 7, 2025
19cb3be
fix: tell scale to react-draggable
nimeratus Sep 7, 2025
6d0344f
fix: round monitor coordinates while dragging
nimeratus Sep 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 29 additions & 31 deletions packages/scratch-gui/src/components/monitor-list/monitor-list.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,39 +11,36 @@ import styles from './monitor-list.css';
const MonitorList = props => (
<Box
// Use static `monitor-overlay` class for bounds of draggables
className={classNames(styles.monitorList, 'monitor-overlay')}
className={classNames(styles.monitorList, styles.monitorListScaler, 'monitor-overlay')}
style={{
width: props.stageSize.width,
height: props.stageSize.height
width: props.stageSize.widthDefault,
height: props.stageSize.heightDefault,
...stageSizeToTransform(props.stageSize)
}}
>
<Box
className={styles.monitorListScaler}
style={stageSizeToTransform(props.stageSize)}
>
{props.monitors.valueSeq().filter(m => m.visible)
.map(monitorData => (
<Monitor
draggable={props.draggable}
height={monitorData.height}
id={monitorData.id}
isDiscrete={monitorData.isDiscrete}
key={monitorData.id}
max={monitorData.sliderMax}
min={monitorData.sliderMin}
mode={monitorData.mode}
opcode={monitorData.opcode}
params={monitorData.params}
spriteName={monitorData.spriteName}
targetId={monitorData.targetId}
value={monitorData.value}
width={monitorData.width}
x={monitorData.x}
y={monitorData.y}
onDragEnd={props.onMonitorChange}
/>
))}
</Box>
{props.monitors.valueSeq().filter(m => m.visible)
.map(monitorData => (
<Monitor
draggable={props.draggable}
scale={props.stageSize.scale}
height={monitorData.height}
id={monitorData.id}
isDiscrete={monitorData.isDiscrete}
key={monitorData.id}
max={monitorData.sliderMax}
min={monitorData.sliderMin}
mode={monitorData.mode}
opcode={monitorData.opcode}
params={monitorData.params}
spriteName={monitorData.spriteName}
targetId={monitorData.targetId}
value={monitorData.value}
width={monitorData.width}
x={monitorData.x}
y={monitorData.y}
onDragEnd={props.onMonitorChange}
/>
))}
</Box>
);

Expand All @@ -55,7 +52,8 @@ MonitorList.propTypes = {
width: PropTypes.number,
height: PropTypes.number,
widthDefault: PropTypes.number,
heightDefault: PropTypes.number
heightDefault: PropTypes.number,
scale: PropTypes.number
}).isRequired
};

Expand Down
4 changes: 4 additions & 0 deletions packages/scratch-gui/src/components/monitor/monitor.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,9 @@ const MonitorComponent = props => (
defaultClassNameDragging={styles.dragging}
disabled={!props.draggable}
onStop={props.onDragEnd}
scale={props.scale} // Used for moving the monitor at the correct speed
grid={[1 * props.scale, 1 * props.scale]} // Tell the Draggable to round the coordinates
// See https://github.com/react-grid-layout/react-draggable/issues/664 on why it's not just [1, 1]
>
<Box
className={styles.monitorContainer}
Expand Down Expand Up @@ -138,6 +141,7 @@ MonitorComponent.propTypes = {
category: PropTypes.oneOf(Object.keys(categoryColorMap)),
componentRef: PropTypes.func.isRequired,
draggable: PropTypes.bool.isRequired,
scale: PropTypes.number,
label: PropTypes.string.isRequired,
mode: PropTypes.oneOf(monitorModes),
onDragEnd: PropTypes.func.isRequired,
Expand Down
9 changes: 9 additions & 0 deletions packages/scratch-gui/src/containers/monitor.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,13 @@ class Monitor extends React.Component {
this.props.removeMonitorRect(this.props.id);
}
handleDragEnd (e, {x, y}) {
// The old version of <Draggable> that Scratch uses doesn't round the coordinates when
// it passes them to the event handler, only when it sets the CSS transform.
// Newer versions of <Draggable> would round the coordinates, but with floating point errors
// so it would still need the manual rounding
x = Math.round(x);
y = Math.round(y);

const newX = parseInt(this.element.style.left, 10) + x;
const newY = parseInt(this.element.style.top, 10) + y;
this.props.onDragEnd(
Expand Down Expand Up @@ -208,6 +215,7 @@ class Monitor extends React.Component {
componentRef={this.setElement}
{...monitorProps}
draggable={this.props.draggable}
scale={this.props.scale}
height={this.props.height}
isDiscrete={this.props.isDiscrete}
max={this.props.max}
Expand All @@ -234,6 +242,7 @@ class Monitor extends React.Component {
Monitor.propTypes = {
addMonitorRect: PropTypes.func.isRequired,
draggable: PropTypes.bool,
scale: PropTypes.number,
height: PropTypes.number,
id: PropTypes.string.isRequired,
intl: intlShape,
Expand Down
Loading