Problem/Motivation

Modal dialogs does not good on small screens. They have different behaviors depending where they are, but some problems we haver are:

* On *admin/structure/block*, modal appears on edit disabled blocks. The width of the modal window is fixed by JS, so on smaller windows we lost a part of the modal.
* On *admin/structure/views/view/content*. The width on the modal window is fixed to 50%, and the left positions i calculated on resize. Maybe we could use this margin for the modal itself.
Views modal
Views modal

Proposed resolution

Modal dialog should have a common behavior on all its appearances in mobile devices / small screens.

CommentFileSizeAuthor
#21 2225349-modal-style-responsive-21.patch915 byteslauriii
#19 Screen Shot 2014-10-03 at 12.04.40.png197.41 KBlauriii
#19 Screen Shot 2014-10-03 at 12.03.57.png206.89 KBlauriii
#19 Screen Shot 2014-10-03 at 12.02.57.png159.67 KBlauriii
#18 interdiff-2225349-12-18.txt767 bytesg.i.joe
#18 2225349-modal-style-responsive-18.patch930 bytesg.i.joe
#12 after.png74.57 KBmerilainen
#12 2225349-modal-style-responsive-12.patch886 bytesmerilainen
#8 2225349-modal-style-responsive-8.patch402 bytesckrina
#4 2113911-modal-style-22.patch33.93 KBckrina
#4 2225349-modal-style-responsive-4.patch472 bytesckrina
views-modal.png70.79 KBckrina
block-modal.png77.57 KBckrina

Comments

ckrina’s picture

ckrina
she/her
Primary language Catalan
Location Barcelona
commented
Issue summary: View changes
sqndr’s picture

sqndr commented

The screenshots are taken with the patch from #2113911 applied to it. Just a notice. Problem also occurs without the patch.

sqndr’s picture

sqndr commented
Related issues: +#2113911: Modal style update
ckrina’s picture

ckrina
she/her
Primary language Catalan
Location Barcelona
commented
StatusFileSize
new 2225349-modal-style-responsive-4.patch472 bytes
new 2113911-modal-style-22.patch33.93 KB

Added a patch to solve that problem that will apply on all .ui-dialog adding max-width and min-width on small devices:

.ui-dialog {
+    max-width: 92%;
+    min-width: 92%;
+  }

Important: This patch needs to be applied after the parent issue #2113911 patch 2113911-modal-style-22.patch because the file core/misc/dialog.theme.css is moved to core/themes/seven/dialog.theme.css.

ckrina’s picture

ckrina
she/her
Primary language Catalan
Location Barcelona
commented
ckrina’s picture

ckrina
she/her
Primary language Catalan
Location Barcelona
commented
Status: Active » Needs review

Ready for some test, updating status.

The last submitted patch, 4: 2225349-modal-style-responsive-4.patch, failed testing.

ckrina’s picture

ckrina
she/her
Primary language Catalan
Location Barcelona
commented
StatusFileSize
new 2225349-modal-style-responsive-8.patch402 bytes

Removed min-width because I don't see it necessary, and adapted the patch to the current d8 branch to pass the test.

Bojhan’s picture

Bojhan commented
Issue tags: +frontend

Is there any reason there is soo much space on the left/right?

Bojhan’s picture

Bojhan commented
Status: Needs review » Needs work
Bojhan’s picture

Bojhan commented
Issue tags: +Novice
merilainen’s picture

merilainen commented
Status: Needs work » Needs review
StatusFileSize
new 2225349-modal-style-responsive-12.patch886 bytes
new after.png74.57 KB

Before you bash me completely for using !important, let me explain myself.
The popup gets a default width: 700 from core/modules/block/src/BlockListBuilder.php and core/modules/config/src/Form/ConfigSys.php, which makes it impossible to change it in css without !important. Removing those default values in mentioned files leads to the dialog getting a default width: 300 from core/assets/vendor/jquery.ui/ui/jquery.ui.dialog.js, which I presume is not to be altered. Using width:auto or width: "92%" in the php files means that the dialog gets very big in wider displays. Hence the !important, yuck.
Attached after.png.

LewisNyman queued 12: 2225349-modal-style-responsive-12.patch for re-testing.

lewisnyman’s picture

lewisnyman
he/him
commented
Issue tags: +Needs reroll

The Seven CSS files have moved. I'm happy with using the !important in this situation. jQuery UI styling makes me sad :(

Status: Needs review » Needs work

The last submitted patch, 12: 2225349-modal-style-responsive-12.patch, failed testing.

sqndr’s picture

sqndr commented 
diff --git a/core/misc/dialog.theme.css b/core/misc/dialog.theme.css
...
+@media all and (max-width: 48em) { /* 768px */
+  .ui-dialog {
+    width: 92% !important;
+  }
...
diff --git a/core/themes/seven/css/dialog.theme.css b/core/themes/seven/css/dialog.theme.css
...
+@media all and (max-width: 48em) { /* 768px */
+  .ui-dialog {
+    width: 92% !important;
+  }

Is there a reason why we are doing the same thing twice?

Update Oh, I see. The stylesheet gets overridden in Seven.

stylesheets-override:
  - css/components/vertical-tabs.css
  - css/components/jquery.ui.theme.css
  - css/components/dialog.theme.css
g.i.joe’s picture

g.i.joe commented

I will look into this issue.

g.i.joe’s picture

g.i.joe commented
Status: Needs work » Needs review
Issue tags: -Needs reroll
StatusFileSize
new 2225349-modal-style-responsive-18.patch930 bytes
new interdiff-2225349-12-18.txt767 bytes

I sat next to sqndr @drupalcon Amsterdam. Where I rerolled this patch.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new Screen Shot 2014-10-03 at 12.02.57.png159.67 KB
new Screen Shot 2014-10-03 at 12.03.57.png206.89 KB
new Screen Shot 2014-10-03 at 12.04.40.png197.41 KB

Thanks @G.I.Joe for the patch!

Seven:

Bartik:

Stark/Classy:

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs reroll 
git ac https://www.drupal.org/files/issues/2225349-modal-style-responsive-18.patch
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100   930  100   930    0     0   2885      0 --:--:-- --:--:-- --:--:--  4818
error: patch failed: core/themes/seven/css/components/dialog.theme.css:10
error: core/themes/seven/css/components/dialog.theme.css: patch does not apply
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Needs work » Reviewed & tested by the community
Issue tags: -Needs reroll
StatusFileSize
new 2225349-modal-style-responsive-21.patch915 bytes
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Committed 5ed7af8 and pushed to 8.0.x. Thanks!

The !important makes me sad.

  • alexpott committed 5ed7af8 on 8.0.x 
    Issue #2225349 by ckrina, G.I.Joe, lauriii, mErilainen: Responsive...

Status: Fixed » Closed (fixed)

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