Skip to content

Allow scrolling Top Instructions despite Blockly Blocks#9753

Merged
Hamms merged 5 commits into
stagingfrom
topInstructions-allow-scrolling-through-blockly-blocks
Aug 1, 2016
Merged

Allow scrolling Top Instructions despite Blockly Blocks#9753
Hamms merged 5 commits into
stagingfrom
topInstructions-allow-scrolling-through-blockly-blocks

Conversation

@Hamms

@Hamms Hamms commented Jul 29, 2016

Copy link
Copy Markdown
Contributor

Formerly, blockly blocks which appear in Top Instructions were
capturing mousewheel events and preventing scrolling when the mouse was
within their blockspace. Now that all ReadOnly blockspaces are being
precisely sized to fit their contents, scrolling readonly blockspaces is
unnecessary and their ability to scroll can safely be removed.

Formerly, blockly blocks which appear in Top Instructions were
capturing mousewheel events and preventing scrolling when the mouse was
within their blockspace. Now that all ReadOnly blockspaces are being
precisely sized to fit their contents, scrolling readonly blockspaces is
unnecessary and their ability to scroll can safely be removed.
if (opt_options.noScrolling) {
blockSpace.scrollbarPair.dispose();
blockSpace.scrollbarPair = null;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we somehow prevent this from being created in the first place, instead of destroying it right after creating the blockSpace here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I was debating. It comes down to whether we view these ReadOnly BlockSpaces as a core feature deserving of refactoring to support, or if it's more of a curiosity we simply make allowances for.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

My instinct would be to make it a core feature; it doesn't sound that hard to do. 😉

@islemaster

Copy link
Copy Markdown
Contributor

Test for the new noScrolling option?

@Hamms

Hamms commented Aug 1, 2016

Copy link
Copy Markdown
Contributor Author

PTAL

}

function test_scrollingCapturesMouseWheelEvents() {
[true, false].forEach(function (scrollingEnabled) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍 Cool.

@islemaster

Copy link
Copy Markdown
Contributor

Beautiful! LGTM.

@Hamms Hamms merged commit 8d90d6d into staging Aug 1, 2016
@Hamms Hamms deleted the topInstructions-allow-scrolling-through-blockly-blocks branch August 1, 2016 21:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants