Problem/Motivation

In jQuery 3.0 the behavior of .outerHeight() has changed to return NaN when an element is not found.

Proposed resolution

If an element is not returned from the selector, use a 0 value.

Files: 
CommentFileSizeAuthor
toolbar.patch1.75 KBdroplet

Comments

droplet created an issue. See original summary.

drpal’s picture

@droplet 👍 This is the same conclusion I just realized.

When closing the toolbar, $('.is-active.toolbar-tray-horizontal') returns nothing, thus .outerHeight() returns NaN breaking the height set.

drpal’s picture

Issue summary: View changes
drpal’s picture

Title: Improve Toolbar Height calculation » Improve Toolbar Height calculation to accommodate jQuery 3.0 changes.
drpal’s picture

Status: Needs review » Reviewed & tested by the community

🚀

Wim Leers’s picture

lgtm!

martin107’s picture

looking at the bottom set of changes..

-      this.model.set('height', $('#toolbar-bar').find('.toolbar-tab').outerHeight() + $('.is-active.toolbar-tray-horizontal').outerHeight());
+      var toolbarTabOuterHeight = $('#toolbar-bar').find('.toolbar-tab').outerHeight() || 0;
+      var toolbarTrayHorizontalOuterHeight = $('.is-active.toolbar-tray-horizontal').outerHeight() || 0;
+      this.model.set('height', toolbarTabOuterHeight + toolbarTrayHorizontalOuterHeight);
 

toolbarTabOuterHeight and toolbarTrayHorizontalOuterHeight are set, not modified and then read.

so like the top set of changes should be defined as const not var.

drpal’s picture

@martin107

The bottom set of changes, from ToolbarVisualView.js, contain the transpiled JavaScript.

martin107’s picture

Ah crap, you are right my mistake... sorry for the noise.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.4.x. Thanks!

  • webchick committed 478ddd3 on 8.4.x
    Issue #2894283 by droplet, drpal: Improve Toolbar Height calculation to...