-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
Movement Updates #2247
Movement Updates #2247
Changes from 42 commits
fb0b710
9819aa6
8c6f895
3fe39be
5fd9fa9
42880fd
c95f6de
3242b7a
df44100
0dc9ac7
fa5563d
dbeb7e7
66ce3fd
f2eb3ac
4e839d5
d81c4f4
580361f
c3866c9
6763fdc
3e2da93
1ce779c
89698ee
55c30a7
2c74e2e
7036dd2
549644f
0e4bf72
c8fce12
9f1a588
6fce504
98ae73f
a4b68e2
a3ee2ce
6a5eee1
99166ba
b6374fc
c2b496c
710e18a
82a7cdb
41c8107
1308469
6c75420
94d326d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -232,71 +232,111 @@ Blockly.createMainWorkspace_ = function(svg, options, blockDragSurface, | |
mainWorkspace.translate(0, 0); | ||
Blockly.mainWorkspace = mainWorkspace; | ||
|
||
if (!options.readOnly && !options.hasScrollbars) { | ||
var workspaceChanged = function(e) { | ||
if (!mainWorkspace.isDragging()) { | ||
var metrics = mainWorkspace.getMetrics(); | ||
var edgeLeft = metrics.viewLeft + metrics.absoluteLeft; | ||
var edgeTop = metrics.viewTop + metrics.absoluteTop; | ||
if (metrics.contentTop < edgeTop || | ||
metrics.contentTop + metrics.contentHeight > | ||
metrics.viewHeight + edgeTop || | ||
metrics.contentLeft < | ||
(options.RTL ? metrics.viewLeft : edgeLeft) || | ||
metrics.contentLeft + metrics.contentWidth > (options.RTL ? | ||
metrics.viewWidth : metrics.viewWidth + edgeLeft)) { | ||
// One or more blocks may be out of bounds. Bump them back in. | ||
var MARGIN = 25; | ||
var blocks = mainWorkspace.getTopBlocks(false); | ||
if (!options.readOnly && !mainWorkspace.isMovable()) { | ||
// Helper functions for the workspaceChanged callback. | ||
var getWorkspaceMetrics = function() { | ||
var workspaceMetrics = Object.create(null); | ||
var defaultMetrics = mainWorkspace.getMetrics(); | ||
var scale = mainWorkspace.scale; | ||
|
||
// Get the view metrics in workspace units. | ||
workspaceMetrics.viewLeft = defaultMetrics.viewLeft / scale; | ||
workspaceMetrics.viewTop = defaultMetrics.viewTop / scale; | ||
workspaceMetrics.viewRight = | ||
(defaultMetrics.viewLeft + defaultMetrics.viewWidth) / scale; | ||
workspaceMetrics.viewBottom = | ||
(defaultMetrics.viewTop + defaultMetrics.viewHeight) / scale; | ||
|
||
// Get the exact content metrics (in workspace units), even if the | ||
// content is bounded. | ||
if (mainWorkspace.isContentBounded()) { | ||
// Already in workspace units, no need to divide by scale. | ||
var blocksBoundingBox = mainWorkspace.getBlocksBoundingBox(); | ||
workspaceMetrics.contentLeft = blocksBoundingBox.x; | ||
workspaceMetrics.contentTop = blocksBoundingBox.y; | ||
workspaceMetrics.contentRight = | ||
blocksBoundingBox.x + blocksBoundingBox.width; | ||
workspaceMetrics.contentBottom = | ||
blocksBoundingBox.y + blocksBoundingBox.height; | ||
} else { | ||
workspaceMetrics.contentLeft = defaultMetrics.contentLeft / scale; | ||
workspaceMetrics.contentTop = defaultMetrics.contentTop / scale; | ||
workspaceMetrics.contentRight = | ||
(defaultMetrics.contentLeft + defaultMetrics.contentWidth) / scale; | ||
workspaceMetrics.contentBottom = | ||
(defaultMetrics.contentTop + defaultMetrics.contentHeight) / scale; | ||
} | ||
|
||
return workspaceMetrics; | ||
}; | ||
|
||
var bumpObjects = function(e) { | ||
// We always check isMovable_ again because the original | ||
// "not movable" state of isMovable_ could have been changed. | ||
if (!mainWorkspace.isDragging() && !mainWorkspace.isMovable() && | ||
(Blockly.Events.BUMP_EVENTS.indexOf(e.type) != -1)) { | ||
var metrics = getWorkspaceMetrics(); | ||
if (metrics.contentTop < metrics.viewTop || | ||
metrics.contentBottom > metrics.viewBottom || | ||
metrics.contentLeft < metrics.viewLeft || | ||
metrics.contentRight > metrics.viewRight) { | ||
|
||
// Handle undo. | ||
var oldGroup = null; | ||
if (e) { | ||
oldGroup = Blockly.Events.getGroup(); | ||
Blockly.Events.setGroup(e.group); | ||
} | ||
var movedBlocks = false; | ||
for (var b = 0, block; block = blocks[b]; b++) { | ||
var blockXY = block.getRelativeToSurfaceXY(); | ||
var blockHW = block.getHeightWidth(); | ||
// Bump any block that's above the top back inside. | ||
var overflowTop = edgeTop + MARGIN - blockHW.height - blockXY.y; | ||
if (overflowTop > 0) { | ||
block.moveBy(0, overflowTop); | ||
movedBlocks = true; | ||
} | ||
// Bump any block that's below the bottom back inside. | ||
var overflowBottom = | ||
edgeTop + metrics.viewHeight - MARGIN - blockXY.y; | ||
if (overflowBottom < 0) { | ||
block.moveBy(0, overflowBottom); | ||
movedBlocks = true; | ||
} | ||
// Bump any block that's off the left back inside. | ||
var overflowLeft = MARGIN + edgeLeft - | ||
blockXY.x - (options.RTL ? 0 : blockHW.width); | ||
if (overflowLeft > 0) { | ||
block.moveBy(overflowLeft, 0); | ||
movedBlocks = true; | ||
} | ||
// Bump any block that's off the right back inside. | ||
var overflowRight = edgeLeft + metrics.viewWidth - MARGIN - | ||
blockXY.x + (options.RTL ? blockHW.width : 0); | ||
if (overflowRight < 0) { | ||
block.moveBy(overflowRight, 0); | ||
movedBlocks = true; | ||
} | ||
|
||
switch (e.type) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should a getObjectById() function be added to the workspace so that we can remove this switch statement? Or do you think that's unnecessary? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that's unnecessary--blocks and comments share a few APIs, but not many, so you'd still usually have to check the type of the object that you got back. |
||
case Blockly.Events.BLOCK_CREATE: | ||
case Blockly.Events.BLOCK_MOVE: | ||
var object = mainWorkspace.getBlockById(e.blockId); | ||
break; | ||
case Blockly.Events.COMMENT_CREATE: | ||
case Blockly.Events.COMMENT_MOVE: | ||
var object = mainWorkspace.getCommentById(e.commentId); | ||
break; | ||
} | ||
var objectMetrics = object.getBoundingRectangle(); | ||
|
||
// Bump any object that's above the top back inside. | ||
var overflowTop = metrics.viewTop - objectMetrics.topLeft.y; | ||
if (overflowTop > 0) { | ||
object.moveBy(0, overflowTop); | ||
} | ||
|
||
// Bump any object that's below the bottom back inside. | ||
var overflowBottom = metrics.viewBottom - objectMetrics.bottomRight.y; | ||
if (overflowBottom < 0) { | ||
object.moveBy(0, overflowBottom); | ||
} | ||
|
||
// Bump any object that's off the left back inside. | ||
var overflowLeft = metrics.viewLeft - objectMetrics.topLeft.x; | ||
if (overflowLeft > 0) { | ||
object.moveBy(overflowLeft, 0); | ||
} | ||
|
||
// Bump any object that's off the right back inside. | ||
var overflowRight = metrics.viewRight - objectMetrics.bottomRight.x; | ||
if (overflowRight < 0) { | ||
object.moveBy(overflowRight, 0); | ||
} | ||
|
||
if (e) { | ||
if (!e.group && movedBlocks) { | ||
console.log('WARNING: Moved blocks in bounds but there was no event group.' | ||
+ ' This may break undo.'); | ||
if (!e.group) { | ||
console.log('WARNING: Moved object in bounds but there was no' + | ||
' event group. This may break undo.'); | ||
} | ||
Blockly.Events.setGroup(oldGroup); | ||
} | ||
} | ||
} | ||
}; | ||
mainWorkspace.addChangeListener(workspaceChanged); | ||
mainWorkspace.addChangeListener(bumpObjects); | ||
} | ||
|
||
// The SVG is now fully assembled. | ||
Blockly.svgResize(mainWorkspace); | ||
Blockly.WidgetDiv.createDom(); | ||
|
@@ -339,12 +379,6 @@ Blockly.init_ = function(mainWorkspace) { | |
mainWorkspace.flyout_.init(mainWorkspace); | ||
mainWorkspace.flyout_.show(options.languageTree.childNodes); | ||
mainWorkspace.flyout_.scrollToStart(); | ||
// Translate the workspace sideways to avoid the fixed flyout. | ||
mainWorkspace.scrollX = mainWorkspace.flyout_.width_; | ||
if (options.toolboxPosition == Blockly.TOOLBOX_AT_RIGHT) { | ||
mainWorkspace.scrollX *= -1; | ||
} | ||
mainWorkspace.translate(mainWorkspace.scrollX, 0); | ||
} | ||
} | ||
|
||
|
@@ -356,9 +390,31 @@ Blockly.init_ = function(mainWorkspace) { | |
mainWorkspace.zoomControls_.init(verticalSpacing); | ||
} | ||
|
||
if (options.hasScrollbars) { | ||
if (options.moveOptions && options.moveOptions.scrollbars) { | ||
mainWorkspace.scrollbar = new Blockly.ScrollbarPair(mainWorkspace); | ||
mainWorkspace.scrollbar.resize(); | ||
} else { | ||
mainWorkspace.setMetrics({x: .5, y: .5}); | ||
} | ||
|
||
if (mainWorkspace.flyout_) { | ||
// Translate the workspace sideways to avoid the fixed flyout. | ||
switch (mainWorkspace.toolboxPosition) { | ||
case Blockly.TOOLBOX_AT_LEFT: | ||
mainWorkspace.scrollX = | ||
mainWorkspace.RTL ? 0 : mainWorkspace.flyout_.width_; | ||
break; | ||
case Blockly.TOOLBOX_AT_RIGHT: | ||
mainWorkspace.scrollX = | ||
mainWorkspace.RTL ? -mainWorkspace.flyout_.width_ : 0; | ||
break; | ||
case Blockly.TOOLBOX_AT_TOP: | ||
mainWorkspace.scrollY = mainWorkspace.flyout_.height_; | ||
break; | ||
// If the toolbox is at the top left (workspace origin) is untouched, | ||
// so no need to include it. | ||
} | ||
mainWorkspace.translate(mainWorkspace.scrollX, mainWorkspace.scrollY); | ||
} | ||
|
||
// Load the sounds. | ||
|
@@ -380,6 +436,9 @@ Blockly.init_ = function(mainWorkspace) { | |
*/ | ||
Blockly.inject.bindDocumentEvents_ = function() { | ||
if (!Blockly.documentEventsBound_) { | ||
Blockly.bindEventWithChecks_(document, 'scroll', null, | ||
Blockly.mainWorkspace.updateInverseScreenCTM | ||
.bind(Blockly.mainWorkspace)); | ||
Blockly.bindEventWithChecks_(document, 'keydown', null, Blockly.onKeyDown_); | ||
// longStop needs to run to stop the context menu from showing up. It | ||
// should run regardless of what other touch event handlers have run. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like it's doing a lot of work and checks that normally happen in
getMetrics
. Why is all the new math here?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The bumpObjects function needs a pretty uncommon set of metrics to work properly:
A) It needs everything in workspace units rather than pixel units.
B) It needs the un-bounded size of the content, even if the content is bounded.
Because the needs were so specific to the bumpObjects function, I wrote everything inside inject, but you're right it is more of a workspace thing. (Also I couldn't come up with a good name, getMetricsUnboundedWorkspaceUnits is a bit longwinded)
I can:
A) Leave as is.
B) Add a new function to the workspace_svg, but I would appreciate a name suggestion.
C) Change the signature of getTopLevelWorkspaceMetrics_() to (bool opt_workspaceUnits, bool opt_forceUnbounded), although this might be a bit confusing because I believe the signature of getMetrics would change for flyouts vs workspaces.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leave as-is for this change, and add a TODO + github isue to circle back and see if we can move most of it out of inject.