In #1821548: Add a "diff" of some kind to the CMI UI, we're opening a dialog with a hard-coded width of 700px. That's not very mobile friendly. Instead of doing that, let's define some dialog classes (e.g., small, medium, large) and figure out good CSS for them. Any volunteers?

Files: 
CommentFileSizeAuthor
#10 dialog-width-responsive-1918744-10.patch3.51 KBdealancer
PASSED: [[SimpleTest]]: [MySQL] 55,327 pass(es).
[ View ]
#9 dialog-width-responsive-1918744-9.patch0 bytesdealancer
PASSED: [[SimpleTest]]: [MySQL] 55,576 pass(es).
[ View ]
#5 dialog-width-responsive-1918744-5.patch2.77 KBdealancer
FAILED: [[SimpleTest]]: [MySQL] 55,373 pass(es), 4 fail(s), and 0 exception(s).
[ View ]

Comments

Issue tags:+dialog

and another tag

Issue tags:-dialog

I'm not sure this should happen. Instead each modal should be given a specific class and all the sizing should be done via CSS (and media queries within that CSS). #1851414: Convert Views to use the abstracted dialog modal introduces a dialog.theme.css file, I think it would make sense to simply specify a default width in there and try to avoid any manual setting of width at all.

Issue tags:+dialog

Right now default width is 300px set by jQueryUI. However it would be nice to have different default widths for different screen widths/devices (e.g. mobile, desktop). This should be done using @media in css.

StatusFileSize
new2.77 KB
FAILED: [[SimpleTest]]: [MySQL] 55,373 pass(es), 4 fail(s), and 0 exception(s).
[ View ]

Ok, so here are couple of concerns when passing width directly into dialog:

A. It is hard to adopt such dialog to the smaller screen.
B. Dialog is not centered when screen is resized.

So I almost agree with @effulgentsia re introducing classes, but I also like idea of @quicksketch to use default width and be able to override it. Also I want to make dialog responsive. Here is a patch that can do all of this.

Usage examples:

    $response->addCommand(new OpenModalDialogCommand($title, $dialog_content)); // 700 px, responsive;
    $response->addCommand(new OpenModalDialogCommand($title, $dialog_content, array('dialogClass' => 'wide'))); // 940 px, responsive
    $response->addCommand(new OpenModalDialogCommand($title, $dialog_content, array('dialogClass' => 'narrow'))); // 460 px, responsive
    $response->addCommand(new OpenModalDialogCommand($title, $dialog_content, array('width' => '555px'))); // overriden

Status:Active» Needs review

Status:Needs review» Needs work
Issue tags:-mobile, -dialog

The last submitted patch, dialog-width-responsive-1918744-5.patch, failed testing.

Status:Needs work» Needs review
Issue tags:+mobile, +dialog

StatusFileSize
new0 bytes
PASSED: [[SimpleTest]]: [MySQL] 55,576 pass(es).
[ View ]

Ok, looks like we need to fix a test.

Here is re-rolled patch!

StatusFileSize
new3.51 KB
PASSED: [[SimpleTest]]: [MySQL] 55,327 pass(es).
[ View ]

Last one was empty.

This patch contains updated test.

Status:Needs review» Needs work

Everything looks ok,
Can you please just change px to % ?

I'm not a front-end developer, so don't have an opinion on #11, but otherwise, I think #10 looks good. I'm curious what @quicksketch thinks though, given #2.

Status:Needs work» Needs review

Re #11, also can't find use cases when we need this option.

We would rather have strictly defined width so the appearance of dialog box is predictable. When it starches to much the content can "swim" to the borders of the monitor. Using media queries we can easily control width of the dialog box for smaller device.

We would like to have more reviews and opinions. So I am settings status back to needs review, otherwise this ticket can be ignored.

Status:Needs review» Needs work

The patch does not apply to me. When trying to apply I get these errors:

error: core/modules/config/config.admin.inc: No such file or directory
error: patch failed: core/modules/system/lib/Drupal/system/Tests/Ajax/DialogTest.php:37
error: core/modules/system/lib/Drupal/system/Tests/Ajax/DialogTest.php: patch does not apply
error: patch failed: core/modules/system/system.module:1317
error: core/modules/system/system.module: patch does not apply

Looks as in the meantime filesystem as changed, core/modules/config/config.admin.inc doesn't exist anymore.

I think we should be writing the widths in percentages, all our core CSS is.

Looking at the initial problem, it seems that we only need one fluid size in order to make it mobile friendly. Why are we adding different sizes?

Status:Needs work» Closed (won't fix)

FYI in Editor module we just used CSS ultimately:

/**
* @file
* Styles for text editors.
*/
.editor-dialog {
  /* This !important is required to override inline CSS of jQuery UI. */
  width: 80% !important;
  max-width: 500px;
}
@media screen and (max-width: 600px) {
  .editor-dialog {
    width: 95% !important;
  }

The unfortunate thing however is that jQuery UI loads FIRST on pages where Edit module is present, since it depends on jQuery UI at page load time and editor.css is loaded on-the-fly after the page load. So we ended up with !important in there until we fix that issue or (better yet) just stop loading the default CSS for jQuery UI (which we override everywhere anyway already).

So I think this is a won't fix. Just use CSS to set dialog widths.

Issue summary:View changes

.