From 3eb5c95478052d73dc1b62f441445ad2948df27c Mon Sep 17 00:00:00 2001
From: Rachel Fenichel <fenichel@google.com>
Date: Fri, 5 Nov 2021 12:07:38 -0700
Subject: [PATCH] fix: bad order of checks for setting flyout visibility
 (#5681)

* chore: fix redeclares in tests

* fix: bad order of checks for setting flyout visibility
---
 core/toolbox/toolbox.js              | 2 +-
 tests/mocha/insertion_marker_test.js | 4 ++--
 tests/mocha/toolbox_test.js          | 5 ++++-
 3 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/core/toolbox/toolbox.js b/core/toolbox/toolbox.js
index 745cac6de..438a3b4a3 100644
--- a/core/toolbox/toolbox.js
+++ b/core/toolbox/toolbox.js
@@ -968,7 +968,7 @@ Toolbox.prototype.selectItemByPosition = function(position) {
  * @protected
  */
 Toolbox.prototype.updateFlyout_ = function(oldItem, newItem) {
-  if ((oldItem === newItem && !newItem.isCollapsible()) || !newItem ||
+  if (!newItem || (oldItem === newItem && !newItem.isCollapsible()) ||
       !newItem.getContents().length) {
     this.flyout_.hide();
   } else {
diff --git a/tests/mocha/insertion_marker_test.js b/tests/mocha/insertion_marker_test.js
index f825afb1f..66b85ba90 100644
--- a/tests/mocha/insertion_marker_test.js
+++ b/tests/mocha/insertion_marker_test.js
@@ -204,8 +204,8 @@ suite('InsertionMarkers', function() {
   });
   suite('Serialization', function() {
     setup(function() {
-      this.assertXml = function(xml, expectXml) {
-        Blockly.Xml.domToWorkspace(xml, this.workspace);
+      this.assertXml = function(xmlIn, expectXml) {
+        Blockly.Xml.domToWorkspace(xmlIn, this.workspace);
         let block = this.workspace.getBlockById('insertion');
         block.setInsertionMarker(true);
         let xml = Blockly.Xml.workspaceToDom(this.workspace);
diff --git a/tests/mocha/toolbox_test.js b/tests/mocha/toolbox_test.js
index e205e8079..dfabe5cbe 100644
--- a/tests/mocha/toolbox_test.js
+++ b/tests/mocha/toolbox_test.js
@@ -407,7 +407,6 @@ suite('Toolbox', function() {
 
     function testHideFlyout(toolbox, oldItem, newItem) {
       let updateFlyoutStub = sinon.stub(toolbox.flyout_, 'hide');
-      const newItem = getNonCollapsibleItem(toolbox);
       toolbox.updateFlyout_(oldItem, newItem);
       sinon.assert.called(updateFlyoutStub);
     }
@@ -419,6 +418,10 @@ suite('Toolbox', function() {
     test('No new item -> Should close flyout', function() {
       testHideFlyout(this.toolbox, null, null);
     });
+    test('Old item but no new item -> Should close flyout', function() {
+      let oldItem = getNonCollapsibleItem(this.toolbox);
+      testHideFlyout(this.toolbox, oldItem, null);
+    });
     test('Select collapsible item -> Should close flyout', function() {
       let newItem = getCollapsibleItem(this.toolbox);
       testHideFlyout(this.toolbox, null, newItem);