Work around a problem with RTL mutators #1774
Conversation
|
Updated to use the browser's value if it looks reasonable. This should protect us if Firefox eventually fixes the bug. |
core/flyout_vertical.js
Outdated
| } | ||
| this.positionAt_(this.width_, this.height_, x, y); | ||
|
|
||
|
|
|
Yeah, I'm not at all happy with it but it's been blocking for too long, and we do know exactly what conditions trigger it. We may eventually fix the root cause the next time we work on the general DOM structure, but that's not going to be soon. |
core/flyout_vertical.js
Outdated
| @@ -155,8 +155,13 @@ 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 | |||
There was a problem hiding this comment.
Firefox is usually capitalized.
core/flyout_vertical.js
Outdated
| } | ||
| this.positionAt_(this.width_, this.height_, x, y); | ||
|
|
||
|
|
|
Updated the math to check whether the browser's value is right, and added a comment to explain what's going on. Thanks for the check @picklesrus! Tested in Chrome by disabling the user-agent check. In Chrome on RTL (when the browser isn't lying) it does the right thing as well. |
| // 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) { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
At that point there should be no flyout, so nothing to call getClientRect on. Then this code won't be hit.
The basics
The details
Resolves
Fixes #1425
Proposed Changes
Change the calculation of the delete area for flyouts in mutators in RTL in firefox. Explicitly add an offset from the left side of the mutator.
Reason for Changes
See #1425, but TLDR: Firefox is reporting the wrong value, and that bug has been around for years. We surfaced it with a combination of changes to our DOM structure and changes to delete logic.
Test Coverage
Tested in the playground in Firefox in RTL mode, with the if/else mutator.
Tested at a variety of scales.
Also tested in LTR mode to make sure there are no regressions.
Tested in Chrome to confirm no regressions.
Tested on:
Additional Information
As soon as we merge this they're going to fix Firefox and this will all break again :(
This should unblock the Blockly Games release.