Problem/Motivation

Code that is calculating modal height in entity_browser.modal.js is

// If no maxHeight is defined, make it responsive.
dialog.option('height', vHeight - 100);

// Because there is no iframe height 100% in HTML 5, we have to set
// the height of the iframe as well.
var contentHeight = $this.find('.ui-dialog-content').height() - 20;
$this.find('iframe').css('height', contentHeight);

This works fine for bigger screens but will create more waste space on smaller screens like mobile:

Proposed resolution

We can try to improve this by removing fixed -100 value and using percentage there and also try to remove -20 value later.

CommentFileSizeAuthor
#9 iphone tested on chrome with change.png24.93 KBoknate
#9 iphone tested on chrome without change.png27.02 KBoknate
#7 entity-browser-modal-improve-height-2993139-7.patch1.89 KBprimsi
#7 entity-browser-modal-improve-height-2993139-7.interdiff.txt823 bytesprimsi
#2 entity-browser-modal-improve-height-2993139-2.patch1.43 KBpivica
#2 Screen Shot 2018-08-16 at 5.01.53 PM.png216.73 KBpivica
Screen Shot 2018-08-16 at 5.18.39 PM.png112.32 KBpivica

Comments

pivica created an issue. See original summary.

pivica’s picture

pivica commented
Status: Active » Needs review
StatusFileSize
new Screen Shot 2018-08-16 at 5.01.53 PM.png216.73 KB
new entity-browser-modal-improve-height-2993139-2.patch1.43 KB

Here is a first patch, and here is how it looks for small screens:

So we got a bit more height here for our mobile.

I needed to add a small amount of CSS because '-20' is removed, without it, that wrapper div is getting more height for some reason and triggering then vertical scrollbar on a parent.

Height is now 92% of screen height but IMHO i don't see why we would not go maybe even to 99% or 100%, on small screens at least but maybe even on bigger?

pivica’s picture

pivica commented

Btw, CSS change looks similar to https://www.drupal.org/project/entity_browser/issues/2692845#comment-109....

primsi’s picture

primsi commented
Status: Needs review » Needs work

Just a question:

+++ b/css/entity_browser.modal.css
@@ -0,0 +1,9 @@
+.ui-dialog-content > div {

Isn't this very broad and could target other jQuery ui dialogs too?

pivica’s picture

pivica commented

> Isn't this very broad and could target other jQuery ui dialogs too?

Hmm yes that is correct, did a long check on this but i cannot figure how to do it differently.

If we look at a HTML structure of jquery dialog we have

div.ui-dialog ui-corner-all ui-widget ui-widget-content ui-front ui-draggable
  div ui-dialog-titlebar ui-corner-all ui-widget-header ui-helper-clearfix ui-draggable-handle
    div.ui-front ui-dialog-content ui-widget-content
      div
        iframe

So there is really no good CSS class to apply that max-height: 100% rule.

We could maybe inject some CSS class in entity_browser.modal_selection.js selection, but that sounds like a maybe bad approach.

Is there some other smarter option to inject CSS in PHP?

primsi’s picture

primsi commented

Seems like we can add classes to specific Dialog components like this for example:

    $response->addCommand(new OpenDialogCommand('#' . Html::getUniqueId($field_name . '-' . $element_name . '-dialog'), $this->configuration['link_text'], $html, [
      'width' => 'auto',
      'height' => 'auto',
      'modal' => TRUE,
      'maxWidth' => $this->configuration['width'],
      'maxHeight' => $this->configuration['height'],
      'fluid' => 1,
      'autoResize' => 0,
      'resizable' => 0,
      'classes' => ['ui-dialog' => 'entity-browser-modal']
    ]));

This is in \Drupal\entity_browser\Plugin\EntityBrowser\Display\Modal::openModal.

The options are defined here: http://api.jqueryui.com/dialog/#option-classes

primsi’s picture

primsi commented
StatusFileSize
new entity-browser-modal-improve-height-2993139-7.interdiff.txt823 bytes
new entity-browser-modal-improve-height-2993139-7.patch1.89 KB

In patch form.

primsi’s picture

primsi commented
Status: Needs work » Needs review
oknate’s picture

oknate
Location Greater New York City Area
commented
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new iphone tested on chrome without change.png27.02 KB
new iphone tested on chrome with change.png24.93 KB

Marking RTBC. This looks like an improvement and low risk. I tested on chrome, emulating iphone 6/7/8 Plus.

Without change:
without change

With change:
with change

  • oknate committed f38cb1a on 8.x-2.x 
    git commit -m 'Issue #2993139 by pivica, Primsi: Improve height...

  • oknate committed d5dc8f5 on 8.x-1.x 
    git commit -m 'Issue #2993139 by pivica, Primsi: Improve height...
oknate’s picture

oknate
Location Greater New York City Area
commented
Status: Reviewed & tested by the community » Fixed

Committed. Thanks for your contribution.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.