Modal style update

The views dialog needs to be mocked up, there are a lot of things going on in them.

Rolled #2173129: Modal dialog style update in this one. scott.gonzalez needs to be given commit credit.

Why did we close that other issue in favor of this one? Much more activity in the other one…

The activity is Bojhan, you and me. doesn't matter which one we go with, this one is older and 6 people are following it, 4 on the other.

Same patch but with the moved files obvious.

When switching from the comment body field for ie an article

• the ugly dialog appears
• a red artifact when switching to 'Restricted HTML'

As #2166399: Changing CKEditor configurations by changing text formats causes markup to be lost: warn the user when performing this advanced action is closed and this is mere styling let's fix it here?

This is a reroll because the indentation standard changed for JavaScript files. No need for commit credit because of this reroll.

This patch corrects the path to dialog.ajax.js and moves the Seven stylesheet so it's always included; it doesn't seem to work in the stylesheets-override section as there isn't a base dialog.theme.css to override any more.

ThemeHandlerTest explictly looks for Seven's CSS. Still not sure this is the right approach to getting dialog.theme.css loaded.

Reduced modal window title padding from 20px to 15px after discussing with @lewisnyman. Now it looks better (see screenshots).

Before

After

Sorry for the junk in previous patch. I have to configure git for deleted files :(

Patch seems to work. Been testing this a lot now. Looks fine to me. Hope someone else could confirm and we can get this RTBC!

Looks good to me!

Patch applies fine, but maybe it could need some work in responsive for small screens. I create a new issue for that: https://drupal.org/node/2225349

It works good to me!

As a developer I can't really tell a difference between the OP and later screenshots, but yay for better looking thingies. :)

Committed and pushed to 8.x. Thanks!

@webchick Thats because the before picture is wrong, hah :)

Tempted to reopen, see attached files.

Contrast too low on modal header.
Modals look like crap on bartik.

Sorry, friends, I have to re-open this...

1. --- a/core/misc/dialog.theme.css
   

   Why was the default dialog.theme.css implementation removed here?

   Dialogs appear even more distorted than previously in Stark now:

   

   They had way too much chrome in the past already, but now their styling is completely off the mark.

2. +++ b/core/themes/seven/dialog.theme.css
   

   Seven has a ./css folder — why are we not using it?

3. +++ b/core/themes/seven/seven.info.yml
   @@ -10,6 +10,7 @@ stylesheets:
   +    - dialog.theme.css
   

   Why are we loading the dialog styles at all times?

   If the library's default dialog.theme.css asset file wouldn't have been removed, then this would be a simple stylesheets-override and it would only be loaded when needed.

Ok, since it sounds like this still needs work/discussion, rolled this back for now.

Nice, seems like this fixed the problem. Been testing on Simplytest and it seems to work fine with all the themes, just like on the screenshots you've provided.

Looks good to me too.

Updating patch with reroll, please review.

Yeah, looks like you straight forgot to add 3 JS files, and 1 CSS file into your last patch, comment #40 patch was over 30KB, your last is only 16 KB and in the console are messages:

GET http://s524bf474844c1cc.s2.simplytest.me/core/themes/seven/css/dialog.theme.css?n3nmhw 404 (Not Found) page:53
GET http://s524bf474844c1cc.s2.simplytest.me/core/misc/dialog/dialog.js?v=8.0-dev 404 (Not Found) VM617 page:99
GET http://s524bf474844c1cc.s2.simplytest.me/core/misc/dialog/dialog.position.js?v=8.0-dev 404 (Not Found) VM617 page:100
GET http://s524bf474844c1cc.s2.simplytest.me/core/misc/dialog/dialog.jquery-ui.js?v=8.0-dev 404 (Not Found) 

Please re-roll ;)

;) updating the patch with required files.

rerolling...

Stat and bartik looks good.
Seven looks good at views.
The modal box looks bad at add block modal window, and the add image modal on node create form.

The views add padding to the form element:

 (views_ui.admin.theme.css:864)
.views-ui-dialog .views-override {
padding: 8px 13px;
}

But the node form doesn't.

In the views modal, there's an overflow: hidden; inline value on the #drupal-modal. Maybe we need to add this from css.

If the dialog is too small, i can't scroll inside it. We could add overflow:scroll; instead of overflow: hidden;.

Here attached rerolled patch and applying the overflow and padding to fix the issues in #53.
Cheers!

Here is a patch on #56 with the removed padding.

OK, that padding was needed for the dialog somewhere else, so here is a patch with it, undoing it for views modal dialogs.

The interdiff is done against #56.

Updating the failing test (path change).

Wonder if @sun could give it another last review. I dont want to roll it back again.

64: drupal8-modal_style_update-2113911-64.patch queued for re-testing.

Looks like DialogTest.php moved?

patch reroll to the working state

changing status to needs review

Looks like something is wrong with the bottom of a view dialog.

This wasn't there in the previous patch, did something change? The white space just above the button

Alright, I didn't spot anything else. So I am going to mark this RTBC. It looks like @sun doesn't have the time to review this atm, so I think we can just put it in core.

Committed 22b7b7e and pushed to 8.x. Thanks!

diff --git a/core/modules/ckeditor/js/ckeditor.admin.js b/core/modules/ckeditor/js/ckeditor.admin.js
index 524d429..4136424 100644
--- a/core/modules/ckeditor/js/ckeditor.admin.js
+++ b/core/modules/ckeditor/js/ckeditor.admin.js
@@ -1361,7 +1361,7 @@
           text: Drupal.t('Cancel'),
           click: function () {
             closeDialog('cancel');
-          },
+          }
         }
       ],
       open: function () {

This broke our eslint rules :) - fixed during commit.

core/modules/ckeditor/js/ckeditor.admin.js
  1364:11  error  Trailing comma  no-comma-dangle