Method resetSize() in dialog.position.js is responsible to reposition and resize jQuery UI dialog on dialog content loading and later on screen resize events, etc.

The problem is that some dialog implementations (like entity_browser) are only setting maxHeight parameter and not the height parameter, and in that combination resetSize() method with fail to set maxHeight on dialog because of missing height parameter.

If we take a look at jQuery UI dialog source _size() method on line 792 we will see that if height option is not set to auto then min/maxHeight options will be ignored.

Comments

pivica created an issue. See original summary.

pivica’s picture

Attaching a patch.

pivica’s picture

Relative #2692845-4: Better height CSS rules for small screens issue comment that is showing how this patch improves entity_browser dialog for smaller screens.

nod_’s picture

Status: Needs review » Needs work

Reasoning looks good. I'd go for something a little bit shorter in the code:

    if (!adjustedOptions.height && (adjustedOptions.minHeight || adjustedOptions.maxHeight)) {
      adjustedOptions.height = 'auto';
    }

We want to know if there is a value, not that the value has been set on this object and not through inheritance.

pivica’s picture

Version: 8.0.x-dev » 8.1.x-dev
Status: Needs work » Needs review
FileSize
777 bytes
860 bytes

Yeah this makes perfect sense.

Here is a new patch, fully tested in Chrome, works fine.

droplet’s picture

Am I missing something.. `height` is default to `auto` already ( so that may changed from custom script ? )

http://api.jqueryui.com/dialog/#option-height

pivica’s picture

Status: Needs review » Needs work

Hmm good point, but we definitely do not have height in #2692845: Better height CSS rules for small screens. Will try to find time and check why it is missing.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

slashrsm’s picture

Issue tags: +D8Media

This issues blocks us in the Media initiative. Tagging accordingly.

droplet’s picture

Issue tags: +Needs JS testing
Berdir’s picture

Reroll.

Berdir’s picture

And a patch that should cleanly apply to 8.2.

Edit: The patch is actually the same, I was confused.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.