From 1067cab697cb4094c8adcbe316a1d8f45e0b6caa Mon Sep 17 00:00:00 2001 From: Neil Fraser Date: Sun, 3 May 2020 12:43:12 -0700 Subject: [PATCH 01/30] Remove cargo-culted bloat from CSS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `goog-menuitem-icon` and `goog-menuitem-noicon` classes are not present in Blockly. Blockly doesn’t support the CSS compiler, so #noflip has no effect. Shorten uncompressible warning string. --- core/css.js | 75 ++++++++++------------------------------------------- 1 file changed, 14 insertions(+), 61 deletions(-) diff --git a/core/css.js b/core/css.js index 8a29f4b6a97..1382f51837d 100644 --- a/core/css.js +++ b/core/css.js @@ -74,13 +74,13 @@ Blockly.Css.inject = function(hasCss, pathToMedia) { /** * Set the cursor to be displayed when over something draggable. - * See See https://github.com/google/blockly/issues/981 for context. + * See https://github.com/google/blockly/issues/981 for context. * @param {*} _cursor Enum. * @deprecated April 2017. */ Blockly.Css.setCursor = function(_cursor) { console.warn('Deprecated call to Blockly.Css.setCursor. ' + - 'See https://github.com/google/blockly/issues/981 for context'); + 'See issue #981 for context'); }; /** @@ -169,11 +169,11 @@ Blockly.Css.CONTENT = [ 'background-color: #fff;', 'border-radius: 2px;', 'padding: 4px;', - 'box-shadow: 0px 0px 3px 1px rgba(0,0,0,.3);', + 'box-shadow: 0 0 3px 1px rgba(0,0,0,.3);', '}', '.blocklyDropDownDiv.focused {', - 'box-shadow: 0px 0px 6px 1px rgba(0,0,0,.3);', + 'box-shadow: 0 0 6px 1px rgba(0,0,0,.3);', '}', '.blocklyDropDownContent {', @@ -481,9 +481,7 @@ Blockly.Css.CONTENT = [ '}', '.blocklyWidgetDiv .goog-option-selected .goog-menuitem-checkbox,', - '.blocklyWidgetDiv .goog-option-selected .goog-menuitem-icon,', - '.blocklyDropDownDiv .goog-option-selected .goog-menuitem-checkbox,', - '.blocklyDropDownDiv .goog-option-selected .goog-menuitem-icon {', + '.blocklyDropDownDiv .goog-option-selected .goog-menuitem-checkbox {', 'background: url(<<>>/sprites.png) no-repeat -48px -16px;', '}', @@ -516,11 +514,11 @@ Blockly.Css.CONTENT = [ 'overflow-x: hidden;', 'max-height: 100%;', 'z-index: 20000;', /* Arbitrary, but some apps depend on it... */ - 'box-shadow: 0px 0px 3px 1px rgba(0,0,0,.3);', + 'box-shadow: 0 0 3px 1px rgba(0,0,0,.3);', '}', '.blocklyWidgetDiv .goog-menu.focused {', - 'box-shadow: 0px 0px 6px 1px rgba(0,0,0,.3);', + 'box-shadow: 0 0 6px 1px rgba(0,0,0,.3);', '}', '.blocklyDropDownDiv .goog-menu {', @@ -546,18 +544,6 @@ Blockly.Css.CONTENT = [ /** * State: resting. - * - * NOTE(mleibman,chrishenry): - * The RTL support in Closure is provided via two mechanisms -- "rtl" CSS - * classes and BiDi flipping done by the CSS compiler. Closure supports RTL - * with or without the use of the CSS compiler. In order for them not to - * conflict with each other, the "rtl" CSS classes need to have the #noflip - * annotation. The non-rtl counterparts should ideally have them as well, - * but, since .goog-menuitem existed without .goog-menuitem-rtl for so long - * before being added, there is a risk of people having templates where they - * are not rendering the .goog-menuitem-rtl class when in RTL and instead - * rely solely on the BiDi flipping by the CSS compiler. That's why we're - * not adding the #noflip to .goog-menuitem. */ '.blocklyWidgetDiv .goog-menuitem,', '.blocklyDropDownDiv .goog-menuitem {', @@ -573,26 +559,13 @@ Blockly.Css.CONTENT = [ 'cursor: pointer;', '}', - /* If a menu doesn't have checkable items or items with icons, - * remove padding. - */ - '.blocklyWidgetDiv .goog-menu-nocheckbox .goog-menuitem,', - '.blocklyWidgetDiv .goog-menu-noicon .goog-menuitem,', - '.blocklyDropDownDiv .goog-menu-nocheckbox .goog-menuitem,', - '.blocklyDropDownDiv .goog-menu-noicon .goog-menuitem {', - 'padding-left: 12px;', - '}', - '.blocklyWidgetDiv .goog-menuitem-content,', '.blocklyDropDownDiv .goog-menuitem-content {', 'font-family: Arial, sans-serif;', 'font-size: 13px;', '}', - '.blocklyWidgetDiv .goog-menuitem-content {', - 'color: #000;', - '}', - + '.blocklyWidgetDiv .goog-menuitem-content,', '.blocklyDropDownDiv .goog-menuitem-content {', 'color: #000;', '}', @@ -608,12 +581,6 @@ Blockly.Css.CONTENT = [ 'color: #ccc !important;', '}', - '.blocklyWidgetDiv .goog-menuitem-disabled .goog-menuitem-icon,', - '.blocklyDropDownDiv .goog-menuitem-disabled .goog-menuitem-icon {', - 'opacity: .3;', - 'filter: alpha(opacity=30);', - '}', - /* State: hover. */ '.blocklyWidgetDiv .goog-menuitem-highlight ,', '.blocklyDropDownDiv .goog-menuitem-highlight {', @@ -622,9 +589,7 @@ Blockly.Css.CONTENT = [ /* State: selected/checked. */ '.blocklyWidgetDiv .goog-menuitem-checkbox,', - '.blocklyWidgetDiv .goog-menuitem-icon,', - '.blocklyDropDownDiv .goog-menuitem-checkbox,', - '.blocklyDropDownDiv .goog-menuitem-icon {', + '.blocklyDropDownDiv .goog-menuitem-checkbox {', 'background-repeat: no-repeat;', 'height: 16px;', 'left: 6px;', @@ -634,32 +599,20 @@ Blockly.Css.CONTENT = [ 'width: 16px;', '}', - /* BiDi override for the selected/checked state. */ - /* #noflip */ - '.blocklyWidgetDiv .goog-menuitem-rtl .goog-menuitem-checkbox,', - '.blocklyWidgetDiv .goog-menuitem-rtl .goog-menuitem-icon,', - '.blocklyDropDownDiv .goog-menuitem-rtl .goog-menuitem-checkbox,', - '.blocklyDropDownDiv .goog-menuitem-rtl .goog-menuitem-icon {', - /* Flip left/right positioning. */ - 'left: auto;', - 'right: 6px;', - '}', - '.blocklyWidgetDiv .goog-option-selected .goog-menuitem-checkbox,', - '.blocklyWidgetDiv .goog-option-selected .goog-menuitem-icon,', - '.blocklyDropDownDiv .goog-option-selected .goog-menuitem-checkbox,', - '.blocklyDropDownDiv .goog-option-selected .goog-menuitem-icon {', + '.blocklyDropDownDiv .goog-option-selected .goog-menuitem-checkbox {', 'position: static;', /* Scroll with the menu. */ 'float: left;', 'margin-left: -24px;', '}', '.blocklyWidgetDiv .goog-menuitem-rtl .goog-menuitem-checkbox,', - '.blocklyWidgetDiv .goog-menuitem-rtl .goog-menuitem-icon,', - '.blocklyDropDownDiv .goog-menuitem-rtl .goog-menuitem-checkbox,', - '.blocklyDropDownDiv .goog-menuitem-rtl .goog-menuitem-icon {', + '.blocklyDropDownDiv .goog-menuitem-rtl .goog-menuitem-checkbox {', 'float: right;', 'margin-right: -24px;', + /* Flip left/right positioning. */ + 'left: auto;', + 'right: 6px;', '}', '.blocklyComputeCanvas {', From 034768c47c49cd6c46467332b6c034a1e5518ff5 Mon Sep 17 00:00:00 2001 From: Neil Fraser Date: Sun, 3 May 2020 14:29:26 -0700 Subject: [PATCH 02/30] More CSS simplifications MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Also remove the “Copied from Closure” notes. These were intended so that the CSS could be easily updated as the Closure Library evolved. We are no longer linked to the Closure Library. --- core/components/menu/menu.js | 2 +- core/components/menu/menuitem.js | 2 +- core/css.js | 119 ++++++++----------------------- 3 files changed, 31 insertions(+), 92 deletions(-) diff --git a/core/components/menu/menu.js b/core/components/menu/menu.js index 79ed646b69b..3f1385fda94 100644 --- a/core/components/menu/menu.js +++ b/core/components/menu/menu.js @@ -92,7 +92,7 @@ Blockly.Menu.prototype.createDom = function() { this.setElementInternal(element); // Set class - element.className = 'goog-menu goog-menu-vertical blocklyNonSelectable'; + element.className = 'goog-menu blocklyNonSelectable'; element.tabIndex = 0; // Initialize ARIA role. diff --git a/core/components/menu/menuitem.js b/core/components/menu/menuitem.js index ae163f30b12..2288c4029e7 100644 --- a/core/components/menu/menuitem.js +++ b/core/components/menu/menuitem.js @@ -64,7 +64,7 @@ Blockly.MenuItem.prototype.createDom = function() { this.setElementInternal(element); // Set class and style - element.className = 'goog-menuitem goog-option ' + + element.className = 'goog-menuitem ' + (!this.enabled_ ? 'goog-menuitem-disabled ' : '') + (this.checked_ ? 'goog-option-selected ' : '') + (this.rightToLeft_ ? 'goog-menuitem-rtl ' : ''); diff --git a/core/css.js b/core/css.js index 1382f51837d..7ba7e7363a0 100644 --- a/core/css.js +++ b/core/css.js @@ -150,8 +150,7 @@ Blockly.Css.CONTENT = [ 'box-shadow: 4px 4px 20px 1px rgba(0,0,0,.15);', 'color: #000;', 'display: none;', - 'font-family: sans-serif;', - 'font-size: 9pt;', + 'font: 9pt sans-serif;', 'opacity: .9;', 'padding: 2px;', 'position: absolute;', @@ -333,7 +332,8 @@ Blockly.Css.CONTENT = [ Don't allow users to select text. It gets annoying when trying to drag a block and selected text moves instead. */ - '.blocklySvg text, .blocklyBlockDragSurface text {', + '.blocklySvg text,', + '.blocklyBlockDragSurface text {', 'user-select: none;', '-ms-user-select: none;', '-webkit-user-select: none;', @@ -416,7 +416,8 @@ Blockly.Css.CONTENT = [ 'z-index: 30;', '}', - '.blocklyScrollbarHorizontal, .blocklyScrollbarVertical {', + '.blocklyScrollbarHorizontal,', + '.blocklyScrollbarVertical {', 'position: absolute;', 'outline: none;', '}', @@ -449,6 +450,22 @@ Blockly.Css.CONTENT = [ 'background: #faa;', '}', + '.blocklyVerticalMarker {', + 'stroke-width: 3px;', + 'fill: rgba(255,255,255,.5);', + 'pointer-events: none', + '}', + + '.blocklyComputeCanvas {', + 'position: absolute;', + 'width: 0;', + 'height: 0;', + '}', + + '.blocklyNoPointerEvents {', + 'pointer-events: none;', + '}', + '.blocklyContextMenu {', 'border-radius: 4px;', 'max-height: 100%;', @@ -466,7 +483,6 @@ Blockly.Css.CONTENT = [ '}', /* BiDi override for the resting state. */ - /* #noflip */ '.blocklyWidgetDiv .blocklyDropdownMenu .goog-menuitem.goog-menuitem-rtl,', '.blocklyDropDownDiv .blocklyDropdownMenu .goog-menuitem.goog-menuitem-rtl {', /* Flip left/right padding for BiDi. */ @@ -474,37 +490,10 @@ Blockly.Css.CONTENT = [ 'padding-right: 28px;', '}', - '.blocklyVerticalMarker {', - 'stroke-width: 3px;', - 'fill: rgba(255,255,255,.5);', - 'pointer-events: none', - '}', - - '.blocklyWidgetDiv .goog-option-selected .goog-menuitem-checkbox,', - '.blocklyDropDownDiv .goog-option-selected .goog-menuitem-checkbox {', - 'background: url(<<>>/sprites.png) no-repeat -48px -16px;', - '}', - - /* Copied from: goog/css/menu.css */ - /* - * Copyright 2009 The Closure Library Authors. All Rights Reserved. - * - * Use of this source code is governed by the Apache License, Version 2.0. - * See the COPYING file for details. - */ - - /** - * Standard styling for menus created by goog.ui.MenuRenderer. - * - * @author attila@google.com (Attila Bodis) - */ - '.blocklyWidgetDiv .goog-menu {', 'background: #fff;', - 'border-color: transparent;', - 'border-style: solid;', - 'border-width: 1px;', - 'cursor: default;', + 'border: 1px solid transparent;', + 'box-shadow: 0 0 3px 1px rgba(0,0,0,.3);', 'font: normal 13px Arial, sans-serif;', 'margin: 0;', 'outline: none;', @@ -514,7 +503,6 @@ Blockly.Css.CONTENT = [ 'overflow-x: hidden;', 'max-height: 100%;', 'z-index: 20000;', /* Arbitrary, but some apps depend on it... */ - 'box-shadow: 0 0 3px 1px rgba(0,0,0,.3);', '}', '.blocklyWidgetDiv .goog-menu.focused {', @@ -522,63 +510,30 @@ Blockly.Css.CONTENT = [ '}', '.blocklyDropDownDiv .goog-menu {', - 'cursor: default;', 'font: normal 13px "Helvetica Neue", Helvetica, sans-serif;', 'outline: none;', 'z-index: 20000;', /* Arbitrary, but some apps depend on it... */ '}', - /* Copied from: goog/css/menuitem.css */ - /* - * Copyright 2009 The Closure Library Authors. All Rights Reserved. - * - * Use of this source code is governed by the Apache License, Version 2.0. - * See the COPYING file for details. - */ - - /** - * Standard styling for menus created by goog.ui.MenuItemRenderer. - * - * @author attila@google.com (Attila Bodis) - */ - - /** - * State: resting. - */ + /* State: resting. */ '.blocklyWidgetDiv .goog-menuitem,', '.blocklyDropDownDiv .goog-menuitem {', + 'border: none;', 'color: #000;', - 'font: normal 13px Arial, sans-serif;', + 'cursor: pointer;', 'list-style: none;', 'margin: 0;', /* 7em on the right for shortcut. */ 'min-width: 7em;', - 'border: none;', 'padding: 6px 15px;', 'white-space: nowrap;', - 'cursor: pointer;', - '}', - - '.blocklyWidgetDiv .goog-menuitem-content,', - '.blocklyDropDownDiv .goog-menuitem-content {', - 'font-family: Arial, sans-serif;', - 'font-size: 13px;', - '}', - - '.blocklyWidgetDiv .goog-menuitem-content,', - '.blocklyDropDownDiv .goog-menuitem-content {', - 'color: #000;', '}', /* State: disabled. */ '.blocklyWidgetDiv .goog-menuitem-disabled,', '.blocklyDropDownDiv .goog-menuitem-disabled {', - 'cursor: inherit;', - '}', - - '.blocklyWidgetDiv .goog-menuitem-disabled .goog-menuitem-content,', - '.blocklyDropDownDiv .goog-menuitem-disabled .goog-menuitem-content {', 'color: #ccc !important;', + 'cursor: inherit;', '}', /* State: hover. */ @@ -590,39 +545,23 @@ Blockly.Css.CONTENT = [ /* State: selected/checked. */ '.blocklyWidgetDiv .goog-menuitem-checkbox,', '.blocklyDropDownDiv .goog-menuitem-checkbox {', - 'background-repeat: no-repeat;', 'height: 16px;', - 'left: 6px;', 'position: absolute;', - 'right: auto;', - 'vertical-align: middle;', 'width: 16px;', '}', '.blocklyWidgetDiv .goog-option-selected .goog-menuitem-checkbox,', '.blocklyDropDownDiv .goog-option-selected .goog-menuitem-checkbox {', - 'position: static;', /* Scroll with the menu. */ + 'background: url(<<>>/sprites.png) no-repeat -48px -16px;', 'float: left;', 'margin-left: -24px;', + 'position: static;', /* Scroll with the menu. */ '}', '.blocklyWidgetDiv .goog-menuitem-rtl .goog-menuitem-checkbox,', '.blocklyDropDownDiv .goog-menuitem-rtl .goog-menuitem-checkbox {', 'float: right;', 'margin-right: -24px;', - /* Flip left/right positioning. */ - 'left: auto;', - 'right: 6px;', - '}', - - '.blocklyComputeCanvas {', - 'position: absolute;', - 'width: 0;', - 'height: 0;', - '}', - - '.blocklyNoPointerEvents {', - 'pointer-events: none;', '}', /* eslint-enable indent */ ]; From fe10b21bb06211d03ce05ecc6e9c1f145f182ca2 Mon Sep 17 00:00:00 2001 From: Neil Fraser Date: Sun, 3 May 2020 15:17:37 -0700 Subject: [PATCH 03/30] Make menu ID map manditory. Blockly never uses the lazy-creation feature. Remove for simplicity. --- core/components/menu/menu.js | 48 ++++++++++++++---------------------- 1 file changed, 18 insertions(+), 30 deletions(-) diff --git a/core/components/menu/menu.js b/core/components/menu/menu.js index 3f1385fda94..35b9154b253 100644 --- a/core/components/menu/menu.js +++ b/core/components/menu/menu.js @@ -78,6 +78,15 @@ Blockly.Menu = function() { * @private */ this.onKeyDownWrapper_ = null; + + /** + * Map of DOM IDs to child menuitems. Each key is the DOM ID of a child + * menuitems's root element; each value is a reference to the child menu + * item itself. + * @type {!Object} + * @private + */ + this.childElementIdMap_ = {}; }; Blockly.utils.object.inherits(Blockly.Menu, Blockly.Component); @@ -214,18 +223,9 @@ Blockly.Menu.prototype.detachEvents_ = function() { // Child component management. -/** - * Map of DOM IDs to child menuitems. Each key is the DOM ID of a child - * menuitems's root element; each value is a reference to the child menu - * item itself. - * @type {?Object} - * @private - */ -Blockly.Menu.prototype.childElementIdMap_ = null; - /** * Creates a DOM ID for the child menuitem and registers it to an internal - * hash table to be able to find it fast by id. + * hash table to be able to find it fast by ID. * @param {Blockly.Component} child The child menuitem. Its root element has * to be created yet. * @private @@ -233,14 +233,8 @@ Blockly.Menu.prototype.childElementIdMap_ = null; Blockly.Menu.prototype.registerChildId_ = function(child) { // Map the DOM ID of the menuitem's root element to the menuitem itself. var childElem = child.getElement(); - // If the menuitem's root element doesn't have a DOM ID assign one. var id = childElem.id || (childElem.id = child.getId()); - - // Lazily create the child element ID map on first use. - if (!this.childElementIdMap_) { - this.childElementIdMap_ = {}; - } this.childElementIdMap_[id] = child; }; @@ -252,17 +246,13 @@ Blockly.Menu.prototype.registerChildId_ = function(child) { * @protected */ Blockly.Menu.prototype.getMenuItem = function(node) { - // Ensure that this menu actually has child menuitems before - // looking up the menuitem. - if (this.childElementIdMap_) { - var elem = this.getElement(); - while (node && node !== elem) { - var id = node.id; - if (id in this.childElementIdMap_) { - return this.childElementIdMap_[id]; - } - node = node.parentNode; + var elem = this.getElement(); + while (node && node !== elem) { + var id = node.id; + if (id in this.childElementIdMap_) { + return this.childElementIdMap_[id]; } + node = node.parentNode; } return null; }; @@ -372,8 +362,7 @@ Blockly.Menu.prototype.highlightPrevious = function() { Blockly.Menu.prototype.highlightHelper = function(fn, startIndex) { // If the start index is -1 (meaning there's nothing currently highlighted), // try starting from the currently open item, if any. - var curIndex = - startIndex < 0 ? -1 : startIndex; + var curIndex = startIndex < 0 ? -1 : startIndex; var numItems = this.getChildCount(); curIndex = fn.call(this, curIndex, numItems); @@ -486,8 +475,7 @@ Blockly.Menu.prototype.handleMouseLeave_ = function(_e) { * @protected */ Blockly.Menu.prototype.handleKeyEvent = function(e) { - if (this.getChildCount() != 0 && - this.handleKeyEventInternal(e)) { + if (this.getChildCount() != 0 && this.handleKeyEventInternal(e)) { e.preventDefault(); e.stopPropagation(); return true; From f2510420c37d00031bcbd0e5daa6c17b7ac3aaab Mon Sep 17 00:00:00 2001 From: Neil Fraser Date: Sun, 3 May 2020 15:44:30 -0700 Subject: [PATCH 04/30] Fix bug (in prod) where menu highlighting is lost MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, open playground. Right-click on workspace. Mouse-over “Add comment” (it highlights). Mouse over “Download screenshot” (disabled option). Mouse over “Add comment” (highlighting is lost). Also remove `canHighlightItem` helper function. In theory this helps abstract the concept of non-highlightable options. But in practice it was only called in one of the several places that it should have been. This was a false abstraction. --- core/components/menu/menu.js | 39 +++++++++--------------------------- 1 file changed, 9 insertions(+), 30 deletions(-) diff --git a/core/components/menu/menu.js b/core/components/menu/menu.js index 35b9154b253..083a363b8c9 100644 --- a/core/components/menu/menu.js +++ b/core/components/menu/menu.js @@ -259,23 +259,11 @@ Blockly.Menu.prototype.getMenuItem = function(node) { // Highlight management. -/** - * Unhighlight the current highlighted item. - * @protected - */ -Blockly.Menu.prototype.unhighlightCurrent = function() { - var highlighted = this.getHighlighted(); - if (highlighted) { - highlighted.setHighlighted(false); - } -}; - /** * Clears the currently highlighted item. * @protected */ Blockly.Menu.prototype.clearHighlighted = function() { - this.unhighlightCurrent(); this.setHighlightedIndex(-1); }; @@ -330,10 +318,11 @@ Blockly.Menu.prototype.setHighlighted = function(item) { * @package */ Blockly.Menu.prototype.highlightNext = function() { - this.unhighlightCurrent(); + var highlightedIndex = this.highlightedIndex_; + this.clearHighlighted(); this.highlightHelper(function(index, max) { return (index + 1) % max; - }, this.highlightedIndex_); + }, highlightedIndex); }; /** @@ -342,11 +331,12 @@ Blockly.Menu.prototype.highlightNext = function() { * @package */ Blockly.Menu.prototype.highlightPrevious = function() { - this.unhighlightCurrent(); + var highlightedIndex = this.highlightedIndex_; + this.clearHighlighted(); this.highlightHelper(function(index, max) { index--; return index < 0 ? max - 1 : index; - }, this.highlightedIndex_); + }, highlightedIndex); }; /** @@ -369,7 +359,7 @@ Blockly.Menu.prototype.highlightHelper = function(fn, startIndex) { var visited = 0; while (visited <= numItems) { var menuItem = /** @type {Blockly.MenuItem} */ (this.getChildAt(curIndex)); - if (menuItem && this.canHighlightItem(menuItem)) { + if (menuItem && menuItem.isEnabled()) { this.setHighlightedIndex(curIndex); return true; } @@ -379,16 +369,6 @@ Blockly.Menu.prototype.highlightHelper = function(fn, startIndex) { return false; }; -/** - * Returns whether the given item can be highlighted. - * @param {Blockly.MenuItem} item The item to check. - * @return {boolean} Whether the item can be highlighted. - * @protected - */ -Blockly.Menu.prototype.canHighlightItem = function(item) { - return item.isEnabled(); -}; - // Mouse events. /** @@ -406,11 +386,10 @@ Blockly.Menu.prototype.handleMouseOver_ = function(e) { if (currentHighlighted === menuItem) { return; } - - this.unhighlightCurrent(); + this.clearHighlighted(); this.setHighlighted(menuItem); } else { - this.unhighlightCurrent(); + this.clearHighlighted(); } } }; From d79b8d1f6dc5faac5feb607b0885def6bb780b22 Mon Sep 17 00:00:00 2001 From: Neil Fraser Date: Sun, 3 May 2020 16:20:18 -0700 Subject: [PATCH 05/30] Add support for Space/PgUp/PgDn/Home/End to menus --- core/components/menu/menu.js | 57 ++++++++++++++++++++++++------------ 1 file changed, 39 insertions(+), 18 deletions(-) diff --git a/core/components/menu/menu.js b/core/components/menu/menu.js index 083a363b8c9..2639ec03f4c 100644 --- a/core/components/menu/menu.js +++ b/core/components/menu/menu.js @@ -339,6 +339,29 @@ Blockly.Menu.prototype.highlightPrevious = function() { }, highlightedIndex); }; +/** + * Highlights the first highlightable item. + * @package + */ +Blockly.Menu.prototype.highlightFirst = function() { + this.clearHighlighted(); + this.highlightHelper(function(index, max) { + return (index + 1) % max; + }, -1); +}; + +/** + * Highlights the ranh highlightable item. + * @package + */ +Blockly.Menu.prototype.highlightLast = function() { + this.clearHighlighted(); + this.highlightHelper(function(index, max) { + index--; + return index < 0 ? max - 1 : index; + }, -1); +}; + /** * Helper function that manages the details of moving the highlight among * child menuitems in response to keyboard events. @@ -449,46 +472,34 @@ Blockly.Menu.prototype.handleMouseLeave_ = function(_e) { * Attempts to handle a keyboard event, if the menuitem is enabled, by calling * {@link handleKeyEventInternal}. Considered protected; should only be used * within this package and by subclasses. - * @param {Event} e Key event to handle. - * @return {boolean} Whether the key event was handled. + * @param {!Event} e Key event to handle. * @protected */ Blockly.Menu.prototype.handleKeyEvent = function(e) { - if (this.getChildCount() != 0 && this.handleKeyEventInternal(e)) { + if (this.getChildCount() && this.handleKeyEventInternal(e)) { e.preventDefault(); e.stopPropagation(); - return true; } - return false; }; /** * Attempts to handle a keyboard event; returns true if the event was handled, - * false otherwise. If the container is enabled, and a child is highlighted, - * calls the child menuitem's `handleKeyEvent` method to give the menuitem - * a chance to handle the event first. - * @param {Event} e Key event to handle. + * false otherwise. + * @param {!Event} e Key event to handle. * @return {boolean} Whether the event was handled by the container (or one of * its children). * @protected */ Blockly.Menu.prototype.handleKeyEventInternal = function(e) { - // Give the highlighted menuitem the chance to handle the key event. - var highlighted = this.getHighlighted(); - if (highlighted && typeof highlighted.handleKeyEvent == 'function' && - highlighted.handleKeyEvent(e)) { - return true; - } - // Do not handle the key event if any modifier key is pressed. if (e.shiftKey || e.ctrlKey || e.metaKey || e.altKey) { return false; } - // Either nothing is highlighted, or the highlighted menuitem didn't handle - // the key event, so attempt to handle it here. + var highlighted = this.getHighlighted(); switch (e.keyCode) { case Blockly.utils.KeyCodes.ENTER: + case Blockly.utils.KeyCodes.SPACE: if (highlighted) { highlighted.performActionInternal(e); } @@ -502,6 +513,16 @@ Blockly.Menu.prototype.handleKeyEventInternal = function(e) { this.highlightNext(); break; + case Blockly.utils.KeyCodes.PAGE_UP: + case Blockly.utils.KeyCodes.HOME: + this.highlightFirst(); + break; + + case Blockly.utils.KeyCodes.PAGE_DOWN: + case Blockly.utils.KeyCodes.END: + this.highlightLast(); + break; + default: return false; } From 3100327c8260f66a8c6bf45ecd426e07089115f5 Mon Sep 17 00:00:00 2001 From: Neil Fraser Date: Sun, 3 May 2020 16:36:20 -0700 Subject: [PATCH 06/30] Eliminate calls to clearHighlighted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The JSDoc for `setHighlightedIndex` specifically states, “If another item was previously highlighted, it is un-highlighted.” This is not what was implemented, but it should be. This commit adds the un-highlighting, and removes all the calls previously required to correct this bug. --- core/components/menu/menu.js | 38 +++++++++++------------------------- 1 file changed, 11 insertions(+), 27 deletions(-) diff --git a/core/components/menu/menu.js b/core/components/menu/menu.js index 2639ec03f4c..9f69b189613 100644 --- a/core/components/menu/menu.js +++ b/core/components/menu/menu.js @@ -259,14 +259,6 @@ Blockly.Menu.prototype.getMenuItem = function(node) { // Highlight management. -/** - * Clears the currently highlighted item. - * @protected - */ -Blockly.Menu.prototype.clearHighlighted = function() { - this.setHighlightedIndex(-1); -}; - /** * Returns the currently highlighted item (if any). * @return {?Blockly.Component} Highlighted item (null if none). @@ -284,18 +276,17 @@ Blockly.Menu.prototype.getHighlighted = function() { * @protected */ Blockly.Menu.prototype.setHighlightedIndex = function(index) { + var currentHighlighted = this.getHighlighted(); + if (currentHighlighted) { + currentHighlighted.setHighlighted(false); + this.highlightedIndex_ = -1; + } var child = this.getChildAt(index); if (child) { child.setHighlighted(true); this.highlightedIndex_ = index; - } else if (this.highlightedIndex_ > -1) { - this.getHighlighted().setHighlighted(false); - this.highlightedIndex_ = -1; - } - - // Bring the highlighted item into view. This has no effect if the menu is not - // scrollable. - if (child) { + // Bring the highlighted item into view. This has no effect if the menu is + // not scrollable. Blockly.utils.style.scrollIntoContainerView( /** @type {!Element} */ (child.getElement()), /** @type {!Element} */ (this.getElement())); @@ -318,11 +309,9 @@ Blockly.Menu.prototype.setHighlighted = function(item) { * @package */ Blockly.Menu.prototype.highlightNext = function() { - var highlightedIndex = this.highlightedIndex_; - this.clearHighlighted(); this.highlightHelper(function(index, max) { return (index + 1) % max; - }, highlightedIndex); + }, this.highlightedIndex_); }; /** @@ -331,12 +320,10 @@ Blockly.Menu.prototype.highlightNext = function() { * @package */ Blockly.Menu.prototype.highlightPrevious = function() { - var highlightedIndex = this.highlightedIndex_; - this.clearHighlighted(); this.highlightHelper(function(index, max) { index--; return index < 0 ? max - 1 : index; - }, highlightedIndex); + }, this.highlightedIndex_); }; /** @@ -344,7 +331,6 @@ Blockly.Menu.prototype.highlightPrevious = function() { * @package */ Blockly.Menu.prototype.highlightFirst = function() { - this.clearHighlighted(); this.highlightHelper(function(index, max) { return (index + 1) % max; }, -1); @@ -355,7 +341,6 @@ Blockly.Menu.prototype.highlightFirst = function() { * @package */ Blockly.Menu.prototype.highlightLast = function() { - this.clearHighlighted(); this.highlightHelper(function(index, max) { index--; return index < 0 ? max - 1 : index; @@ -409,10 +394,9 @@ Blockly.Menu.prototype.handleMouseOver_ = function(e) { if (currentHighlighted === menuItem) { return; } - this.clearHighlighted(); this.setHighlighted(menuItem); } else { - this.clearHighlighted(); + this.setHighlightedIndex(-1); } } }; @@ -462,7 +446,7 @@ Blockly.Menu.prototype.handleMouseEnter_ = function(_e) { Blockly.Menu.prototype.handleMouseLeave_ = function(_e) { if (this.getElement()) { this.blur(); - this.clearHighlighted(); + this.setHighlightedIndex(-1); } }; From 13e7b3c8b045074cdb2daa8a49d0d416eeb561c9 Mon Sep 17 00:00:00 2001 From: Neil Fraser Date: Sun, 3 May 2020 17:37:12 -0700 Subject: [PATCH 07/30] Stop wrapping at top or bottom of menu. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Real OS menus don’t wrap when one cursors off the top or bottom. Also, replace the overly complicated helper function with a simple 1/-1 step value. --- core/components/menu/menu.js | 50 +++++++++++------------------------- 1 file changed, 15 insertions(+), 35 deletions(-) diff --git a/core/components/menu/menu.js b/core/components/menu/menu.js index 9f69b189613..617df98cdf6 100644 --- a/core/components/menu/menu.js +++ b/core/components/menu/menu.js @@ -309,9 +309,7 @@ Blockly.Menu.prototype.setHighlighted = function(item) { * @package */ Blockly.Menu.prototype.highlightNext = function() { - this.highlightHelper(function(index, max) { - return (index + 1) % max; - }, this.highlightedIndex_); + this.highlightHelper(this.highlightedIndex_, 1); }; /** @@ -320,10 +318,8 @@ Blockly.Menu.prototype.highlightNext = function() { * @package */ Blockly.Menu.prototype.highlightPrevious = function() { - this.highlightHelper(function(index, max) { - index--; - return index < 0 ? max - 1 : index; - }, this.highlightedIndex_); + this.highlightHelper(this.highlightedIndex_ < 0 ? + this.getChildCount() : this.highlightedIndex_, -1); }; /** @@ -331,50 +327,34 @@ Blockly.Menu.prototype.highlightPrevious = function() { * @package */ Blockly.Menu.prototype.highlightFirst = function() { - this.highlightHelper(function(index, max) { - return (index + 1) % max; - }, -1); + this.highlightHelper(-1, 1); }; /** - * Highlights the ranh highlightable item. + * Highlights the last highlightable item. * @package */ Blockly.Menu.prototype.highlightLast = function() { - this.highlightHelper(function(index, max) { - index--; - return index < 0 ? max - 1 : index; - }, -1); + this.highlightHelper(this.getChildCount(), -1); }; /** * Helper function that manages the details of moving the highlight among * child menuitems in response to keyboard events. - * @param {function(this: Blockly.Component, number, number) : number} fn - * Function that accepts the current and maximum indices, and returns the - * next index to check. * @param {number} startIndex Start index. - * @return {boolean} Whether the highlight has changed. + * @param {boolean} delta Step direction: 1 to go down, -1 to go up. * @protected */ -Blockly.Menu.prototype.highlightHelper = function(fn, startIndex) { - // If the start index is -1 (meaning there's nothing currently highlighted), - // try starting from the currently open item, if any. - var curIndex = startIndex < 0 ? -1 : startIndex; - var numItems = this.getChildCount(); - - curIndex = fn.call(this, curIndex, numItems); - var visited = 0; - while (visited <= numItems) { - var menuItem = /** @type {Blockly.MenuItem} */ (this.getChildAt(curIndex)); - if (menuItem && menuItem.isEnabled()) { - this.setHighlightedIndex(curIndex); - return true; +Blockly.Menu.prototype.highlightHelper = function(startIndex, delta) { + var index = startIndex + delta; + var menuItem; + while ((menuItem = this.getChildAt(index))) { + if (menuItem.isEnabled()) { + this.setHighlightedIndex(index); + break; } - visited++; - curIndex = fn.call(this, curIndex, numItems); + index += delta; } - return false; }; // Mouse events. From 47c21395777e4701923b78b91328280c8b056f88 Mon Sep 17 00:00:00 2001 From: Neil Fraser Date: Sun, 3 May 2020 19:20:33 -0700 Subject: [PATCH 08/30] Fix JS Doc type. --- core/components/menu/menu.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/components/menu/menu.js b/core/components/menu/menu.js index 617df98cdf6..15f3d73c890 100644 --- a/core/components/menu/menu.js +++ b/core/components/menu/menu.js @@ -342,7 +342,7 @@ Blockly.Menu.prototype.highlightLast = function() { * Helper function that manages the details of moving the highlight among * child menuitems in response to keyboard events. * @param {number} startIndex Start index. - * @param {boolean} delta Step direction: 1 to go down, -1 to go up. + * @param {number} delta Step direction: 1 to go down, -1 to go up. * @protected */ Blockly.Menu.prototype.highlightHelper = function(startIndex, delta) { From 56eaff6a4a5f94ca47a062890cc0fdc563ce0b58 Mon Sep 17 00:00:00 2001 From: Neil Fraser Date: Sun, 3 May 2020 20:52:09 -0700 Subject: [PATCH 09/30] Remove unused menu code --- core/components/menu/menu.js | 7 ++--- core/components/menu/menuitem.js | 48 ++++++-------------------------- 2 files changed, 12 insertions(+), 43 deletions(-) diff --git a/core/components/menu/menu.js b/core/components/menu/menu.js index 15f3d73c890..22684b02703 100644 --- a/core/components/menu/menu.js +++ b/core/components/menu/menu.js @@ -403,9 +403,8 @@ Blockly.Menu.prototype.handleClick_ = function(e) { } var menuItem = this.getMenuItem(/** @type {Node} */ (e.target)); - - if (menuItem && menuItem.handleClick(e)) { - e.preventDefault(); + if (menuItem) { + menuItem.performAction(); } }; @@ -465,7 +464,7 @@ Blockly.Menu.prototype.handleKeyEventInternal = function(e) { case Blockly.utils.KeyCodes.ENTER: case Blockly.utils.KeyCodes.SPACE: if (highlighted) { - highlighted.performActionInternal(e); + highlighted.performAction(); } break; diff --git a/core/components/menu/menuitem.js b/core/components/menu/menuitem.js index 2288c4029e7..2c9682ce3c5 100644 --- a/core/components/menu/menuitem.js +++ b/core/components/menu/menuitem.js @@ -38,18 +38,6 @@ Blockly.MenuItem = function(content, opt_value) { * @private */ this.enabled_ = true; - - /** - * @type {Blockly.MenuItem} - * @private - */ - this.previousSibling_; - - /** - * @type {Blockly.MenuItem} - * @private - */ - this.nextSibling_; }; Blockly.utils.object.inherits(Blockly.MenuItem, Blockly.Component); @@ -240,43 +228,25 @@ Blockly.MenuItem.prototype.setEnabled = function(enabled) { } }; -/** - * Handles click events. If the component is enabled, trigger - * the action associated with this menu item. - * @param {Event} _e Mouse event to handle. - * @package - */ -Blockly.MenuItem.prototype.handleClick = function(_e) { - if (this.isEnabled()) { - this.setHighlighted(true); - this.performActionInternal(); - } -}; - /** * Performs the appropriate action when the menu item is activated * by the user. - * @protected + * @package */ -Blockly.MenuItem.prototype.performActionInternal = function() { - if (this.checkable_) { - this.setChecked(!this.checked_); - } - if (this.actionHandler_) { - this.actionHandler_.call(/** @type {?} */ (this.actionHandlerObj_), this); +Blockly.MenuItem.prototype.performAction = function() { + if (this.isEnabled() && this.actionHandler_) { + this.actionHandler_.call(this.actionHandlerObj_, this); } }; /** - * Set the handler that's triggered when the menu item is activated - * by the user. If `opt_obj` is provided, it will be used as the - * 'this' object in the function when called. + * Set the handler that's called when the menu item is activated by the user. + * `obj` will be used as the 'this' object in the function when called. * @param {function(this:T,!Blockly.MenuItem):?} fn The handler. - * @param {T=} opt_obj Used as the 'this' object in f when called. - * @template T + * @param {!Object} obj Used as the 'this' object in fn when called. * @package */ -Blockly.MenuItem.prototype.onAction = function(fn, opt_obj) { +Blockly.MenuItem.prototype.onAction = function(fn, obj) { this.actionHandler_ = fn; - this.actionHandlerObj_ = opt_obj; + this.actionHandlerObj_ = obj; }; From e216076eb951688fad88de14b33c990a1f298211 Mon Sep 17 00:00:00 2001 From: Neil Fraser Date: Sun, 3 May 2020 23:56:17 -0700 Subject: [PATCH 10/30] Simplify menu roles --- core/components/menu/menuitem.js | 6 ++---- core/contextmenu.js | 1 + 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/core/components/menu/menuitem.js b/core/components/menu/menuitem.js index 2c9682ce3c5..3629a373dbe 100644 --- a/core/components/menu/menuitem.js +++ b/core/components/menu/menuitem.js @@ -69,9 +69,7 @@ Blockly.MenuItem.prototype.createDom = function() { content.appendChild(this.getContentDom()); // Initialize ARIA role and state. - Blockly.utils.aria.setRole(element, this.roleName_ || (this.checkable_ ? - Blockly.utils.aria.Role.MENUITEMCHECKBOX : - Blockly.utils.aria.Role.MENUITEM)); + Blockly.utils.aria.setRole(element, this.roleName_); Blockly.utils.aria.setState(element, Blockly.utils.aria.State.SELECTED, (this.checkable_ && this.checked_) || false); }; @@ -242,7 +240,7 @@ Blockly.MenuItem.prototype.performAction = function() { /** * Set the handler that's called when the menu item is activated by the user. * `obj` will be used as the 'this' object in the function when called. - * @param {function(this:T,!Blockly.MenuItem):?} fn The handler. + * @param {function(!Blockly.MenuItem)} fn The handler. * @param {!Object} obj Used as the 'this' object in fn when called. * @package */ diff --git a/core/contextmenu.js b/core/contextmenu.js index 1391967b28e..c9e0906e8cb 100644 --- a/core/contextmenu.js +++ b/core/contextmenu.js @@ -81,6 +81,7 @@ Blockly.ContextMenu.populate_ = function(options, rtl) { for (var i = 0, option; (option = options[i]); i++) { var menuItem = new Blockly.MenuItem(option.text); menuItem.setRightToLeft(rtl); + menuItem.setRole(Blockly.utils.aria.Role.MENUITEM); menu.addChild(menuItem, true); menuItem.setEnabled(option.enabled); if (option.enabled) { From 5df126a724f2311f2a3d77b5090dd01df8c4a409 Mon Sep 17 00:00:00 2001 From: Neil Fraser Date: Mon, 4 May 2020 08:05:16 -0700 Subject: [PATCH 11/30] Replace menu item element map with a property. Remove unneeded sets to RTL on Menu (only MenuItem cares). --- core/components/menu/menu.js | 73 ++++++++------------------------ core/components/menu/menuitem.js | 1 + core/contextmenu.js | 2 +- core/field_dropdown.js | 1 - 4 files changed, 20 insertions(+), 57 deletions(-) diff --git a/core/components/menu/menu.js b/core/components/menu/menu.js index 22684b02703..a3ca09d7300 100644 --- a/core/components/menu/menu.js +++ b/core/components/menu/menu.js @@ -78,15 +78,6 @@ Blockly.Menu = function() { * @private */ this.onKeyDownWrapper_ = null; - - /** - * Map of DOM IDs to child menuitems. Each key is the DOM ID of a child - * menuitems's root element; each value is a reference to the child menu - * item itself. - * @type {!Object} - * @private - */ - this.childElementIdMap_ = {}; }; Blockly.utils.object.inherits(Blockly.Menu, Blockly.Component); @@ -105,8 +96,7 @@ Blockly.Menu.prototype.createDom = function() { element.tabIndex = 0; // Initialize ARIA role. - Blockly.utils.aria.setRole(element, - this.roleName_ || Blockly.utils.aria.Role.MENU); + Blockly.utils.aria.setRole(element, this.roleName_); }; /** @@ -142,17 +132,25 @@ Blockly.Menu.prototype.setRole = function(roleName) { this.roleName_ = roleName; }; -/** @override */ +/** + * Adds the event listeners to the menu. + * @override + */ Blockly.Menu.prototype.enterDocument = function() { Blockly.Menu.superClass_.enterDocument.call(this); - this.forEachChild(function(child) { - if (child.isInDocument()) { - this.registerChildId_(child); - } - }, this); + var el = /** @type {!EventTarget} */ (this.getElement()); - this.attachEvents_(); + this.mouseOverHandler_ = Blockly.bindEventWithChecks_(el, + 'mouseover', this, this.handleMouseOver_, true); + this.clickHandler_ = Blockly.bindEventWithChecks_(el, + 'click', this, this.handleClick_, true); + this.mouseEnterHandler_ = Blockly.bindEventWithChecks_(el, + 'mouseenter', this, this.handleMouseEnter_, true); + this.mouseLeaveHandler_ = Blockly.bindEventWithChecks_(el, + 'mouseleave', this, this.handleMouseLeave_, true); + this.onKeyDownWrapper_ = Blockly.bindEventWithChecks_(el, + 'keydown', this, this.handleKeyEvent); }; /** @@ -175,25 +173,6 @@ Blockly.Menu.prototype.disposeInternal = function() { this.detachEvents_(); }; -/** - * Adds the event listeners to the menu. - * @private - */ -Blockly.Menu.prototype.attachEvents_ = function() { - var el = /** @type {!EventTarget} */ (this.getElement()); - - this.mouseOverHandler_ = Blockly.bindEventWithChecks_(el, - 'mouseover', this, this.handleMouseOver_, true); - this.clickHandler_ = Blockly.bindEventWithChecks_(el, - 'click', this, this.handleClick_, true); - this.mouseEnterHandler_ = Blockly.bindEventWithChecks_(el, - 'mouseenter', this, this.handleMouseEnter_, true); - this.mouseLeaveHandler_ = Blockly.bindEventWithChecks_(el, - 'mouseleave', this, this.handleMouseLeave_, true); - this.onKeyDownWrapper_ = Blockly.bindEventWithChecks_(el, - 'keydown', this, this.handleKeyEvent); -}; - /** * Removes the event listeners from the menu. * @private @@ -223,21 +202,6 @@ Blockly.Menu.prototype.detachEvents_ = function() { // Child component management. -/** - * Creates a DOM ID for the child menuitem and registers it to an internal - * hash table to be able to find it fast by ID. - * @param {Blockly.Component} child The child menuitem. Its root element has - * to be created yet. - * @private - */ -Blockly.Menu.prototype.registerChildId_ = function(child) { - // Map the DOM ID of the menuitem's root element to the menuitem itself. - var childElem = child.getElement(); - // If the menuitem's root element doesn't have a DOM ID assign one. - var id = childElem.id || (childElem.id = child.getId()); - this.childElementIdMap_[id] = child; -}; - /** * Returns the child menuitem that owns the given DOM node, or null if no such * menuitem is found. @@ -248,9 +212,8 @@ Blockly.Menu.prototype.registerChildId_ = function(child) { Blockly.Menu.prototype.getMenuItem = function(node) { var elem = this.getElement(); while (node && node !== elem) { - var id = node.id; - if (id in this.childElementIdMap_) { - return this.childElementIdMap_[id]; + if (node.blocklyMenuItem) { + return node.blocklyMenuItem; } node = node.parentNode; } diff --git a/core/components/menu/menuitem.js b/core/components/menu/menuitem.js index 3629a373dbe..db6042f9214 100644 --- a/core/components/menu/menuitem.js +++ b/core/components/menu/menuitem.js @@ -49,6 +49,7 @@ Blockly.utils.object.inherits(Blockly.MenuItem, Blockly.Component); Blockly.MenuItem.prototype.createDom = function() { var element = document.createElement('div'); element.id = this.getId(); + element.blocklyMenuItem = this; // Link DOM back to this data structure. this.setElementInternal(element); // Set class and style diff --git a/core/contextmenu.js b/core/contextmenu.js index c9e0906e8cb..2aefa476915 100644 --- a/core/contextmenu.js +++ b/core/contextmenu.js @@ -77,7 +77,7 @@ Blockly.ContextMenu.populate_ = function(options, rtl) { callback: Blockly.MakeItSo} */ var menu = new Blockly.Menu(); - menu.setRightToLeft(rtl); + menu.setRole(Blockly.utils.aria.Role.MENU); for (var i = 0, option; (option = options[i]); i++) { var menuItem = new Blockly.MenuItem(option.text); menuItem.setRightToLeft(rtl); diff --git a/core/field_dropdown.js b/core/field_dropdown.js index 87062c5a436..20ec2153c22 100644 --- a/core/field_dropdown.js +++ b/core/field_dropdown.js @@ -308,7 +308,6 @@ Blockly.FieldDropdown.prototype.showEditor_ = function(opt_e) { */ Blockly.FieldDropdown.prototype.dropdownCreate_ = function() { var menu = new Blockly.Menu(); - menu.setRightToLeft(this.sourceBlock_.RTL); menu.setRole(Blockly.utils.aria.Role.LISTBOX); var options = this.getOptions(false); From e1ee16155ac68fd50959e596c2916c9f10f4c4d5 Mon Sep 17 00:00:00 2001 From: Neil Fraser Date: Mon, 4 May 2020 19:41:08 -0700 Subject: [PATCH 12/30] Fix lack of disposal for context menus. Context menus only disposed properly when an option was clicked. If they were dismissed by clicking outside the menu there was no disposal. This might result in a memory leak. Also un-extract (inject?) several now trivial functions. --- core/components/menu/menu.js | 24 +------- core/components/menu/menuitem.js | 96 ++++++++++---------------------- core/contextmenu.js | 21 ++++--- core/field_dropdown.js | 3 +- 4 files changed, 48 insertions(+), 96 deletions(-) diff --git a/core/components/menu/menu.js b/core/components/menu/menu.js index a3ca09d7300..f75f8351445 100644 --- a/core/components/menu/menu.js +++ b/core/components/menu/menu.js @@ -154,30 +154,10 @@ Blockly.Menu.prototype.enterDocument = function() { }; /** - * Cleans up the container before its DOM is removed from the document, and - * removes event handlers. Overrides {@link Blockly.Component#exitDocument}. + * Removes event handlers. Overrides {@link Blockly.Component#exitDocument}. * @override */ Blockly.Menu.prototype.exitDocument = function() { - // {@link #setHighlightedIndex} has to be called before - // {@link Blockly.Component#exitDocument}, otherwise it has no effect. - this.setHighlightedIndex(-1); - - Blockly.Menu.superClass_.exitDocument.call(this); -}; - -/** @override */ -Blockly.Menu.prototype.disposeInternal = function() { - Blockly.Menu.superClass_.disposeInternal.call(this); - - this.detachEvents_(); -}; - -/** - * Removes the event listeners from the menu. - * @private - */ -Blockly.Menu.prototype.detachEvents_ = function() { if (this.mouseOverHandler_) { Blockly.unbindEvent_(this.mouseOverHandler_); this.mouseOverHandler_ = null; @@ -198,6 +178,8 @@ Blockly.Menu.prototype.detachEvents_ = function() { Blockly.unbindEvent_(this.onKeyDownWrapper_); this.onKeyDownWrapper_ = null; } + + Blockly.Menu.superClass_.exitDocument.call(this); }; // Child component management. diff --git a/core/components/menu/menuitem.js b/core/components/menu/menuitem.js index db6042f9214..1f002e015cb 100644 --- a/core/components/menu/menuitem.js +++ b/core/components/menu/menuitem.js @@ -30,10 +30,22 @@ goog.require('Blockly.utils.object'); Blockly.MenuItem = function(content, opt_value) { Blockly.Component.call(this); - this.setContentInternal(content); - this.setValue(opt_value); + /** + * Human-readable text of this menu item. + * @type {string} + * @private + */ + this.content_ = content; + + /** + * Machine-readable value of this menu item. + * @type {string|undefined} + * @private + */ + this.value_ = opt_value; /** + * Is the menu clickable, as opposed to greyed-out. * @type {boolean} * @private */ @@ -58,16 +70,17 @@ Blockly.MenuItem.prototype.createDom = function() { (this.checked_ ? 'goog-option-selected ' : '') + (this.rightToLeft_ ? 'goog-menuitem-rtl ' : ''); - var content = this.getContentWrapperDom(); - element.appendChild(content); - + var content = document.createElement('div'); + content.className = 'goog-menuitem-content'; // Add a checkbox for checkable menu items. - var checkboxDom = this.getCheckboxDom(); - if (checkboxDom) { - content.appendChild(checkboxDom); + if (!this.checkable_) { + var checkbox = document.createElement('div'); + checkbox.className = 'goog-menuitem-checkbox'; + content.appendChild(checkbox); } - content.appendChild(this.getContentDom()); + content.appendChild(document.createTextNode(this.content_)); + element.appendChild(content); // Initialize ARIA role and state. Blockly.utils.aria.setRole(element, this.roleName_); @@ -75,60 +88,11 @@ Blockly.MenuItem.prototype.createDom = function() { (this.checkable_ && this.checked_) || false); }; -/** - * @return {Element} The HTML element for the checkbox. - * @protected - */ -Blockly.MenuItem.prototype.getCheckboxDom = function() { - if (!this.checkable_) { - return null; - } - var menuItemCheckbox = document.createElement('div'); - menuItemCheckbox.className = 'goog-menuitem-checkbox'; - return menuItemCheckbox; -}; - -/** - * @return {!Element} The HTML for the content. - * @protected - */ -Blockly.MenuItem.prototype.getContentDom = function() { - var content = this.content_; - if (typeof content === 'string') { - content = document.createTextNode(content); - } - return content; -}; - -/** - * @return {!Element} The HTML for the content wrapper. - * @protected - */ -Blockly.MenuItem.prototype.getContentWrapperDom = function() { - var contentWrapper = document.createElement('div'); - contentWrapper.className = 'goog-menuitem-content'; - return contentWrapper; -}; - -/** - * Sets the content associated with the menu item. - * @param {string} content Text caption to set as the - * menuitem's contents. - * @protected - */ -Blockly.MenuItem.prototype.setContentInternal = function(content) { - this.content_ = content; -}; - -/** - * Sets the value associated with the menu item. - * @param {*} value Value to be associated with the menu item. - * @package - */ -Blockly.MenuItem.prototype.setValue = function(value) { - this.value_ = value; +/** @override */ +Blockly.MenuItem.prototype.disposeInternal = function() { + this.getElement().blocklyMenuItem = null; + Blockly.MenuItem.superClass_.disposeInternal.call(this); }; - /** * Gets the value associated with the menu item. * @return {*} value Value associated with the menu item. @@ -164,7 +128,7 @@ Blockly.MenuItem.prototype.setCheckable = function(checkable) { */ Blockly.MenuItem.prototype.setChecked = function(checked) { if (!this.checkable_) { - return; + throw Error('MenuItem not checkable'); } this.checked_ = checked; @@ -219,10 +183,10 @@ Blockly.MenuItem.prototype.setEnabled = function(enabled) { var el = this.getElement(); if (el) { - if (!this.enabled_) { - Blockly.utils.dom.addClass(el, 'goog-menuitem-disabled'); - } else { + if (enabled) { Blockly.utils.dom.removeClass(el, 'goog-menuitem-disabled'); + } else { + Blockly.utils.dom.addClass(el, 'goog-menuitem-disabled'); } } }; diff --git a/core/contextmenu.js b/core/contextmenu.js index 2aefa476915..5d5f8c6f6df 100644 --- a/core/contextmenu.js +++ b/core/contextmenu.js @@ -36,11 +36,11 @@ goog.require('Blockly.Xml'); Blockly.ContextMenu.currentBlock = null; /** - * Opaque data that can be passed to unbindEvent_. - * @type {Array.} + * Menu object. + * @type {Blockly.Menu} * @private */ -Blockly.ContextMenu.eventWrapper_ = null; +Blockly.ContextMenu.menu_ = null; /** * Construct the menu based on the list of options and show the menu. @@ -49,12 +49,13 @@ Blockly.ContextMenu.eventWrapper_ = null; * @param {boolean} rtl True if RTL, false if LTR. */ Blockly.ContextMenu.show = function(e, options, rtl) { - Blockly.WidgetDiv.show(Blockly.ContextMenu, rtl, null); + Blockly.WidgetDiv.show(Blockly.ContextMenu, rtl, Blockly.ContextMenu.dispose); if (!options.length) { Blockly.ContextMenu.hide(); return; } var menu = Blockly.ContextMenu.populate_(options, rtl); + Blockly.ContextMenu.menu_ = menu; Blockly.ContextMenu.position_(menu, e, rtl); // 1ms delay is required for focusing on context menus because some other @@ -154,9 +155,15 @@ Blockly.ContextMenu.createWidget_ = function(menu) { Blockly.ContextMenu.hide = function() { Blockly.WidgetDiv.hideIfOwner(Blockly.ContextMenu); Blockly.ContextMenu.currentBlock = null; - if (Blockly.ContextMenu.eventWrapper_) { - Blockly.unbindEvent_(Blockly.ContextMenu.eventWrapper_); - Blockly.ContextMenu.eventWrapper_ = null; +}; + +/** + * Dispose of the menu. + */ +Blockly.ContextMenu.dispose = function() { + if (Blockly.ContextMenu.menu_) { + Blockly.ContextMenu.menu_.dispose(); + Blockly.ContextMenu.menu_ = null; } }; diff --git a/core/field_dropdown.js b/core/field_dropdown.js index 20ec2153c22..062cec71082 100644 --- a/core/field_dropdown.js +++ b/core/field_dropdown.js @@ -322,10 +322,9 @@ Blockly.FieldDropdown.prototype.dropdownCreate_ = function() { image.alt = content['alt'] || ''; content = image; } - var menuItem = new Blockly.MenuItem(content); + var menuItem = new Blockly.MenuItem(content, value); menuItem.setRole(Blockly.utils.aria.Role.OPTION); menuItem.setRightToLeft(this.sourceBlock_.RTL); - menuItem.setValue(value); menuItem.setCheckable(true); menu.addChild(menuItem, true); menuItem.setChecked(value == this.value_); From 9b4529aaa496f263b3d184b6fcdb979bf2a6e5e2 Mon Sep 17 00:00:00 2001 From: Neil Fraser Date: Mon, 4 May 2020 21:09:18 -0700 Subject: [PATCH 13/30] Fix lint error. --- core/components/menu/menuitem.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core/components/menu/menuitem.js b/core/components/menu/menuitem.js index 1f002e015cb..6b2fa5b5c1f 100644 --- a/core/components/menu/menuitem.js +++ b/core/components/menu/menuitem.js @@ -70,7 +70,7 @@ Blockly.MenuItem.prototype.createDom = function() { (this.checked_ ? 'goog-option-selected ' : '') + (this.rightToLeft_ ? 'goog-menuitem-rtl ' : ''); - var content = document.createElement('div'); + var content = document.createElement('div'); content.className = 'goog-menuitem-content'; // Add a checkbox for checkable menu items. if (!this.checkable_) { @@ -93,6 +93,7 @@ Blockly.MenuItem.prototype.disposeInternal = function() { this.getElement().blocklyMenuItem = null; Blockly.MenuItem.superClass_.disposeInternal.call(this); }; + /** * Gets the value associated with the menu item. * @return {*} value Value associated with the menu item. From a41c52dc4319e5b75cefa1f2fe455e5836d79803 Mon Sep 17 00:00:00 2001 From: Neil Fraser Date: Tue, 5 May 2020 01:18:24 -0700 Subject: [PATCH 14/30] Remove Component dependency from Menu & MenuItem Component is now only used by the category tree. --- core/components/menu/menu.js | 116 ++++++++++++++++++------------- core/components/menu/menuitem.js | 89 +++++++++++++++--------- core/field_dropdown.js | 14 ++-- 3 files changed, 131 insertions(+), 88 deletions(-) diff --git a/core/components/menu/menu.js b/core/components/menu/menu.js index f75f8351445..d78cb17586b 100644 --- a/core/components/menu/menu.js +++ b/core/components/menu/menu.js @@ -12,7 +12,6 @@ goog.provide('Blockly.Menu'); -goog.require('Blockly.Component'); goog.require('Blockly.utils.aria'); goog.require('Blockly.utils.Coordinate'); goog.require('Blockly.utils.dom'); @@ -22,10 +21,14 @@ goog.require('Blockly.utils.object'); /** * A basic menu class. * @constructor - * @extends {Blockly.Component} */ Blockly.Menu = function() { - Blockly.Component.call(this); + /** + * Array of menu items. + * @type {!Array.} + * @private + */ + this.menuItems_ = []; /** * Coordinates of the mousedown event that caused this menu to open. Used to @@ -38,7 +41,7 @@ Blockly.Menu = function() { /** * This is the element that we will listen to the real focus events on. - * A value of -1 means no menuitem is highlighted. + * A value of -1 means no menu item is highlighted. * @type {number} * @private */ @@ -79,24 +82,54 @@ Blockly.Menu = function() { */ this.onKeyDownWrapper_ = null; }; -Blockly.utils.object.inherits(Blockly.Menu, Blockly.Component); +/** + * Add a new menu item to the bottom of this menu. + * @param {!Blockly.MenuItem} menuItem Menu item to append. + */ +Blockly.Menu.prototype.addChild = function(menuItem) { + this.menuItems_.push(menuItem); +}; + /** * Creates the menu DOM. - * @override + * @param {!Element} container Element upon which to append this menu. */ -Blockly.Menu.prototype.createDom = function() { +Blockly.Menu.prototype.render = function(container) { var element = document.createElement('div'); - element.id = this.getId(); - this.setElementInternal(element); - - // Set class element.className = 'goog-menu blocklyNonSelectable'; element.tabIndex = 0; - - // Initialize ARIA role. Blockly.utils.aria.setRole(element, this.roleName_); + this.element_ = element; + + // Add menu items. + for (var i = 0, menuItem; (menuItem = this.menuItems_[i]); i++) { + element.appendChild(menuItem.createDom()); + } + + // Add event handlers. + this.mouseOverHandler_ = Blockly.bindEventWithChecks_(element, + 'mouseover', this, this.handleMouseOver_, true); + this.clickHandler_ = Blockly.bindEventWithChecks_(element, + 'click', this, this.handleClick_, true); + this.mouseEnterHandler_ = Blockly.bindEventWithChecks_(element, + 'mouseenter', this, this.handleMouseEnter_, true); + this.mouseLeaveHandler_ = Blockly.bindEventWithChecks_(element, + 'mouseleave', this, this.handleMouseLeave_, true); + this.onKeyDownWrapper_ = Blockly.bindEventWithChecks_(element, + 'keydown', this, this.handleKeyEvent); + + container.appendChild(element); +}; + +/** + * Gets the menu's element. + * @return {Element} The DOM element. + * @package + */ +Blockly.Menu.prototype.getElement = function() { + return this.element_; }; /** @@ -133,31 +166,10 @@ Blockly.Menu.prototype.setRole = function(roleName) { }; /** - * Adds the event listeners to the menu. - * @override + * Dispose of this menu. */ -Blockly.Menu.prototype.enterDocument = function() { - Blockly.Menu.superClass_.enterDocument.call(this); - - var el = /** @type {!EventTarget} */ (this.getElement()); - - this.mouseOverHandler_ = Blockly.bindEventWithChecks_(el, - 'mouseover', this, this.handleMouseOver_, true); - this.clickHandler_ = Blockly.bindEventWithChecks_(el, - 'click', this, this.handleClick_, true); - this.mouseEnterHandler_ = Blockly.bindEventWithChecks_(el, - 'mouseenter', this, this.handleMouseEnter_, true); - this.mouseLeaveHandler_ = Blockly.bindEventWithChecks_(el, - 'mouseleave', this, this.handleMouseLeave_, true); - this.onKeyDownWrapper_ = Blockly.bindEventWithChecks_(el, - 'keydown', this, this.handleKeyEvent); -}; - -/** - * Removes event handlers. Overrides {@link Blockly.Component#exitDocument}. - * @override - */ -Blockly.Menu.prototype.exitDocument = function() { +Blockly.Menu.prototype.dispose = function() { + // Remove event handlers. if (this.mouseOverHandler_) { Blockly.unbindEvent_(this.mouseOverHandler_); this.mouseOverHandler_ = null; @@ -179,16 +191,20 @@ Blockly.Menu.prototype.exitDocument = function() { this.onKeyDownWrapper_ = null; } - Blockly.Menu.superClass_.exitDocument.call(this); + // Remove menu items. + for (var i = 0, menuItem; (menuItem = this.menuItems_[i]); i++) { + menuItem.dispose(); + } + this.element_ = null; }; // Child component management. /** - * Returns the child menuitem that owns the given DOM node, or null if no such - * menuitem is found. + * Returns the child menu item that owns the given DOM node, or null if no such + * menui tem is found. * @param {Node} node DOM node whose owner is to be returned. - * @return {?Blockly.MenuItem} menuitem for which the DOM node belongs to. + * @return {?Blockly.MenuItem} Menu item for which the DOM node belongs to. * @protected */ Blockly.Menu.prototype.getMenuItem = function(node) { @@ -206,11 +222,11 @@ Blockly.Menu.prototype.getMenuItem = function(node) { /** * Returns the currently highlighted item (if any). - * @return {?Blockly.Component} Highlighted item (null if none). + * @return {?Blockly.MenuItem} Highlighted menu item (null if none). * @protected */ Blockly.Menu.prototype.getHighlighted = function() { - return this.getChildAt(this.highlightedIndex_); + return this.menuItems_[this.highlightedIndex_]; }; /** @@ -226,7 +242,7 @@ Blockly.Menu.prototype.setHighlightedIndex = function(index) { currentHighlighted.setHighlighted(false); this.highlightedIndex_ = -1; } - var child = this.getChildAt(index); + var child = this.menuItems_[index]; if (child) { child.setHighlighted(true); this.highlightedIndex_ = index; @@ -245,7 +261,7 @@ Blockly.Menu.prototype.setHighlightedIndex = function(index) { * @protected */ Blockly.Menu.prototype.setHighlighted = function(item) { - this.setHighlightedIndex(this.indexOfChild(item)); + this.setHighlightedIndex(this.menuItems_.indexOf(item)); }; /** @@ -264,7 +280,7 @@ Blockly.Menu.prototype.highlightNext = function() { */ Blockly.Menu.prototype.highlightPrevious = function() { this.highlightHelper(this.highlightedIndex_ < 0 ? - this.getChildCount() : this.highlightedIndex_, -1); + this.menuItems_.length : this.highlightedIndex_, -1); }; /** @@ -280,7 +296,7 @@ Blockly.Menu.prototype.highlightFirst = function() { * @package */ Blockly.Menu.prototype.highlightLast = function() { - this.highlightHelper(this.getChildCount(), -1); + this.highlightHelper(this.menuItems_.length, -1); }; /** @@ -293,7 +309,7 @@ Blockly.Menu.prototype.highlightLast = function() { Blockly.Menu.prototype.highlightHelper = function(startIndex, delta) { var index = startIndex + delta; var menuItem; - while ((menuItem = this.getChildAt(index))) { + while ((menuItem = this.menuItems_[index])) { if (menuItem.isEnabled()) { this.setHighlightedIndex(index); break; @@ -377,14 +393,14 @@ Blockly.Menu.prototype.handleMouseLeave_ = function(_e) { // Keyboard events. /** - * Attempts to handle a keyboard event, if the menuitem is enabled, by calling + * Attempts to handle a keyboard event, if the menu item is enabled, by calling * {@link handleKeyEventInternal}. Considered protected; should only be used * within this package and by subclasses. * @param {!Event} e Key event to handle. * @protected */ Blockly.Menu.prototype.handleKeyEvent = function(e) { - if (this.getChildCount() && this.handleKeyEventInternal(e)) { + if (this.menuItems_.length && this.handleKeyEventInternal(e)) { e.preventDefault(); e.stopPropagation(); } diff --git a/core/components/menu/menuitem.js b/core/components/menu/menuitem.js index 6b2fa5b5c1f..63a1ff831b1 100644 --- a/core/components/menu/menuitem.js +++ b/core/components/menu/menuitem.js @@ -12,9 +12,9 @@ goog.provide('Blockly.MenuItem'); -goog.require('Blockly.Component'); goog.require('Blockly.utils.aria'); goog.require('Blockly.utils.dom'); +goog.require('Blockly.utils.IdGenerator'); goog.require('Blockly.utils.object'); @@ -25,11 +25,8 @@ goog.require('Blockly.utils.object'); * the item. * @param {string=} opt_value Data/model associated with the menu item. * @constructor - * @extends {Blockly.Component} */ Blockly.MenuItem = function(content, opt_value) { - Blockly.Component.call(this); - /** * Human-readable text of this menu item. * @type {string} @@ -45,24 +42,37 @@ Blockly.MenuItem = function(content, opt_value) { this.value_ = opt_value; /** - * Is the menu clickable, as opposed to greyed-out. + * Is the menu item clickable, as opposed to greyed-out. * @type {boolean} * @private */ this.enabled_ = true; + + /** + * The DOM element for the menu item. + * @type {?Element} + * @private + */ + this.element_ = null; + + /** + * Whether the menu item is rendered right-to-left. + * @type {boolean} + * @protected + */ + this.rightToLeft_ = false; }; -Blockly.utils.object.inherits(Blockly.MenuItem, Blockly.Component); /** * Creates the menuitem's DOM. - * @override + * @return {!Element} Completed DOM. */ Blockly.MenuItem.prototype.createDom = function() { var element = document.createElement('div'); - element.id = this.getId(); + element.id = Blockly.utils.IdGenerator.getNextUniqueId(); element.blocklyMenuItem = this; // Link DOM back to this data structure. - this.setElementInternal(element); + this.element_ = element; // Set class and style element.className = 'goog-menuitem ' + @@ -73,7 +83,7 @@ Blockly.MenuItem.prototype.createDom = function() { var content = document.createElement('div'); content.className = 'goog-menuitem-content'; // Add a checkbox for checkable menu items. - if (!this.checkable_) { + if (this.checkable_) { var checkbox = document.createElement('div'); checkbox.className = 'goog-menuitem-checkbox'; content.appendChild(checkbox); @@ -86,12 +96,36 @@ Blockly.MenuItem.prototype.createDom = function() { Blockly.utils.aria.setRole(element, this.roleName_); Blockly.utils.aria.setState(element, Blockly.utils.aria.State.SELECTED, (this.checkable_ && this.checked_) || false); + + return element; }; -/** @override */ -Blockly.MenuItem.prototype.disposeInternal = function() { - this.getElement().blocklyMenuItem = null; - Blockly.MenuItem.superClass_.disposeInternal.call(this); +/** + * Dispose of this menu item. + */ +Blockly.MenuItem.prototype.dispose = function() { + if (this.element_) { + this.element_.blocklyMenuItem = null; + this.element_ = null; + } +}; + +/** + * Gets the menu item's element. + * @return {Element} The DOM element. + * @package + */ +Blockly.MenuItem.prototype.getElement = function() { + return this.element_; +}; + +/** + * Gets the unique ID for this menu item. + * @return {string} Unique component ID. + * @package + */ +Blockly.MenuItem.prototype.getId = function() { + return this.element_.id_; }; /** @@ -104,7 +138,16 @@ Blockly.MenuItem.prototype.getValue = function() { }; /** - * Set the menu accessibility role. + * Set menu item's rendering direction. + * @param {boolean} rightToLeft True if RTL, false if LTR. + * @package + */ +Blockly.MenuItem.prototype.setRightToLeft = function(rtl) { + this.rightToLeft_ = rtl; +}; + +/** + * Set the menu item's accessibility role. * @param {!Blockly.utils.aria.Role} roleName Role name. * @package */ @@ -128,23 +171,7 @@ Blockly.MenuItem.prototype.setCheckable = function(checkable) { * @package */ Blockly.MenuItem.prototype.setChecked = function(checked) { - if (!this.checkable_) { - throw Error('MenuItem not checkable'); - } this.checked_ = checked; - - var el = this.getElement(); - if (el && this.isEnabled()) { - if (checked) { - Blockly.utils.dom.addClass(el, 'goog-option-selected'); - Blockly.utils.aria.setState(el, - Blockly.utils.aria.State.SELECTED, true); - } else { - Blockly.utils.dom.removeClass(el, 'goog-option-selected'); - Blockly.utils.aria.setState(el, - Blockly.utils.aria.State.SELECTED, false); - } - } }; /** diff --git a/core/field_dropdown.js b/core/field_dropdown.js index 062cec71082..548b0c882dd 100644 --- a/core/field_dropdown.js +++ b/core/field_dropdown.js @@ -270,8 +270,12 @@ Blockly.FieldDropdown.prototype.showEditor_ = function(opt_e) { } // Element gets created in render. this.menu_.render(Blockly.DropDownDiv.getContentDiv()); - Blockly.utils.dom.addClass( - /** @type {!Element} */ (this.menu_.getElement()), 'blocklyDropdownMenu'); + var menuElement = /** @type {!Element} */ (this.menu_.getElement()); + Blockly.utils.dom.addClass(menuElement, 'blocklyDropdownMenu'); + + Blockly.utils.aria.setState(menuElement, + Blockly.utils.aria.State.ACTIVEDESCENDANT, + this.selectedMenuItem_ ? this.selectedMenuItem_.getId() : ''); if (this.getConstants().FIELD_DROPDOWN_COLOURED_DIV) { var primaryColour = (this.sourceBlock_.isShadow()) ? @@ -295,7 +299,7 @@ Blockly.FieldDropdown.prototype.showEditor_ = function(opt_e) { if (this.selectedMenuItem_) { Blockly.utils.style.scrollIntoContainerView( /** @type {!Element} */ (this.selectedMenuItem_.getElement()), - /** @type {!Element} */ (this.menu_.getElement())); + menuElement); } this.applyColour(); @@ -334,10 +338,6 @@ Blockly.FieldDropdown.prototype.dropdownCreate_ = function() { menuItem.onAction(this.handleMenuActionEvent_, this); } - Blockly.utils.aria.setState(/** @type {!Element} */ (menu.getElement()), - Blockly.utils.aria.State.ACTIVEDESCENDANT, - this.selectedMenuItem_ ? this.selectedMenuItem_.getId() : ''); - return menu; }; From 6215dc4e69164f7920b41238271a942b5bc3eca2 Mon Sep 17 00:00:00 2001 From: Neil Fraser Date: Tue, 5 May 2020 01:33:48 -0700 Subject: [PATCH 15/30] Remove unused functions in Component These were used by Menu/MenuItem. --- core/components/component.js | 39 ------------------------------------ 1 file changed, 39 deletions(-) diff --git a/core/components/component.js b/core/components/component.js index 24bc7940341..6d7c13656e7 100644 --- a/core/components/component.js +++ b/core/components/component.js @@ -223,19 +223,6 @@ Blockly.Component.prototype.render = function(opt_parentElement) { this.render_(opt_parentElement); }; -/** - * Renders the component before another element. The other element should be in - * the document already. - * - * Throws an Error if the component is already rendered. - * - * @param {Node} sibling Node to render the component before. - * @protected - */ -Blockly.Component.prototype.renderBefore = function(sibling) { - this.render_(/** @type {Element} */ (sibling.parentNode), sibling); -}; - /** * Renders the component. If a parent element is supplied, the component's * element will be appended to it. If there is no optional parent element and @@ -497,21 +484,6 @@ Blockly.Component.prototype.getContentElement = function() { return this.element_; }; -/** - * Set is right-to-left. This function should be used if the component needs - * to know the rendering direction during DOM creation (i.e. before - * {@link #enterDocument} is called and is right-to-left is set). - * @param {boolean} rightToLeft Whether the component is rendered - * right-to-left. - * @package - */ -Blockly.Component.prototype.setRightToLeft = function(rightToLeft) { - if (this.inDocument_) { - throw Error(Blockly.Component.Error.ALREADY_RENDERED); - } - this.rightToLeft_ = rightToLeft; -}; - /** * Returns true if the component has children. * @return {boolean} True if the component has children. @@ -569,14 +541,3 @@ Blockly.Component.prototype.forEachChild = function(f, opt_obj) { f.call(/** @type {?} */ (opt_obj), this.children_[i], i); } }; - -/** - * Returns the 0-based index of the given child component, or -1 if no such - * child is found. - * @param {?Blockly.Component} child The child component. - * @return {number} 0-based index of the child component; -1 if not found. - * @protected - */ -Blockly.Component.prototype.indexOfChild = function(child) { - return this.children_.indexOf(child); -}; From 6c67a38564ce2828186bceb9e51e88a25d973352 Mon Sep 17 00:00:00 2001 From: Neil Fraser Date: Tue, 5 May 2020 01:39:10 -0700 Subject: [PATCH 16/30] Fix dependencies. --- core/components/menu/menu.js | 3 ++- core/components/menu/menuitem.js | 3 +-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/core/components/menu/menu.js b/core/components/menu/menu.js index d78cb17586b..a736f35a374 100644 --- a/core/components/menu/menu.js +++ b/core/components/menu/menu.js @@ -15,7 +15,8 @@ goog.provide('Blockly.Menu'); goog.require('Blockly.utils.aria'); goog.require('Blockly.utils.Coordinate'); goog.require('Blockly.utils.dom'); -goog.require('Blockly.utils.object'); +goog.require('Blockly.utils.KeyCodes'); +goog.require('Blockly.utils.style'); /** diff --git a/core/components/menu/menuitem.js b/core/components/menu/menuitem.js index 63a1ff831b1..a768cfad778 100644 --- a/core/components/menu/menuitem.js +++ b/core/components/menu/menuitem.js @@ -15,7 +15,6 @@ goog.provide('Blockly.MenuItem'); goog.require('Blockly.utils.aria'); goog.require('Blockly.utils.dom'); goog.require('Blockly.utils.IdGenerator'); -goog.require('Blockly.utils.object'); /** @@ -139,7 +138,7 @@ Blockly.MenuItem.prototype.getValue = function() { /** * Set menu item's rendering direction. - * @param {boolean} rightToLeft True if RTL, false if LTR. + * @param {boolean} rtl True if RTL, false if LTR. * @package */ Blockly.MenuItem.prototype.setRightToLeft = function(rtl) { From 10647ece0f8e57780c22c1c9650ea17d2283d9f5 Mon Sep 17 00:00:00 2001 From: Neil Fraser Date: Tue, 5 May 2020 01:53:03 -0700 Subject: [PATCH 17/30] Fix compile errors. --- core/components/menu/menu.js | 2 +- core/components/menu/menuitem.js | 2 +- core/contextmenu.js | 2 +- core/dropdowndiv.js | 2 +- core/field_dropdown.js | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/core/components/menu/menu.js b/core/components/menu/menu.js index a736f35a374..1f51d613a9e 100644 --- a/core/components/menu/menu.js +++ b/core/components/menu/menu.js @@ -258,7 +258,7 @@ Blockly.Menu.prototype.setHighlightedIndex = function(index) { /** * Highlights the given item if it exists and is a child of the container; * otherwise un-highlights the currently highlighted item. - * @param {Blockly.MenuItem} item Item to highlight. + * @param {!Blockly.MenuItem} item Item to highlight. * @protected */ Blockly.Menu.prototype.setHighlighted = function(item) { diff --git a/core/components/menu/menuitem.js b/core/components/menu/menuitem.js index a768cfad778..9ecd6d47417 100644 --- a/core/components/menu/menuitem.js +++ b/core/components/menu/menuitem.js @@ -124,7 +124,7 @@ Blockly.MenuItem.prototype.getElement = function() { * @package */ Blockly.MenuItem.prototype.getId = function() { - return this.element_.id_; + return this.element_.id; }; /** diff --git a/core/contextmenu.js b/core/contextmenu.js index 5d5f8c6f6df..034a7f728f4 100644 --- a/core/contextmenu.js +++ b/core/contextmenu.js @@ -83,7 +83,7 @@ Blockly.ContextMenu.populate_ = function(options, rtl) { var menuItem = new Blockly.MenuItem(option.text); menuItem.setRightToLeft(rtl); menuItem.setRole(Blockly.utils.aria.Role.MENUITEM); - menu.addChild(menuItem, true); + menu.addChild(menuItem); menuItem.setEnabled(option.enabled); if (option.enabled) { var actionHandler = function() { diff --git a/core/dropdowndiv.js b/core/dropdowndiv.js index 7add63838b2..47af904d515 100644 --- a/core/dropdowndiv.js +++ b/core/dropdowndiv.js @@ -184,7 +184,7 @@ Blockly.DropDownDiv.setBoundsElement = function(boundsElement) { /** * Provide the div for inserting content into the drop-down. - * @return {Element} Div to populate with content + * @return {!Element} Div to populate with content. */ Blockly.DropDownDiv.getContentDiv = function() { return Blockly.DropDownDiv.content_; diff --git a/core/field_dropdown.js b/core/field_dropdown.js index 548b0c882dd..5f2abdb9449 100644 --- a/core/field_dropdown.js +++ b/core/field_dropdown.js @@ -330,7 +330,7 @@ Blockly.FieldDropdown.prototype.dropdownCreate_ = function() { menuItem.setRole(Blockly.utils.aria.Role.OPTION); menuItem.setRightToLeft(this.sourceBlock_.RTL); menuItem.setCheckable(true); - menu.addChild(menuItem, true); + menu.addChild(menuItem); menuItem.setChecked(value == this.value_); if (value == this.value_) { this.selectedMenuItem_ = menuItem; From 518ab35718eae082b15480c9e82da7bf7047501b Mon Sep 17 00:00:00 2001 From: Neil Fraser Date: Tue, 5 May 2020 18:58:33 -0700 Subject: [PATCH 18/30] Record highlighted menu item by object, not index Less code, simpler. --- core/components/menu/menu.js | 81 +++++++++++++----------------------- 1 file changed, 29 insertions(+), 52 deletions(-) diff --git a/core/components/menu/menu.js b/core/components/menu/menu.js index 1f51d613a9e..04b8f7c248e 100644 --- a/core/components/menu/menu.js +++ b/core/components/menu/menu.js @@ -42,11 +42,11 @@ Blockly.Menu = function() { /** * This is the element that we will listen to the real focus events on. - * A value of -1 means no menu item is highlighted. - * @type {number} + * A value of null means no menu item is highlighted. + * @type {Blockly.MenuItem} * @private */ - this.highlightedIndex_ = -1; + this.highlightedItem_ = null; /** * Mouse over event data. @@ -81,7 +81,7 @@ Blockly.Menu = function() { * @type {?Blockly.EventData} * @private */ - this.onKeyDownWrapper_ = null; + this.onKeyDownHandler_ = null; }; @@ -118,7 +118,7 @@ Blockly.Menu.prototype.render = function(container) { 'mouseenter', this, this.handleMouseEnter_, true); this.mouseLeaveHandler_ = Blockly.bindEventWithChecks_(element, 'mouseleave', this, this.handleMouseLeave_, true); - this.onKeyDownWrapper_ = Blockly.bindEventWithChecks_(element, + this.onKeyDownHandler_ = Blockly.bindEventWithChecks_(element, 'keydown', this, this.handleKeyEvent); container.appendChild(element); @@ -187,9 +187,9 @@ Blockly.Menu.prototype.dispose = function() { Blockly.unbindEvent_(this.mouseLeaveHandler_); this.mouseLeaveHandler_ = null; } - if (this.onKeyDownWrapper_) { - Blockly.unbindEvent_(this.onKeyDownWrapper_); - this.onKeyDownWrapper_ = null; + if (this.onKeyDownHandler_) { + Blockly.unbindEvent_(this.onKeyDownHandler_); + this.onKeyDownHandler_ = null; } // Remove menu items. @@ -203,7 +203,7 @@ Blockly.Menu.prototype.dispose = function() { /** * Returns the child menu item that owns the given DOM node, or null if no such - * menui tem is found. + * menu item is found. * @param {Node} node DOM node whose owner is to be returned. * @return {?Blockly.MenuItem} Menu item for which the DOM node belongs to. * @protected @@ -222,56 +222,35 @@ Blockly.Menu.prototype.getMenuItem = function(node) { // Highlight management. /** - * Returns the currently highlighted item (if any). - * @return {?Blockly.MenuItem} Highlighted menu item (null if none). - * @protected - */ -Blockly.Menu.prototype.getHighlighted = function() { - return this.menuItems_[this.highlightedIndex_]; -}; - -/** - * Highlights the item at the given 0-based index (if any). If another item - * was previously highlighted, it is un-highlighted. - * @param {number} index Index of item to highlight (-1 removes the current - * highlight). + * Highlights the given menu item, or clears highlighting if null. + * @param {!Blockly.MenuItem} item Item to highlight. * @protected */ -Blockly.Menu.prototype.setHighlightedIndex = function(index) { - var currentHighlighted = this.getHighlighted(); +Blockly.Menu.prototype.setHighlighted = function(item) { + var currentHighlighted = this.highlightedItem_; if (currentHighlighted) { currentHighlighted.setHighlighted(false); - this.highlightedIndex_ = -1; + this.highlightedItem_ = null; } - var child = this.menuItems_[index]; - if (child) { - child.setHighlighted(true); - this.highlightedIndex_ = index; + if (item) { + item.setHighlighted(true); + this.highlightedItem_ = item; // Bring the highlighted item into view. This has no effect if the menu is // not scrollable. Blockly.utils.style.scrollIntoContainerView( - /** @type {!Element} */ (child.getElement()), + /** @type {!Element} */ (item.getElement()), /** @type {!Element} */ (this.getElement())); } }; -/** - * Highlights the given item if it exists and is a child of the container; - * otherwise un-highlights the currently highlighted item. - * @param {!Blockly.MenuItem} item Item to highlight. - * @protected - */ -Blockly.Menu.prototype.setHighlighted = function(item) { - this.setHighlightedIndex(this.menuItems_.indexOf(item)); -}; - /** * Highlights the next highlightable item (or the first if nothing is currently * highlighted). * @package */ Blockly.Menu.prototype.highlightNext = function() { - this.highlightHelper(this.highlightedIndex_, 1); + var index = this.menuItems_.indexOf(this.highlightedItem_); + this.highlightHelper(index, 1); }; /** @@ -280,8 +259,8 @@ Blockly.Menu.prototype.highlightNext = function() { * @package */ Blockly.Menu.prototype.highlightPrevious = function() { - this.highlightHelper(this.highlightedIndex_ < 0 ? - this.menuItems_.length : this.highlightedIndex_, -1); + var index = this.menuItems_.indexOf(this.highlightedItem_); + this.highlightHelper(index < 0 ? this.menuItems_.length : index, -1); }; /** @@ -312,7 +291,7 @@ Blockly.Menu.prototype.highlightHelper = function(startIndex, delta) { var menuItem; while ((menuItem = this.menuItems_[index])) { if (menuItem.isEnabled()) { - this.setHighlightedIndex(index); + this.setHighlighted(menuItem); break; } index += delta; @@ -332,13 +311,11 @@ Blockly.Menu.prototype.handleMouseOver_ = function(e) { if (menuItem) { if (menuItem.isEnabled()) { - var currentHighlighted = this.getHighlighted(); - if (currentHighlighted === menuItem) { - return; + if (this.highlightedItem_ != menuItem) { + this.setHighlighted(menuItem); } - this.setHighlighted(menuItem); } else { - this.setHighlightedIndex(-1); + this.setHighlighted(null); } } }; @@ -353,7 +330,7 @@ Blockly.Menu.prototype.handleClick_ = function(e) { var oldCoords = this.openingCoords; // Clear out the saved opening coords immediately so they're not used twice. this.openingCoords = null; - if (oldCoords && typeof e.clientX === 'number') { + if (oldCoords && typeof e.clientX == 'number') { var newCoords = new Blockly.utils.Coordinate(e.clientX, e.clientY); if (Blockly.utils.Coordinate.distance(oldCoords, newCoords) < 1) { // This menu was opened by a mousedown and we're handling the consequent @@ -387,7 +364,7 @@ Blockly.Menu.prototype.handleMouseEnter_ = function(_e) { Blockly.Menu.prototype.handleMouseLeave_ = function(_e) { if (this.getElement()) { this.blur(); - this.setHighlightedIndex(-1); + this.setHighlighted(null); } }; @@ -421,7 +398,7 @@ Blockly.Menu.prototype.handleKeyEventInternal = function(e) { return false; } - var highlighted = this.getHighlighted(); + var highlighted = this.highlightedItem_; switch (e.keyCode) { case Blockly.utils.KeyCodes.ENTER: case Blockly.utils.KeyCodes.SPACE: From 76b5a36862424a69388c9f8bdbb05bf6539a5e91 Mon Sep 17 00:00:00 2001 From: Neil Fraser Date: Tue, 5 May 2020 20:24:46 -0700 Subject: [PATCH 19/30] Rename CSS classes goog-menu* to blocklyMenu* Old classes remain in DOM and are deprecated so that any custom CSS will continue to function. --- core/components/menu/menu.js | 9 +++++---- core/components/menu/menuitem.js | 33 +++++++++++++++++++++---------- core/css.js | 34 ++++++++++++-------------------- core/dropdowndiv.js | 4 ++-- 4 files changed, 43 insertions(+), 37 deletions(-) diff --git a/core/components/menu/menu.js b/core/components/menu/menu.js index 04b8f7c248e..201ba07e5ed 100644 --- a/core/components/menu/menu.js +++ b/core/components/menu/menu.js @@ -99,7 +99,8 @@ Blockly.Menu.prototype.addChild = function(menuItem) { */ Blockly.Menu.prototype.render = function(container) { var element = document.createElement('div'); - element.className = 'goog-menu blocklyNonSelectable'; + // goog-menu is deprecated, use blocklyMenu. May 2020. + element.className = 'blocklyMenu goog-menu blocklyNonSelectable'; element.tabIndex = 0; Blockly.utils.aria.setRole(element, this.roleName_); this.element_ = element; @@ -141,7 +142,7 @@ Blockly.Menu.prototype.focus = function() { var el = this.getElement(); if (el) { el.focus({preventScroll:true}); - Blockly.utils.dom.addClass(el, 'focused'); + Blockly.utils.dom.addClass(el, 'blocklyFocused'); } }; @@ -153,7 +154,7 @@ Blockly.Menu.prototype.blur = function() { var el = this.getElement(); if (el) { el.blur(); - Blockly.utils.dom.removeClass(el, 'focused'); + Blockly.utils.dom.removeClass(el, 'blocklyFocused'); } }; @@ -223,7 +224,7 @@ Blockly.Menu.prototype.getMenuItem = function(node) { /** * Highlights the given menu item, or clears highlighting if null. - * @param {!Blockly.MenuItem} item Item to highlight. + * @param {Blockly.MenuItem} item Item to highlight, or null. * @protected */ Blockly.Menu.prototype.setHighlighted = function(item) { diff --git a/core/components/menu/menuitem.js b/core/components/menu/menuitem.js index 9ecd6d47417..55bbe606a86 100644 --- a/core/components/menu/menuitem.js +++ b/core/components/menu/menuitem.js @@ -74,17 +74,18 @@ Blockly.MenuItem.prototype.createDom = function() { this.element_ = element; // Set class and style - element.className = 'goog-menuitem ' + - (!this.enabled_ ? 'goog-menuitem-disabled ' : '') + - (this.checked_ ? 'goog-option-selected ' : '') + - (this.rightToLeft_ ? 'goog-menuitem-rtl ' : ''); + // goog-menuitem* is deprecated, use blocklyMenuItem*. May 2020. + element.className = 'blocklyMenuItem goog-menuitem ' + + (this.enabled_ ? '' : 'blocklyMenuItemDisabled goog-menuitem-disabled ') + + (this.checked_ ? 'blocklyMenuItemSelected goog-option-selected ' : '') + + (this.rightToLeft_ ? 'blocklyMenuItemRtl goog-menuitem-rtl ' : ''); var content = document.createElement('div'); - content.className = 'goog-menuitem-content'; + content.className = 'blocklyMenuItemContent goog-menuitem-content'; // Add a checkbox for checkable menu items. if (this.checkable_) { var checkbox = document.createElement('div'); - checkbox.className = 'goog-menuitem-checkbox'; + checkbox.className = 'blocklyMenuItemCheckbox goog-menuitem-checkbox'; content.appendChild(checkbox); } @@ -183,10 +184,16 @@ Blockly.MenuItem.prototype.setHighlighted = function(highlight) { var el = this.getElement(); if (el && this.isEnabled()) { + // goog-menuitem-highlight is deprecated, use blocklyMenuItemHighlight. + // May 2020. + var name = 'blocklyMenuItemHighlight'; + var nameDep = 'goog-menuitem-highlight'; if (highlight) { - Blockly.utils.dom.addClass(el, 'goog-menuitem-highlight'); + Blockly.utils.dom.addClass(el, name); + Blockly.utils.dom.addClass(el, nameDep); } else { - Blockly.utils.dom.removeClass(el, 'goog-menuitem-highlight'); + Blockly.utils.dom.removeClass(el, name); + Blockly.utils.dom.removeClass(el, nameDep); } } }; @@ -210,10 +217,16 @@ Blockly.MenuItem.prototype.setEnabled = function(enabled) { var el = this.getElement(); if (el) { + // goog-menuitem-disabled is deprecated, use blocklyMenuItemDisabled. + // May 2020. + var name = 'blocklyMenuItemDisabled'; + var nameDep = 'goog-menuitem-disabled'; if (enabled) { - Blockly.utils.dom.removeClass(el, 'goog-menuitem-disabled'); + Blockly.utils.dom.removeClass(el, name); + Blockly.utils.dom.removeClass(el, nameDep); } else { - Blockly.utils.dom.addClass(el, 'goog-menuitem-disabled'); + Blockly.utils.dom.addClass(el, name); + Blockly.utils.dom.addClass(el, nameDep); } } }; diff --git a/core/css.js b/core/css.js index 7ba7e7363a0..aa5b773dbfb 100644 --- a/core/css.js +++ b/core/css.js @@ -171,7 +171,7 @@ Blockly.Css.CONTENT = [ 'box-shadow: 0 0 3px 1px rgba(0,0,0,.3);', '}', - '.blocklyDropDownDiv.focused {', + '.blocklyDropDownDiv.blocklyFocused {', 'box-shadow: 0 0 6px 1px rgba(0,0,0,.3);', '}', @@ -476,21 +476,19 @@ Blockly.Css.CONTENT = [ 'padding: 0 !important;', '}', - '.blocklyWidgetDiv .blocklyDropdownMenu .goog-menuitem,', - '.blocklyDropDownDiv .blocklyDropdownMenu .goog-menuitem {', + '.blocklyDropdownMenu .blocklyMenuItem {', /* 28px on the left for icon or checkbox. */ 'padding-left: 28px;', '}', /* BiDi override for the resting state. */ - '.blocklyWidgetDiv .blocklyDropdownMenu .goog-menuitem.goog-menuitem-rtl,', - '.blocklyDropDownDiv .blocklyDropdownMenu .goog-menuitem.goog-menuitem-rtl {', + '.blocklyDropdownMenu .blocklyMenuItemRtl {', /* Flip left/right padding for BiDi. */ 'padding-left: 5px;', 'padding-right: 28px;', '}', - '.blocklyWidgetDiv .goog-menu {', + '.blocklyWidgetDiv .blocklyMenu {', 'background: #fff;', 'border: 1px solid transparent;', 'box-shadow: 0 0 3px 1px rgba(0,0,0,.3);', @@ -505,19 +503,18 @@ Blockly.Css.CONTENT = [ 'z-index: 20000;', /* Arbitrary, but some apps depend on it... */ '}', - '.blocklyWidgetDiv .goog-menu.focused {', + '.blocklyWidgetDiv .blocklyMenu.blocklyFocused {', 'box-shadow: 0 0 6px 1px rgba(0,0,0,.3);', '}', - '.blocklyDropDownDiv .goog-menu {', + '.blocklyDropDownDiv .blocklyMenu {', 'font: normal 13px "Helvetica Neue", Helvetica, sans-serif;', 'outline: none;', 'z-index: 20000;', /* Arbitrary, but some apps depend on it... */ '}', /* State: resting. */ - '.blocklyWidgetDiv .goog-menuitem,', - '.blocklyDropDownDiv .goog-menuitem {', + '.blocklyMenuItem {', 'border: none;', 'color: #000;', 'cursor: pointer;', @@ -530,36 +527,31 @@ Blockly.Css.CONTENT = [ '}', /* State: disabled. */ - '.blocklyWidgetDiv .goog-menuitem-disabled,', - '.blocklyDropDownDiv .goog-menuitem-disabled {', - 'color: #ccc !important;', + '.blocklyMenuItemDisabled {', + 'color: #ccc;', 'cursor: inherit;', '}', /* State: hover. */ - '.blocklyWidgetDiv .goog-menuitem-highlight ,', - '.blocklyDropDownDiv .goog-menuitem-highlight {', + '.blocklyMenuItemHighlight {', 'background-color: rgba(0,0,0,.1);', '}', /* State: selected/checked. */ - '.blocklyWidgetDiv .goog-menuitem-checkbox,', - '.blocklyDropDownDiv .goog-menuitem-checkbox {', + '.blocklyMenuItemCheckbox {', 'height: 16px;', 'position: absolute;', 'width: 16px;', '}', - '.blocklyWidgetDiv .goog-option-selected .goog-menuitem-checkbox,', - '.blocklyDropDownDiv .goog-option-selected .goog-menuitem-checkbox {', + '.blocklyMenuItemSelected .blocklyMenuItemCheckbox {', 'background: url(<<>>/sprites.png) no-repeat -48px -16px;', 'float: left;', 'margin-left: -24px;', 'position: static;', /* Scroll with the menu. */ '}', - '.blocklyWidgetDiv .goog-menuitem-rtl .goog-menuitem-checkbox,', - '.blocklyDropDownDiv .goog-menuitem-rtl .goog-menuitem-checkbox {', + '.blocklyMenuItemRtl .blocklyMenuItemCheckbox {', 'float: right;', 'margin-right: -24px;', '}', diff --git a/core/dropdowndiv.js b/core/dropdowndiv.js index 47af904d515..a84920b1ccd 100644 --- a/core/dropdowndiv.js +++ b/core/dropdowndiv.js @@ -166,10 +166,10 @@ Blockly.DropDownDiv.createDom = function() { // Handle focusin/out events to add a visual indicator when // a child is focused or blurred. div.addEventListener('focusin', function() { - Blockly.utils.dom.addClass(div, 'focused'); + Blockly.utils.dom.addClass(div, 'blocklyFocused'); }); div.addEventListener('focusout', function() { - Blockly.utils.dom.removeClass(div, 'focused'); + Blockly.utils.dom.removeClass(div, 'blocklyFocused'); }); }; From 3148ae352084e36e7c4407710b4bb3def7001d32 Mon Sep 17 00:00:00 2001 From: Neil Fraser Date: Tue, 5 May 2020 20:25:06 -0700 Subject: [PATCH 20/30] Remove unused focus tracker in tree. --- core/components/tree/treecontrol.js | 63 +---------------------------- 1 file changed, 1 insertion(+), 62 deletions(-) diff --git a/core/components/tree/treecontrol.js b/core/components/tree/treecontrol.js index d69e54bf576..35d6cf02810 100644 --- a/core/components/tree/treecontrol.js +++ b/core/components/tree/treecontrol.js @@ -33,20 +33,6 @@ goog.require('Blockly.utils.style'); Blockly.tree.TreeControl = function(toolbox, config) { this.toolbox_ = toolbox; - /** - * Focus event data. - * @type {?Blockly.EventData} - * @private - */ - this.onFocusWrapper_ = null; - - /** - * Blur event data. - * @type {?Blockly.EventData} - * @private - */ - this.onBlurWrapper_ = null; - /** * Click event data. * @type {?Blockly.EventData} @@ -66,7 +52,7 @@ Blockly.tree.TreeControl = function(toolbox, config) { // The root is open and selected by default. this.expanded_ = true; this.selected_ = true; - + /** * Currently selected item. * @type {Blockly.tree.BaseNode} @@ -101,41 +87,6 @@ Blockly.tree.TreeControl.prototype.getDepth = function() { return 0; }; -/** - * Handles focus on the tree. - * @param {!Event} _e The browser event. - * @private - */ -Blockly.tree.TreeControl.prototype.handleFocus_ = function(_e) { - this.focused_ = true; - var el = /** @type {!Element} */ (this.getElement()); - Blockly.utils.dom.addClass(el, 'focused'); - - if (this.selectedItem_) { - this.selectedItem_.select(); - } -}; - -/** - * Handles blur on the tree. - * @param {!Event} _e The browser event. - * @private - */ -Blockly.tree.TreeControl.prototype.handleBlur_ = function(_e) { - this.focused_ = false; - var el = /** @type {!Element} */ (this.getElement()); - Blockly.utils.dom.removeClass(el, 'focused'); -}; - -/** - * Get whether this tree has focus or not. - * @return {boolean} True if it has focus. - * @package - */ -Blockly.tree.TreeControl.prototype.hasFocus = function() { - return this.focused_; -}; - /** @override */ Blockly.tree.TreeControl.prototype.setExpanded = function(expanded) { this.expanded_ = expanded; @@ -278,10 +229,6 @@ Blockly.tree.TreeControl.prototype.attachEvents_ = function() { var el = this.getElement(); el.tabIndex = 0; - this.onFocusWrapper_ = Blockly.bindEvent_(el, - 'focus', this, this.handleFocus_); - this.onBlurWrapper_ = Blockly.bindEvent_(el, - 'blur', this, this.handleBlur_); this.onClickWrapper_ = Blockly.bindEventWithChecks_(el, 'click', this, this.handleMouseEvent_); this.onKeydownWrapper_ = Blockly.bindEvent_(el, @@ -293,14 +240,6 @@ Blockly.tree.TreeControl.prototype.attachEvents_ = function() { * @private */ Blockly.tree.TreeControl.prototype.detachEvents_ = function() { - if (this.onFocusWrapper_) { - Blockly.unbindEvent_(this.onFocusWrapper_); - this.onFocusWrapper_ = null; - } - if (this.onBlurWrapper_) { - Blockly.unbindEvent_(this.onBlurWrapper_); - this.onBlurWrapper_ = null; - } if (this.onClickWrapper_) { Blockly.unbindEvent_(this.onClickWrapper_); this.onClickWrapper_ = null; From 2e0e6df226bd413f7338fc74298f099124a9cdbd Mon Sep 17 00:00:00 2001 From: Neil Fraser Date: Tue, 5 May 2020 20:44:37 -0700 Subject: [PATCH 21/30] Add support for space/enter to toggle tree cats --- core/components/tree/basenode.js | 29 +++++++++++++++++------------ core/components/tree/treecontrol.js | 14 ++++---------- 2 files changed, 21 insertions(+), 22 deletions(-) diff --git a/core/components/tree/basenode.js b/core/components/tree/basenode.js index a9c49b4ee40..0eb2ab2f677 100644 --- a/core/components/tree/basenode.js +++ b/core/components/tree/basenode.js @@ -731,14 +731,14 @@ Blockly.tree.BaseNode.prototype.onKeyDown = function(e) { var handled = true; switch (e.keyCode) { case Blockly.utils.KeyCodes.RIGHT: - if (e.altKey) { + if (e.altKey) { // Why? break; } handled = this.selectChild(); break; case Blockly.utils.KeyCodes.LEFT: - if (e.altKey) { + if (e.altKey) { // Why? break; } handled = this.selectParent(); @@ -752,6 +752,12 @@ Blockly.tree.BaseNode.prototype.onKeyDown = function(e) { handled = this.selectPrevious(); break; + case Blockly.utils.KeyCodes.ENTER: + case Blockly.utils.KeyCodes.SPACE: + this.setExpanded(!this.expanded_); + handled = true; + break; + default: handled = false; } @@ -847,18 +853,17 @@ Blockly.tree.BaseNode.prototype.getLastShownDescendant = function() { Blockly.tree.BaseNode.prototype.getNextShownNode = function() { if (this.hasChildren() && this.expanded_) { return this.getChildAt(0); - } else { - var parent = this; - var next; - while (parent != this.getTree()) { - next = parent.getNextSibling(); - if (next != null) { - return next; - } - parent = parent.getParent(); + } + var parent = this; + var next; + while (parent != this.getTree()) { + next = parent.getNextSibling(); + if (next != null) { + return next; } - return null; + parent = parent.getParent(); } + return null; }; /** diff --git a/core/components/tree/treecontrol.js b/core/components/tree/treecontrol.js index 35d6cf02810..37f8dc779d2 100644 --- a/core/components/tree/treecontrol.js +++ b/core/components/tree/treecontrol.js @@ -257,12 +257,8 @@ Blockly.tree.TreeControl.prototype.detachEvents_ = function() { */ Blockly.tree.TreeControl.prototype.handleMouseEvent_ = function(e) { var node = this.getNodeFromEvent_(e); - if (node) { - switch (e.type) { - case 'click': - node.onClick_(e); - break; - } + if (node && e.type == 'click') { + node.onClick_(e); } }; @@ -273,10 +269,8 @@ Blockly.tree.TreeControl.prototype.handleMouseEvent_ = function(e) { * @private */ Blockly.tree.TreeControl.prototype.handleKeyEvent_ = function(e) { - var handled = false; - // Handle navigation keystrokes. - handled = (this.selectedItem_ && this.selectedItem_.onKeyDown(e)) || handled; + var handled = !!(this.selectedItem_ && this.selectedItem_.onKeyDown(e)); if (handled) { Blockly.utils.style.scrollIntoContainerView( @@ -299,7 +293,7 @@ Blockly.tree.TreeControl.prototype.getNodeFromEvent_ = function(e) { // find the right node var node = null; var target = e.target; - while (target != null) { + while (target) { var id = target.id; node = Blockly.tree.BaseNode.allNodes[id]; if (node) { From 197f91938e7d1a54c102719f9082c5422e5a53b2 Mon Sep 17 00:00:00 2001 From: Neil Fraser Date: Tue, 5 May 2020 21:54:36 -0700 Subject: [PATCH 22/30] Delete unsettable .isUserCollapsible_ from tree --- core/components/component.js | 12 ++++++++---- core/components/menu/menu.js | 4 +++- core/components/tree/basenode.js | 25 ++++++++----------------- core/components/tree/treenode.js | 2 +- 4 files changed, 20 insertions(+), 23 deletions(-) diff --git a/core/components/component.js b/core/components/component.js index 6d7c13656e7..0d985b7ef47 100644 --- a/core/components/component.js +++ b/core/components/component.js @@ -115,7 +115,12 @@ Blockly.Component.Error = { * Error when an attempt is made to add a child component at an out-of-bounds * index. We don't support sparse child arrays. */ - CHILD_INDEX_OUT_OF_BOUNDS: 'Child component index out of bounds' + CHILD_INDEX_OUT_OF_BOUNDS: 'Child component index out of bounds', + + /** + * Error when calling an abstract method that should be overriden. + */ + ABSTRACT_METHOD: 'Unimplemented abstract method' }; /** @@ -195,12 +200,11 @@ Blockly.Component.prototype.isInDocument = function() { }; /** - * Creates the initial DOM representation for the component. The default - * implementation is to set this.element_ = div. + * Creates the initial DOM representation for the component. * @protected */ Blockly.Component.prototype.createDom = function() { - this.element_ = document.createElement('div'); + throw Error(Blockly.Component.Error.ABSTRACT_METHOD); }; /** diff --git a/core/components/menu/menu.js b/core/components/menu/menu.js index 201ba07e5ed..ee0aa991500 100644 --- a/core/components/menu/menu.js +++ b/core/components/menu/menu.js @@ -26,7 +26,9 @@ goog.require('Blockly.utils.style'); Blockly.Menu = function() { /** * Array of menu items. - * @type {!Array.} + * (Nulls are never in the array, but typing the array as nullable prevents + * the compiler from objecting to .indexOf(null)) + * @type {!Array.} * @private */ this.menuItems_ = []; diff --git a/core/components/tree/basenode.js b/core/components/tree/basenode.js index 0eb2ab2f677..793dd1c2c06 100644 --- a/core/components/tree/basenode.js +++ b/core/components/tree/basenode.js @@ -91,13 +91,6 @@ Blockly.tree.BaseNode = function(content, config) { */ this.expanded_ = false; - /** - * Whether to allow user to collapse this node. - * @type {boolean} - * @protected - */ - this.isUserCollapsible_ = true; - /** * Nesting depth of this node; cached result of getDepth. * -1 if value has not been cached. @@ -174,8 +167,7 @@ Blockly.tree.BaseNode.prototype.initAccessibility = function() { var ce = this.getChildrenElement(); if (ce) { - Blockly.utils.aria.setRole(ce, - Blockly.utils.aria.Role.GROUP); + Blockly.utils.aria.setRole(ce, Blockly.utils.aria.Role.GROUP); // In case the children will be created lazily. if (ce.hasChildNodes()) { @@ -347,18 +339,17 @@ Blockly.tree.BaseNode.prototype.setDepth_ = function(depth) { * @protected */ Blockly.tree.BaseNode.prototype.contains = function(node) { - var current = node; - while (current) { - if (current == this) { + while (node) { + if (node == this) { return true; } - current = current.getParent(); + node = node.getParent(); } return false; }; /** - * This is re-defined here to indicate to the closure compiler the correct + * This is re-defined here to indicate to the Closure Compiler the correct * child return type. * @param {number} index 0-based index. * @return {Blockly.tree.BaseNode} The child at the given index; null if none. @@ -617,7 +608,7 @@ Blockly.tree.BaseNode.prototype.getIconDom = function() { * @protected */ Blockly.tree.BaseNode.prototype.getCalculatedIconClass = function() { - throw Error('unimplemented abstract method'); + throw Error(Blockly.Component.Error.ABSTRACT_METHOD); }; /** @@ -754,7 +745,7 @@ Blockly.tree.BaseNode.prototype.onKeyDown = function(e) { case Blockly.utils.KeyCodes.ENTER: case Blockly.utils.KeyCodes.SPACE: - this.setExpanded(!this.expanded_); + this.toggle(); handled = true; break; @@ -802,7 +793,7 @@ Blockly.tree.BaseNode.prototype.selectPrevious = function() { * @package */ Blockly.tree.BaseNode.prototype.selectParent = function() { - if (this.hasChildren() && this.expanded_ && this.isUserCollapsible_) { + if (this.hasChildren() && this.expanded_) { this.setExpanded(false); } else { var parent = this.getParent(); diff --git a/core/components/tree/treenode.js b/core/components/tree/treenode.js index db75f3cefa1..455e7b0f2d9 100644 --- a/core/components/tree/treenode.js +++ b/core/components/tree/treenode.js @@ -93,7 +93,7 @@ Blockly.tree.TreeNode.prototype.getCalculatedIconClass = function() { */ Blockly.tree.TreeNode.prototype.onClick_ = function(_e) { // Expand icon. - if (this.hasChildren() && this.isUserCollapsible_) { + if (this.hasChildren()) { this.toggle(); this.select(); } else if (this.isSelected()) { From f269e122568bc8b5e859916d51b38f730806c603 Mon Sep 17 00:00:00 2001 From: Neil Fraser Date: Tue, 5 May 2020 22:46:51 -0700 Subject: [PATCH 23/30] Minor CSS cleanup --- core/block_dragger.js | 2 +- core/components/tree/basenode.js | 4 ++-- core/css.js | 4 ++-- core/flyout_button.js | 2 +- core/keyboard_nav/basic_cursor.js | 2 +- core/keyboard_nav/marker.js | 2 -- core/renderers/common/constants.js | 9 ++++----- core/renderers/common/info.js | 2 +- core/renderers/geras/constants.js | 2 +- core/renderers/geras/path_object.js | 2 +- core/renderers/zelos/constants.js | 21 ++++++++++----------- core/theme.js | 2 +- core/toolbox.js | 7 +++---- core/workspace_audio.js | 2 +- core/workspace_comment_svg.js | 6 +++--- 15 files changed, 32 insertions(+), 37 deletions(-) diff --git a/core/block_dragger.js b/core/block_dragger.js index 1a8b9ea3689..c545ba748ae 100644 --- a/core/block_dragger.js +++ b/core/block_dragger.js @@ -220,7 +220,7 @@ Blockly.BlockDragger.prototype.endBlockDrag = function(e, currentDragDeltaXY) { this.dragBlock(e, currentDragDeltaXY); this.dragIconData_ = []; this.fireDragEndEvent_(); - + Blockly.utils.dom.stopTextWidthCache(); Blockly.blockAnimations.disconnectUiStop(); diff --git a/core/components/tree/basenode.js b/core/components/tree/basenode.js index 793dd1c2c06..9c7fbea78b9 100644 --- a/core/components/tree/basenode.js +++ b/core/components/tree/basenode.js @@ -332,8 +332,8 @@ Blockly.tree.BaseNode.prototype.setDepth_ = function(depth) { }; /** - * Returns true if the node is a descendant of this node - * @param {Blockly.tree.BaseNode} node The node to check. + * Returns true if the node is a descendant of this node. + * @param {Blockly.tree.Component} node The node to check. * @return {boolean} True if the node is a descendant of this node, false * otherwise. * @protected diff --git a/core/css.js b/core/css.js index aa5b773dbfb..d03769f38ca 100644 --- a/core/css.js +++ b/core/css.js @@ -308,7 +308,7 @@ Blockly.Css.CONTENT = [ '.blocklyInsertionMarker>.blocklyPathLight,', '.blocklyInsertionMarker>.blocklyPathDark {', 'fill-opacity: .2;', - 'stroke: none', + 'stroke: none;', '}', '.blocklyMultilineText {', @@ -453,7 +453,7 @@ Blockly.Css.CONTENT = [ '.blocklyVerticalMarker {', 'stroke-width: 3px;', 'fill: rgba(255,255,255,.5);', - 'pointer-events: none', + 'pointer-events: none;', '}', '.blocklyComputeCanvas {', diff --git a/core/flyout_button.js b/core/flyout_button.js index f4cead8af6b..e0c6c010b55 100644 --- a/core/flyout_button.js +++ b/core/flyout_button.js @@ -166,7 +166,7 @@ Blockly.FlyoutButton.prototype.createDom = function() { var fontMetrics = Blockly.utils.dom.measureFontMetrics(text, fontSize, fontWeight, fontFamily); this.height = fontMetrics.height; - + if (!this.isLabel_) { this.width += 2 * Blockly.FlyoutButton.MARGIN_X; this.height += 2 * Blockly.FlyoutButton.MARGIN_Y; diff --git a/core/keyboard_nav/basic_cursor.js b/core/keyboard_nav/basic_cursor.js index 981c8cc89ae..dca508745fb 100644 --- a/core/keyboard_nav/basic_cursor.js +++ b/core/keyboard_nav/basic_cursor.js @@ -71,7 +71,7 @@ Blockly.BasicCursor.prototype.prev = function() { return null; } var newNode = this.getPreviousNode_(curNode, this.validNode_); - + if (newNode) { this.setCurNode(newNode); } diff --git a/core/keyboard_nav/marker.js b/core/keyboard_nav/marker.js index 069475985a7..2394e393e1e 100644 --- a/core/keyboard_nav/marker.js +++ b/core/keyboard_nav/marker.js @@ -23,7 +23,6 @@ goog.require('Blockly.navigation'); * @constructor */ Blockly.Marker = function() { - /** * The colour of the marker. * @type {?string} @@ -119,4 +118,3 @@ Blockly.Marker.prototype.dispose = function() { this.getDrawer().dispose(); } }; - diff --git a/core/renderers/common/constants.js b/core/renderers/common/constants.js index 8a5b19cff95..434e5e3cdfe 100644 --- a/core/renderers/common/constants.js +++ b/core/renderers/common/constants.js @@ -256,7 +256,7 @@ Blockly.blockRendering.ConstantProvider = function() { * @type {number} */ this.FIELD_TEXT_HEIGHT = -1; // Dynamically set - + /** * Text baseline. This constant is dynamically set in ``setFontConstants_`` * to be the baseline of the text based on the font used. @@ -1168,9 +1168,8 @@ Blockly.blockRendering.ConstantProvider.prototype.getCSS_ = function(selector) { // Text. selector + ' .blocklyText, ', selector + ' .blocklyFlyoutLabelText {', - 'font-family: ' + this.FIELD_TEXT_FONTFAMILY + ';', - 'font-size: ' + this.FIELD_TEXT_FONTSIZE + 'pt;', - 'font-weight: ' + this.FIELD_TEXT_FONTWEIGHT + ';', + 'font: ' + this.FIELD_TEXT_FONTWEIGHT + ' ' + + this.FIELD_TEXT_FONTSIZE + 'pt ' + this.FIELD_TEXT_FONTFAMILY + ';', '}', // Fields. @@ -1233,7 +1232,7 @@ Blockly.blockRendering.ConstantProvider.prototype.getCSS_ = function(selector) { // Insertion marker. selector + ' .blocklyInsertionMarker>.blocklyPath {', 'fill-opacity: ' + this.INSERTION_MARKER_OPACITY + ';', - 'stroke: none', + 'stroke: none;', '}', /* eslint-enable indent */ ]; diff --git a/core/renderers/common/info.js b/core/renderers/common/info.js index a496243161c..e58fc842dd9 100644 --- a/core/renderers/common/info.js +++ b/core/renderers/common/info.js @@ -581,7 +581,7 @@ Blockly.blockRendering.RenderInfo.prototype.addAlignmentPadding_ = function(row, if (row.hasExternalInput || row.hasStatement) { row.widthWithConnectedBlocks += missingSpace; } - + // Decide where the extra padding goes. if (row.align == Blockly.ALIGN_LEFT) { // Add padding to the end of the row. diff --git a/core/renderers/geras/constants.js b/core/renderers/geras/constants.js index 0499a135bc0..ee7b9685686 100644 --- a/core/renderers/geras/constants.js +++ b/core/renderers/geras/constants.js @@ -57,7 +57,7 @@ Blockly.geras.ConstantProvider.prototype.getCSS_ = function(selector) { selector + ' .blocklyInsertionMarker>.blocklyPathLight,', selector + ' .blocklyInsertionMarker>.blocklyPathDark {', 'fill-opacity: ' + this.INSERTION_MARKER_OPACITY + ';', - 'stroke: none', + 'stroke: none;', '}', /* eslint-enable indent */ ]); diff --git a/core/renderers/geras/path_object.js b/core/renderers/geras/path_object.js index eed4775fef4..dd572aab7c8 100644 --- a/core/renderers/geras/path_object.js +++ b/core/renderers/geras/path_object.js @@ -122,7 +122,7 @@ Blockly.geras.PathObject.prototype.applyColour = function(block) { this.svgPathDark.setAttribute('fill', this.colourDark); Blockly.geras.PathObject.superClass_.applyColour.call(this, block); - + this.svgPath.setAttribute('stroke', 'none'); }; diff --git a/core/renderers/zelos/constants.js b/core/renderers/zelos/constants.js index 57ebdca1176..acfe8c36e08 100644 --- a/core/renderers/zelos/constants.js +++ b/core/renderers/zelos/constants.js @@ -69,7 +69,7 @@ Blockly.zelos.ConstantProvider = function() { * @override */ this.NOTCH_OFFSET_LEFT = 3 * this.GRID_UNIT; - + /** * @override */ @@ -259,7 +259,7 @@ Blockly.zelos.ConstantProvider = function() { * @override */ this.FIELD_BORDER_RECT_X_PADDING = 2 * this.GRID_UNIT; - + /** * @override */ @@ -905,13 +905,12 @@ Blockly.zelos.ConstantProvider.prototype.getCSS_ = function(selector) { return [ /* eslint-disable indent */ // Text. - selector + ' .blocklyText, ', + selector + ' .blocklyText,', selector + ' .blocklyFlyoutLabelText {', - 'font-family: ' + this.FIELD_TEXT_FONTFAMILY + ';', - 'font-size: ' + this.FIELD_TEXT_FONTSIZE + 'pt;', - 'font-weight: ' + this.FIELD_TEXT_FONTWEIGHT + ';', + 'font: ' + this.FIELD_TEXT_FONTWEIGHT + ' ' + + this.FIELD_TEXT_FONTSIZE + 'pt ' + this.FIELD_TEXT_FONTFAMILY + ';', '}', - + // Fields. selector + ' .blocklyText {', 'fill: #fff;', @@ -926,7 +925,7 @@ Blockly.zelos.ConstantProvider.prototype.getCSS_ = function(selector) { selector + ' .blocklyEditableText>g>text {', 'fill: #575E75;', '}', - + // Flyout labels. selector + ' .blocklyFlyoutLabelText {', 'fill: #575E75;', @@ -939,7 +938,7 @@ Blockly.zelos.ConstantProvider.prototype.getCSS_ = function(selector) { // Editable field hover. selector + ' .blocklyDraggable:not(.blocklyDisabled)', - ' .blocklyEditableText:not(.editing):hover>rect ,', + ' .blocklyEditableText:not(.editing):hover>rect,', selector + ' .blocklyDraggable:not(.blocklyDisabled)', ' .blocklyEditableText:not(.editing):hover>.blocklyPath {', 'stroke: #fff;', @@ -952,7 +951,7 @@ Blockly.zelos.ConstantProvider.prototype.getCSS_ = function(selector) { 'font-weight: ' + this.FIELD_TEXT_FONTWEIGHT + ';', 'color: #575E75;', '}', - + // Dropdown field. selector + ' .blocklyDropdownText {', 'fill: #fff !important;', @@ -979,7 +978,7 @@ Blockly.zelos.ConstantProvider.prototype.getCSS_ = function(selector) { // Insertion marker. selector + ' .blocklyInsertionMarker>.blocklyPath {', 'fill-opacity: ' + this.INSERTION_MARKER_OPACITY + ';', - 'stroke: none', + 'stroke: none;', '}', /* eslint-enable indent */ ]; diff --git a/core/theme.js b/core/theme.js index ea88f5a6410..e314b764201 100644 --- a/core/theme.js +++ b/core/theme.js @@ -210,7 +210,7 @@ Blockly.Theme.defineTheme = function(name, themeObj) { Blockly.utils.object.deepMerge(theme, base); theme.name = name; } - + Blockly.utils.object.deepMerge(theme.blockStyles, themeObj['blockStyles']); Blockly.utils.object.deepMerge(theme.categoryStyles, diff --git a/core/toolbox.js b/core/toolbox.js index 8798c2d9451..38ce94f01b6 100644 --- a/core/toolbox.js +++ b/core/toolbox.js @@ -180,7 +180,7 @@ Blockly.Toolbox.prototype.init = function() { 'rendererOverrides': workspace.options.rendererOverrides })); workspaceOptions.toolboxPosition = workspace.options.toolboxPosition; - + if (workspace.horizontalLayout) { if (!Blockly.HorizontalFlyout) { throw Error('Missing require for Blockly.HorizontalFlyout'); @@ -196,7 +196,7 @@ Blockly.Toolbox.prototype.init = function() { throw Error('One of Blockly.VerticalFlyout or Blockly.Horizontal must be' + 'required.'); } - + // Insert the flyout after the workspace. Blockly.utils.dom.insertAfter(this.flyout_.createDom('svg'), svg); this.flyout_.init(workspace); @@ -830,8 +830,7 @@ Blockly.Css.register([ '.blocklyTreeLabel {', 'cursor: default;', - 'font-family: sans-serif;', - 'font-size: 16px;', + 'font: 16px sans-serif;', 'padding: 0 3px;', 'vertical-align: middle;', '}', diff --git a/core/workspace_audio.js b/core/workspace_audio.js index 9d2b358ffcf..3de7eb1f489 100644 --- a/core/workspace_audio.js +++ b/core/workspace_audio.js @@ -110,7 +110,7 @@ Blockly.WorkspaceAudio.prototype.preload = function() { } else { sound.pause(); } - + // iOS can only process one sound at a time. Trying to load more than one // corrupts the earlier ones. Just load one and leave the others uncached. if (Blockly.utils.userAgent.IPAD || Blockly.utils.userAgent.IPHONE) { diff --git a/core/workspace_comment_svg.js b/core/workspace_comment_svg.js index 5301018cf38..b9fa05a4cac 100644 --- a/core/workspace_comment_svg.js +++ b/core/workspace_comment_svg.js @@ -640,7 +640,7 @@ Blockly.Css.register([ '.blocklyCommentRect {', 'fill: #E7DE8E;', 'stroke: #bcA903;', - 'stroke-width: 1px', + 'stroke-width: 1px;', '}', '.blocklyCommentTarget {', @@ -673,11 +673,11 @@ Blockly.Css.register([ '.blocklyCommentDeleteIcon {', 'cursor: pointer;', 'fill: #000;', - 'display: none', + 'display: none;', '}', '.blocklySelected > .blocklyCommentDeleteIcon {', - 'display: block', + 'display: block;', '}', '.blocklyDeleteIconShape {', From 3e27d5837b50f370e593cd436d709e4a31c850ef Mon Sep 17 00:00:00 2001 From: Neil Fraser Date: Tue, 5 May 2020 22:55:21 -0700 Subject: [PATCH 24/30] Fix bad type annotation. --- core/components/tree/basenode.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/components/tree/basenode.js b/core/components/tree/basenode.js index 9c7fbea78b9..258c5b0a303 100644 --- a/core/components/tree/basenode.js +++ b/core/components/tree/basenode.js @@ -333,7 +333,7 @@ Blockly.tree.BaseNode.prototype.setDepth_ = function(depth) { /** * Returns true if the node is a descendant of this node. - * @param {Blockly.tree.Component} node The node to check. + * @param {Blockly.Component} node The node to check. * @return {boolean} True if the node is a descendant of this node, false * otherwise. * @protected From 2c973e16bfb9de40c90c108c1ebabe320862023f Mon Sep 17 00:00:00 2001 From: Neil Fraser Date: Wed, 6 May 2020 15:32:18 -0700 Subject: [PATCH 25/30] Change visibility tags throughout menus. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous tags were inherited from Closure and don’t reflect current usage in the Blockly codebase. The core/components/tree files are non-compliant in this regard, but I’m not going to update them since they need to be replaced and there’s no need to create an interim API change. --- core/components/menu/menu.js | 71 ++++++++++++++++---------------- core/components/menu/menuitem.js | 2 +- 2 files changed, 36 insertions(+), 37 deletions(-) diff --git a/core/components/menu/menu.js b/core/components/menu/menu.js index ee0aa991500..c678e938f91 100644 --- a/core/components/menu/menu.js +++ b/core/components/menu/menu.js @@ -122,7 +122,7 @@ Blockly.Menu.prototype.render = function(container) { this.mouseLeaveHandler_ = Blockly.bindEventWithChecks_(element, 'mouseleave', this, this.handleMouseLeave_, true); this.onKeyDownHandler_ = Blockly.bindEventWithChecks_(element, - 'keydown', this, this.handleKeyEvent); + 'keydown', this, this.handleKeyEvent_); container.appendChild(element); }; @@ -209,9 +209,9 @@ Blockly.Menu.prototype.dispose = function() { * menu item is found. * @param {Node} node DOM node whose owner is to be returned. * @return {?Blockly.MenuItem} Menu item for which the DOM node belongs to. - * @protected + * @private */ -Blockly.Menu.prototype.getMenuItem = function(node) { +Blockly.Menu.prototype.getMenuItem_ = function(node) { var elem = this.getElement(); while (node && node !== elem) { if (node.blocklyMenuItem) { @@ -227,9 +227,9 @@ Blockly.Menu.prototype.getMenuItem = function(node) { /** * Highlights the given menu item, or clears highlighting if null. * @param {Blockly.MenuItem} item Item to highlight, or null. - * @protected + * @private */ -Blockly.Menu.prototype.setHighlighted = function(item) { +Blockly.Menu.prototype.setHighlighted_ = function(item) { var currentHighlighted = this.highlightedItem_; if (currentHighlighted) { currentHighlighted.setHighlighted(false); @@ -249,37 +249,37 @@ Blockly.Menu.prototype.setHighlighted = function(item) { /** * Highlights the next highlightable item (or the first if nothing is currently * highlighted). - * @package + * @private */ -Blockly.Menu.prototype.highlightNext = function() { +Blockly.Menu.prototype.highlightNext_ = function() { var index = this.menuItems_.indexOf(this.highlightedItem_); - this.highlightHelper(index, 1); + this.highlightHelper_(index, 1); }; /** * Highlights the previous highlightable item (or the last if nothing is * currently highlighted). - * @package + * @private */ -Blockly.Menu.prototype.highlightPrevious = function() { +Blockly.Menu.prototype.highlightPrevious_ = function() { var index = this.menuItems_.indexOf(this.highlightedItem_); - this.highlightHelper(index < 0 ? this.menuItems_.length : index, -1); + this.highlightHelper_(index < 0 ? this.menuItems_.length : index, -1); }; /** * Highlights the first highlightable item. - * @package + * @private */ -Blockly.Menu.prototype.highlightFirst = function() { - this.highlightHelper(-1, 1); +Blockly.Menu.prototype.highlightFirst_ = function() { + this.highlightHelper_(-1, 1); }; /** * Highlights the last highlightable item. - * @package + * @private */ -Blockly.Menu.prototype.highlightLast = function() { - this.highlightHelper(this.menuItems_.length, -1); +Blockly.Menu.prototype.highlightLast_ = function() { + this.highlightHelper_(this.menuItems_.length, -1); }; /** @@ -287,14 +287,14 @@ Blockly.Menu.prototype.highlightLast = function() { * child menuitems in response to keyboard events. * @param {number} startIndex Start index. * @param {number} delta Step direction: 1 to go down, -1 to go up. - * @protected + * @private */ -Blockly.Menu.prototype.highlightHelper = function(startIndex, delta) { +Blockly.Menu.prototype.highlightHelper_ = function(startIndex, delta) { var index = startIndex + delta; var menuItem; while ((menuItem = this.menuItems_[index])) { if (menuItem.isEnabled()) { - this.setHighlighted(menuItem); + this.setHighlighted_(menuItem); break; } index += delta; @@ -310,15 +310,15 @@ Blockly.Menu.prototype.highlightHelper = function(startIndex, delta) { * @private */ Blockly.Menu.prototype.handleMouseOver_ = function(e) { - var menuItem = this.getMenuItem(/** @type {Node} */ (e.target)); + var menuItem = this.getMenuItem_(/** @type {Node} */ (e.target)); if (menuItem) { if (menuItem.isEnabled()) { if (this.highlightedItem_ != menuItem) { - this.setHighlighted(menuItem); + this.setHighlighted_(menuItem); } } else { - this.setHighlighted(null); + this.setHighlighted_(null); } } }; @@ -344,7 +344,7 @@ Blockly.Menu.prototype.handleClick_ = function(e) { } } - var menuItem = this.getMenuItem(/** @type {Node} */ (e.target)); + var menuItem = this.getMenuItem_(/** @type {Node} */ (e.target)); if (menuItem) { menuItem.performAction(); } @@ -367,7 +367,7 @@ Blockly.Menu.prototype.handleMouseEnter_ = function(_e) { Blockly.Menu.prototype.handleMouseLeave_ = function(_e) { if (this.getElement()) { this.blur(); - this.setHighlighted(null); + this.setHighlighted_(null); } }; @@ -375,13 +375,12 @@ Blockly.Menu.prototype.handleMouseLeave_ = function(_e) { /** * Attempts to handle a keyboard event, if the menu item is enabled, by calling - * {@link handleKeyEventInternal}. Considered protected; should only be used - * within this package and by subclasses. + * {@link handleKeyEventInternal_}. * @param {!Event} e Key event to handle. - * @protected + * @private */ -Blockly.Menu.prototype.handleKeyEvent = function(e) { - if (this.menuItems_.length && this.handleKeyEventInternal(e)) { +Blockly.Menu.prototype.handleKeyEvent_ = function(e) { + if (this.menuItems_.length && this.handleKeyEventInternal_(e)) { e.preventDefault(); e.stopPropagation(); } @@ -393,9 +392,9 @@ Blockly.Menu.prototype.handleKeyEvent = function(e) { * @param {!Event} e Key event to handle. * @return {boolean} Whether the event was handled by the container (or one of * its children). - * @protected + * @private */ -Blockly.Menu.prototype.handleKeyEventInternal = function(e) { +Blockly.Menu.prototype.handleKeyEventInternal_ = function(e) { // Do not handle the key event if any modifier key is pressed. if (e.shiftKey || e.ctrlKey || e.metaKey || e.altKey) { return false; @@ -411,21 +410,21 @@ Blockly.Menu.prototype.handleKeyEventInternal = function(e) { break; case Blockly.utils.KeyCodes.UP: - this.highlightPrevious(); + this.highlightPrevious_(); break; case Blockly.utils.KeyCodes.DOWN: - this.highlightNext(); + this.highlightNext_(); break; case Blockly.utils.KeyCodes.PAGE_UP: case Blockly.utils.KeyCodes.HOME: - this.highlightFirst(); + this.highlightFirst_(); break; case Blockly.utils.KeyCodes.PAGE_DOWN: case Blockly.utils.KeyCodes.END: - this.highlightLast(); + this.highlightLast_(); break; default: diff --git a/core/components/menu/menuitem.js b/core/components/menu/menuitem.js index 55bbe606a86..8c24889243f 100644 --- a/core/components/menu/menuitem.js +++ b/core/components/menu/menuitem.js @@ -57,7 +57,7 @@ Blockly.MenuItem = function(content, opt_value) { /** * Whether the menu item is rendered right-to-left. * @type {boolean} - * @protected + * @private */ this.rightToLeft_ = false; }; From 881734b27408f232b7d50bb4babecef2935dbd92 Mon Sep 17 00:00:00 2001 From: Neil Fraser Date: Wed, 6 May 2020 16:37:22 -0700 Subject: [PATCH 26/30] Address review comments on changes to menu. --- core/components/menu/menu.js | 61 +++++++++++++++++--------------- core/components/menu/menuitem.js | 48 +++++++++++++++---------- core/components/tree/basenode.js | 6 ---- core/contextmenu.js | 6 ++-- 4 files changed, 66 insertions(+), 55 deletions(-) diff --git a/core/components/menu/menu.js b/core/components/menu/menu.js index c678e938f91..abf58517979 100644 --- a/core/components/menu/menu.js +++ b/core/components/menu/menu.js @@ -84,6 +84,20 @@ Blockly.Menu = function() { * @private */ this.onKeyDownHandler_ = null; + + /** + * The menu's root DOM element. + * @type {Element} + * @private + */ + this.element_ = null; + + /** + * ARIA name for this menu. + * @type {string} + * @private + */ + this.roleName_ = ''; }; @@ -150,9 +164,9 @@ Blockly.Menu.prototype.focus = function() { /** * Blur the menu element. - * @package + * @private */ -Blockly.Menu.prototype.blur = function() { +Blockly.Menu.prototype.blur_ = function() { var el = this.getElement(); if (el) { el.blur(); @@ -249,9 +263,9 @@ Blockly.Menu.prototype.setHighlighted_ = function(item) { /** * Highlights the next highlightable item (or the first if nothing is currently * highlighted). - * @private + * @package */ -Blockly.Menu.prototype.highlightNext_ = function() { +Blockly.Menu.prototype.highlightNext = function() { var index = this.menuItems_.indexOf(this.highlightedItem_); this.highlightHelper_(index, 1); }; @@ -259,9 +273,9 @@ Blockly.Menu.prototype.highlightNext_ = function() { /** * Highlights the previous highlightable item (or the last if nothing is * currently highlighted). - * @private + * @package */ -Blockly.Menu.prototype.highlightPrevious_ = function() { +Blockly.Menu.prototype.highlightPrevious = function() { var index = this.menuItems_.indexOf(this.highlightedItem_); this.highlightHelper_(index < 0 ? this.menuItems_.length : index, -1); }; @@ -366,7 +380,7 @@ Blockly.Menu.prototype.handleMouseEnter_ = function(_e) { */ Blockly.Menu.prototype.handleMouseLeave_ = function(_e) { if (this.getElement()) { - this.blur(); + this.blur_(); this.setHighlighted_(null); } }; @@ -380,24 +394,13 @@ Blockly.Menu.prototype.handleMouseLeave_ = function(_e) { * @private */ Blockly.Menu.prototype.handleKeyEvent_ = function(e) { - if (this.menuItems_.length && this.handleKeyEventInternal_(e)) { - e.preventDefault(); - e.stopPropagation(); + if (!this.menuItems_.length) { + // Empty menu. + return; } -}; - -/** - * Attempts to handle a keyboard event; returns true if the event was handled, - * false otherwise. - * @param {!Event} e Key event to handle. - * @return {boolean} Whether the event was handled by the container (or one of - * its children). - * @private - */ -Blockly.Menu.prototype.handleKeyEventInternal_ = function(e) { - // Do not handle the key event if any modifier key is pressed. if (e.shiftKey || e.ctrlKey || e.metaKey || e.altKey) { - return false; + // Do not handle the key event if any modifier key is pressed. + return; } var highlighted = this.highlightedItem_; @@ -410,11 +413,11 @@ Blockly.Menu.prototype.handleKeyEventInternal_ = function(e) { break; case Blockly.utils.KeyCodes.UP: - this.highlightPrevious_(); + this.highlightPrevious(); break; case Blockly.utils.KeyCodes.DOWN: - this.highlightNext_(); + this.highlightNext(); break; case Blockly.utils.KeyCodes.PAGE_UP: @@ -428,8 +431,10 @@ Blockly.Menu.prototype.handleKeyEventInternal_ = function(e) { break; default: - return false; + // Not a key the menu is interested in. + return; } - - return true; + // The menu used this key, don't let it have secondary effects. + e.preventDefault(); + e.stopPropagation(); }; diff --git a/core/components/menu/menuitem.js b/core/components/menu/menuitem.js index 8c24889243f..62dff438c9c 100644 --- a/core/components/menu/menuitem.js +++ b/core/components/menu/menuitem.js @@ -60,6 +60,34 @@ Blockly.MenuItem = function(content, opt_value) { * @private */ this.rightToLeft_ = false; + + /** + * ARIA name for this menu. + * @type {string} + * @private + */ + this.roleName_ = ''; + + /** + * Is this menu item checkable. + * @type {boolean} + * @private + */ + this.checkable_ = false; + + /** + * Is this menu item currently checked. + * @type {boolean} + * @private + */ + this.checked_ = false; + + /** + * Bound function to call when this menu item is clicked. + * @type {Function} + * @private + */ + this.actionHandler_ = null; }; @@ -214,21 +242,6 @@ Blockly.MenuItem.prototype.isEnabled = function() { */ Blockly.MenuItem.prototype.setEnabled = function(enabled) { this.enabled_ = enabled; - - var el = this.getElement(); - if (el) { - // goog-menuitem-disabled is deprecated, use blocklyMenuItemDisabled. - // May 2020. - var name = 'blocklyMenuItemDisabled'; - var nameDep = 'goog-menuitem-disabled'; - if (enabled) { - Blockly.utils.dom.removeClass(el, name); - Blockly.utils.dom.removeClass(el, nameDep); - } else { - Blockly.utils.dom.addClass(el, name); - Blockly.utils.dom.addClass(el, nameDep); - } - } }; /** @@ -238,7 +251,7 @@ Blockly.MenuItem.prototype.setEnabled = function(enabled) { */ Blockly.MenuItem.prototype.performAction = function() { if (this.isEnabled() && this.actionHandler_) { - this.actionHandler_.call(this.actionHandlerObj_, this); + this.actionHandler_(this); } }; @@ -250,6 +263,5 @@ Blockly.MenuItem.prototype.performAction = function() { * @package */ Blockly.MenuItem.prototype.onAction = function(fn, obj) { - this.actionHandler_ = fn; - this.actionHandlerObj_ = obj; + this.actionHandler_ = fn.bind(obj); }; diff --git a/core/components/tree/basenode.js b/core/components/tree/basenode.js index 258c5b0a303..4416c6b6a62 100644 --- a/core/components/tree/basenode.js +++ b/core/components/tree/basenode.js @@ -722,16 +722,10 @@ Blockly.tree.BaseNode.prototype.onKeyDown = function(e) { var handled = true; switch (e.keyCode) { case Blockly.utils.KeyCodes.RIGHT: - if (e.altKey) { // Why? - break; - } handled = this.selectChild(); break; case Blockly.utils.KeyCodes.LEFT: - if (e.altKey) { // Why? - break; - } handled = this.selectParent(); break; diff --git a/core/contextmenu.js b/core/contextmenu.js index 034a7f728f4..b8f22aeb40d 100644 --- a/core/contextmenu.js +++ b/core/contextmenu.js @@ -60,7 +60,7 @@ Blockly.ContextMenu.show = function(e, options, rtl) { Blockly.ContextMenu.position_(menu, e, rtl); // 1ms delay is required for focusing on context menus because some other // mouse event is still waiting in the queue and clears focus. - setTimeout(function() {menu.getElement().focus();}, 1); + setTimeout(function() {menu.focus();}, 1); Blockly.ContextMenu.currentBlock = null; // May be set by Blockly.Block. }; @@ -86,7 +86,7 @@ Blockly.ContextMenu.populate_ = function(options, rtl) { menu.addChild(menuItem); menuItem.setEnabled(option.enabled); if (option.enabled) { - var actionHandler = function() { + var actionHandler = function(_menuItem) { var option = this; Blockly.ContextMenu.hide(); option.callback(); @@ -128,7 +128,7 @@ Blockly.ContextMenu.position_ = function(menu, e, rtl) { // Calling menuDom.focus() has to wait until after the menu has been placed // correctly. Otherwise it will cause a page scroll to get the misplaced menu // in view. See issue #1329. - menu.getElement().focus(); + menu.focus(); }; /** From 50bbfb188ec17b65b775bab115ab19ae7c938f46 Mon Sep 17 00:00:00 2001 From: Neil Fraser Date: Wed, 6 May 2020 16:49:11 -0700 Subject: [PATCH 27/30] Fix rollName types. --- core/components/menu/menu.js | 8 +++++--- core/components/menu/menuitem.js | 8 +++++--- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/core/components/menu/menu.js b/core/components/menu/menu.js index abf58517979..4cde2739325 100644 --- a/core/components/menu/menu.js +++ b/core/components/menu/menu.js @@ -94,10 +94,10 @@ Blockly.Menu = function() { /** * ARIA name for this menu. - * @type {string} + * @type {Blockly.utils.aria.Role} * @private */ - this.roleName_ = ''; + this.roleName_ = null; }; @@ -118,7 +118,9 @@ Blockly.Menu.prototype.render = function(container) { // goog-menu is deprecated, use blocklyMenu. May 2020. element.className = 'blocklyMenu goog-menu blocklyNonSelectable'; element.tabIndex = 0; - Blockly.utils.aria.setRole(element, this.roleName_); + if (this.roleName_) { + Blockly.utils.aria.setRole(element, this.roleName_); + } this.element_ = element; // Add menu items. diff --git a/core/components/menu/menuitem.js b/core/components/menu/menuitem.js index 62dff438c9c..02293fd0256 100644 --- a/core/components/menu/menuitem.js +++ b/core/components/menu/menuitem.js @@ -63,10 +63,10 @@ Blockly.MenuItem = function(content, opt_value) { /** * ARIA name for this menu. - * @type {string} + * @type {Blockly.utils.aria.Role} * @private */ - this.roleName_ = ''; + this.roleName_ = null; /** * Is this menu item checkable. @@ -121,7 +121,9 @@ Blockly.MenuItem.prototype.createDom = function() { element.appendChild(content); // Initialize ARIA role and state. - Blockly.utils.aria.setRole(element, this.roleName_); + if (this.roleName_) { + Blockly.utils.aria.setRole(element, this.roleName_); + } Blockly.utils.aria.setState(element, Blockly.utils.aria.State.SELECTED, (this.checkable_ && this.checked_) || false); From dd441ca006deeaefede6381f0b50290ee93470d0 Mon Sep 17 00:00:00 2001 From: Neil Fraser Date: Wed, 6 May 2020 17:52:15 -0700 Subject: [PATCH 28/30] Remove property on DOM element linking to JS obj MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Performance is slower (O(n) rather than (O(1)), but ’n’ is the number of entries on the menu, so shouldn’t be more than a dozen or so. --- core/components/menu/menu.js | 20 +++++++++++++++----- core/components/menu/menuitem.js | 8 ++------ 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/core/components/menu/menu.js b/core/components/menu/menu.js index 4cde2739325..d4f6aa48c5a 100644 --- a/core/components/menu/menu.js +++ b/core/components/menu/menu.js @@ -94,7 +94,7 @@ Blockly.Menu = function() { /** * ARIA name for this menu. - * @type {Blockly.utils.aria.Role} + * @type {?Blockly.utils.aria.Role} * @private */ this.roleName_ = null; @@ -215,6 +215,7 @@ Blockly.Menu.prototype.dispose = function() { for (var i = 0, menuItem; (menuItem = this.menuItems_[i]); i++) { menuItem.dispose(); } + this.menuItems_length = 0; this.element_ = null; }; @@ -228,10 +229,19 @@ Blockly.Menu.prototype.dispose = function() { * @private */ Blockly.Menu.prototype.getMenuItem_ = function(node) { - var elem = this.getElement(); - while (node && node !== elem) { - if (node.blocklyMenuItem) { - return node.blocklyMenuItem; + var menuElem = this.getElement(); + // Node might be the menu border (resulting in no associated menu item), or + // a menu item's div, or some node within the menu item. + // Walk up parents until one meets either the menu's root element, or + // a menu item's div. + while (node && node != menuElem) { + if (Blockly.utils.dom.hasClass(node, 'blocklyMenuItem')) { + // Having found a menu item's div, locate that menu item in this menu. + for (var i = 0, menuItem; (menuItem = this.menuItems_[i]); i++) { + if (menuItem.getElement() == node) { + return menuItem; + } + } } node = node.parentNode; } diff --git a/core/components/menu/menuitem.js b/core/components/menu/menuitem.js index 02293fd0256..778c78fdee0 100644 --- a/core/components/menu/menuitem.js +++ b/core/components/menu/menuitem.js @@ -63,7 +63,7 @@ Blockly.MenuItem = function(content, opt_value) { /** * ARIA name for this menu. - * @type {Blockly.utils.aria.Role} + * @type {?Blockly.utils.aria.Role} * @private */ this.roleName_ = null; @@ -98,7 +98,6 @@ Blockly.MenuItem = function(content, opt_value) { Blockly.MenuItem.prototype.createDom = function() { var element = document.createElement('div'); element.id = Blockly.utils.IdGenerator.getNextUniqueId(); - element.blocklyMenuItem = this; // Link DOM back to this data structure. this.element_ = element; // Set class and style @@ -134,10 +133,7 @@ Blockly.MenuItem.prototype.createDom = function() { * Dispose of this menu item. */ Blockly.MenuItem.prototype.dispose = function() { - if (this.element_) { - this.element_.blocklyMenuItem = null; - this.element_ = null; - } + this.element_ = null; }; /** From 2e8ef42616d9c32909e304f8b0ddbe9749f8d603 Mon Sep 17 00:00:00 2001 From: Neil Fraser Date: Wed, 6 May 2020 18:16:30 -0700 Subject: [PATCH 29/30] Event.target is never a Node. Fix types. Fixes a compile error (node != element). --- core/components/menu/menu.js | 34 ++++++++++++++++------------------ 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/core/components/menu/menu.js b/core/components/menu/menu.js index d4f6aa48c5a..3b4e51ca8bf 100644 --- a/core/components/menu/menu.js +++ b/core/components/menu/menu.js @@ -222,28 +222,28 @@ Blockly.Menu.prototype.dispose = function() { // Child component management. /** - * Returns the child menu item that owns the given DOM node, or null if no such - * menu item is found. - * @param {Node} node DOM node whose owner is to be returned. - * @return {?Blockly.MenuItem} Menu item for which the DOM node belongs to. + * Returns the child menu item that owns the given DOM element, + * or null if no such menu item is found. + * @param {Element} elem DOM element whose owner is to be returned. + * @return {?Blockly.MenuItem} Menu item for which the DOM element belongs to. * @private */ -Blockly.Menu.prototype.getMenuItem_ = function(node) { +Blockly.Menu.prototype.getMenuItem_ = function(elem) { var menuElem = this.getElement(); // Node might be the menu border (resulting in no associated menu item), or - // a menu item's div, or some node within the menu item. + // a menu item's div, or some element within the menu item. // Walk up parents until one meets either the menu's root element, or // a menu item's div. - while (node && node != menuElem) { - if (Blockly.utils.dom.hasClass(node, 'blocklyMenuItem')) { + while (elem && elem != menuElem) { + if (Blockly.utils.dom.hasClass(elem, 'blocklyMenuItem')) { // Having found a menu item's div, locate that menu item in this menu. for (var i = 0, menuItem; (menuItem = this.menuItems_[i]); i++) { - if (menuItem.getElement() == node) { + if (menuItem.getElement() == elem) { return menuItem; } } } - node = node.parentNode; + elem = elem.parentNode; } return null; }; @@ -330,13 +330,12 @@ Blockly.Menu.prototype.highlightHelper_ = function(startIndex, delta) { // Mouse events. /** - * Handles mouseover events. Highlight menuitems as the user - * hovers over them. - * @param {Event} e Mouse event to handle. + * Handles mouseover events. Highlight menuitems as the user hovers over them. + * @param {!Event} e Mouse event to handle. * @private */ Blockly.Menu.prototype.handleMouseOver_ = function(e) { - var menuItem = this.getMenuItem_(/** @type {Node} */ (e.target)); + var menuItem = this.getMenuItem_(/** @type {Element} */ (e.target)); if (menuItem) { if (menuItem.isEnabled()) { @@ -350,9 +349,8 @@ Blockly.Menu.prototype.handleMouseOver_ = function(e) { }; /** - * Handles click events. Pass the event onto the child - * menuitem to handle. - * @param {Event} e Click to handle. + * Handles click events. Pass the event onto the child menuitem to handle. + * @param {!Event} e Click event to handle. * @private */ Blockly.Menu.prototype.handleClick_ = function(e) { @@ -370,7 +368,7 @@ Blockly.Menu.prototype.handleClick_ = function(e) { } } - var menuItem = this.getMenuItem_(/** @type {Node} */ (e.target)); + var menuItem = this.getMenuItem_(/** @type {Element} */ (e.target)); if (menuItem) { menuItem.performAction(); } From ef59a9a5510d38482e5427430bfc1aa8c5b4e705 Mon Sep 17 00:00:00 2001 From: Neil Fraser Date: Wed, 6 May 2020 19:41:37 -0700 Subject: [PATCH 30/30] Fixes a compile error (node != element) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Usually we avoid parentElement in Blockly. That’s because it has very spotty behaviour with SVG. But in this case we are in pure HTML. --- core/components/menu/menu.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/components/menu/menu.js b/core/components/menu/menu.js index 3b4e51ca8bf..75ab6afae7a 100644 --- a/core/components/menu/menu.js +++ b/core/components/menu/menu.js @@ -243,7 +243,7 @@ Blockly.Menu.prototype.getMenuItem_ = function(elem) { } } } - elem = elem.parentNode; + elem = elem.parentElement; } return null; };