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.

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

Jump to bottom

Work around a problem with RTL mutators #1774

Merged
merged 4 commits into from
Apr 12, 2018
Merged

Work around a problem with RTL mutators #1774

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
29571e9
Work around a problem with RTL mutators
rachel-fenichel Apr 11, 2018
40e74d4
Check if the browser's value was plausible, and use it if so.
rachel-fenichel Apr 12, 2018
6606c7c
Fix excess newlines
rachel-fenichel Apr 12, 2018
01e4860
Fix my definition of plausible
rachel-fenichel Apr 12, 2018
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
27 changes: 27 additions & 0 deletions core/flyout_vertical.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,9 @@ Blockly.VerticalFlyout.prototype.position = function() {
var x = targetWorkspaceMetrics.absoluteLeft;
if (this.toolboxPosition_ == Blockly.TOOLBOX_AT_RIGHT) {
x += (targetWorkspaceMetrics.viewWidth - this.width_);
// Save the location of the left edge of the flyout, for use when Firefox
// gets the bounding client rect wrong.
this.leftEdge_ = x;
}
this.positionAt_(this.width_, this.height_, x, y);
};
Expand Down Expand Up @@ -315,6 +318,30 @@ Blockly.VerticalFlyout.prototype.getClientRect = function() {
return new goog.math.Rect(x - BIG_NUM, -BIG_NUM, BIG_NUM + width,
BIG_NUM * 2);
} else { // Right
// Firefox sometimes reports the wrong value for the client rect.
// See https://github.com/google/blockly/issues/1425 and
// https://bugzilla.mozilla.org/show_bug.cgi?id=1066435
if (goog.userAgent.GECKO &&
this.targetWorkspace_ && this.targetWorkspace_.isMutator) {
// The position of the left side of the mutator workspace in pixels
// relative to the window origin.
var targetWsLeftPixels =
this.targetWorkspace_.svgGroup_.getBoundingClientRect().x;
// The client rect is in pixels relative to the window origin. When the
// browser gets the wrong value it reports that the flyout left is the
// same as the mutator workspace left.
// We know that in a mutator workspace with the flyout on the right, the
// visible area of the workspace should be more than ten pixels wide. If
// the browser reports that the flyout is within ten pixels of the left
// side of the workspace, ignore it and manually calculate the value.
if (Math.abs(targetWsLeftPixels - x) < 10) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Will the 10px assumption be invalidated if we allow mutators without flyouts/with empty flyouts, à la #1759. In practice, a mutator with an empty/invisible empty toolbox (no way to add new blocks), probably doesn't a need for a block delete area. Technically, the issues are distinct.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

At that point there should be no flyout, so nothing to call getClientRect on. Then this code won't be hit.

// If we're in a mutator, its scale is always 1, purely because of some
// oddities in our rendering optimizations. The actual scale is the
// same as the scale on the parent workspace.
var scale = this.targetWorkspace_.options.parentWorkspace.scale;
x = x + this.leftEdge_ * scale;
}
}
return new goog.math.Rect(x, -BIG_NUM, BIG_NUM + width, BIG_NUM * 2);
}
};
Expand Down