Problem/Motivation

Bartik modal windows do not look very "Bartik" and have some formatting issues. It would be good to spruce them up.


(Current modal style 26/01/2015)

Proposed resolution

Let's implement a design in Bartik that fits with the rest of the theme.

Remaining tasks

Come up with styling in CSS.
Get design thumbs up.

User interface changes

A nicer looking modal in Bartik.

API changes

None

Original report by @LewisNyman

In #2113911: Modal style update we removed core/misc/dialog.css Bartik now displays the default Jquery UI styling for the modal which looks ugly.

CommentFileSizeAuthor
#38 patch-review.png83.38 KBtmjoseantonio
#36 2226189after.jpeg156.68 KBmherchel
#36 2226189-style-bartik-modal-36.patch1.47 KBmherchel
#33 2226189after.jpeg167.09 KBmherchel
#33 2226189-style-bartik-modal-33.patch1.37 KBmherchel
#31 Screen Shot 2015-01-26 at 17.03.00.png146.29 KBemma.maria
#28 style_the_modal_in_bartik_ui_dialog_2226189-28.patch1.29 KBSivaji_Ganesh_Jojodae
#28 style_the_modal_in_bartik_editor_dialog-2226189-28.patch1.3 KBSivaji_Ganesh_Jojodae
#20 Create Article | Drupal 8 - Google Chrome_007.png96.34 KBSivaji_Ganesh_Jojodae
#19 style_the_modal_in_bartik-2226189-19.patch1.08 KBSivaji_Ganesh_Jojodae
#18 style_the_modal_in_bartik-2226189-18.patch1.03 KBalvar0hurtad0
#18 interdiff.txt781 bytesalvar0hurtad0
#17 style_the_modal_in_bartik-2226189-17.patch1.03 KBalvar0hurtad0
#17 interdiff.txt612 bytesalvar0hurtad0
#11 style_the_modal_in_bartik-2226189-11.patch1.03 KBalvar0hurtad0
#11 Captura de pantalla 2014-12-16 a la(s) 11.15.55.png53.18 KBalvar0hurtad0
#9 style_the_modal_in_bartik-2226189-9.patch881 bytesalvar0hurtad0
#7 Screen_Shot_2014-11-23_at_21_31_41.png301.31 KBemma.maria
#1 Screenshot 2014-03-26 21.08.04.jpg143.59 KBLewisNyman
bartik-ugly.png37.78 KBLewisNyman
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

LewisNyman’s picture

LewisNyman
he/him
CreditAttribution: LewisNyman commented
FileSize
Screenshot 2014-03-26 21.08.04.jpg143.59 KB

This is how it used to look:

ChandeepKhosa’s picture

ChandeepKhosa CreditAttribution: ChandeepKhosa commented
Assigned: Unassigned » ChandeepKhosa

I've started this and so far have some new CSS for the title section & background so far, will continue later today

LewisNyman’s picture

LewisNyman
he/him
CreditAttribution: LewisNyman commented
Status: Active » Postponed

We might not need to fix this now depending on #2113911: Modal style update

mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford commented
Status: Postponed » Active
Related issues: +#2113911: Modal style update
emma.maria’s picture

emma.maria
she/her
CreditAttribution: emma.maria commented
Assigned: ChandeepKhosa » Unassigned
emma.maria’s picture

emma.maria
she/her
CreditAttribution: emma.maria commented

Now the modal window is back to being styled with the dialog.theme.css but the modal windows still are not very "Bartik with the heading fonts and sizes, button styles etc.
I am going to update the issue summary to suggest adding Bartik styles to the modal windows.

emma.maria’s picture

emma.maria
she/her
CreditAttribution: emma.maria commented
Issue summary: View changes
Issue tags: +frontend, +CSS, +Novice
FileSize
Screen_Shot_2014-11-23_at_21_31_41.png301.31 KB
emma.maria’s picture

emma.maria
she/her
CreditAttribution: emma.maria commented
Issue summary: View changes
alvar0hurtad0’s picture

alvar0hurtad0
he/him
Primary language Spanish
Location Cáceres
CreditAttribution: alvar0hurtad0 commented
Status: Active » Needs review
FileSize
style_the_modal_in_bartik-2226189-9.patch881 bytes

Hi there,

here we've a patch

emma.maria’s picture

emma.maria
she/her
CreditAttribution: emma.maria commented
Issue tags: +Needs screenshots
alvar0hurtad0’s picture

alvar0hurtad0
he/him
Primary language Spanish
Location Cáceres
CreditAttribution: alvar0hurtad0 commented
FileSize
Captura de pantalla 2014-12-16 a la(s) 11.15.55.png53.18 KB
style_the_modal_in_bartik-2226189-11.patch1.03 KB

I'm going to do it better, I've refactor the patch (and take a bit more time to study the current CSS).

Those are the patch and the screenshot.

LewisNyman’s picture

LewisNyman
he/him
CreditAttribution: LewisNyman commented

This is looking good, but I'm worried about using the .ui-widget class, I think this is used widely by jQuery UI so it could introduce regressions all over the place. Is there a more specific class we can use for the modal?

alvar0hurtad0’s picture

alvar0hurtad0
he/him
Primary language Spanish
Location Cáceres
CreditAttribution: alvar0hurtad0 commented

For example .ui-dialog?

cosmicdreams’s picture

cosmicdreams CreditAttribution: cosmicdreams commented

Yep, .ui-widget and .ui-dialog are both defined by jquery-ui.css

LewisNyman’s picture

LewisNyman
he/him
CreditAttribution: LewisNyman commented

ui-dialog sounds safer

LewisNyman’s picture

LewisNyman
he/him
CreditAttribution: LewisNyman commented
Status: Needs review » Needs work
alvar0hurtad0’s picture

alvar0hurtad0
he/him
Primary language Spanish
Location Cáceres
CreditAttribution: alvar0hurtad0 commented
Status: Needs work » Needs review
FileSize
interdiff.txt612 bytes
style_the_modal_in_bartik-2226189-17.patch1.03 KB

ok, ui-dialog :D

alvar0hurtad0’s picture

alvar0hurtad0
he/him
Primary language Spanish
Location Cáceres
CreditAttribution: alvar0hurtad0 commented
FileSize
interdiff.txt781 bytes
style_the_modal_in_bartik-2226189-18.patch1.03 KB

Forget my last comment, sorry about not reviewing before.

Sivaji_Ganesh_Jojodae’s picture

Sivaji_Ganesh_Jojodae CreditAttribution: Sivaji_Ganesh_Jojodae commented
FileSize
style_the_modal_in_bartik-2226189-19.patch1.08 KB

This issue is a nice catch.

#18, needs work as mentioned in #14.

Yep, .ui-widget and .ui-dialog are both defined by jquery-ui.css

This is very true! I ran grep from drupal root to confirm this.

Have re-rolled patch with better class name.

Independent of theme, Align items would need better spacing.

Sivaji_Ganesh_Jojodae’s picture

Sivaji_Ganesh_Jojodae CreditAttribution: Sivaji_Ganesh_Jojodae commented
FileSize
Create Article | Drupal 8 - Google Chrome_007.png96.34 KB

screenshot

Status: Needs review » Needs work

The last submitted patch, 19: style_the_modal_in_bartik-2226189-19.patch, failed testing.

sivaji@knackforge.com queued 19: style_the_modal_in_bartik-2226189-19.patch for re-testing.

alvar0hurtad0’s picture

alvar0hurtad0
he/him
Primary language Spanish
Location Cáceres
CreditAttribution: alvar0hurtad0 commented

But, what about the other modals? for example links.

Sivaji_Ganesh_Jojodae’s picture

Sivaji_Ganesh_Jojodae CreditAttribution: Sivaji_Ganesh_Jojodae commented

Good question! I see editor-dialog class in common. Perhaps we can use the same.

LewisNyman’s picture

LewisNyman
he/him
CreditAttribution: LewisNyman commented

I would recommend not changing the styling for forms within modals, only the modal itself, and tackle that other stuff in a separate issue. We shouldn't need to add special styling to something just because it's inside a modal, it probably has the same problem outside modals as well.

Sivaji_Ganesh_Jojodae’s picture

Sivaji_Ganesh_Jojodae CreditAttribution: Sivaji_Ganesh_Jojodae commented

@LewisNyman, agreed.

Anyway the last patch in #19 tries to use different class as .ui-widget and .ui-dialog are being used in jquery ui.

LewisNyman’s picture

LewisNyman
he/him
CreditAttribution: LewisNyman commented

Oh, we can use .ui-dialog, the mark up is generated by jQuery UI so it makes sense to use it.

+++ b/core/themes/bartik/css/style.css
@@ -130,6 +130,7 @@ body,
+.editor-image-dialog,

This selector would only style the image dialog in the wysiwyg

Sivaji_Ganesh_Jojodae’s picture

Sivaji_Ganesh_Jojodae CreditAttribution: Sivaji_Ganesh_Jojodae commented
Status: Needs work » Needs review
FileSize
style_the_modal_in_bartik_editor_dialog-2226189-28.patch1.3 KB
style_the_modal_in_bartik_ui_dialog_2226189-28.patch1.29 KB

Okay, I have provided patch for both .ui-dialog and .editor-dialog.

Should I re-open #2113911: Modal style update or create a new issues for styling forms within modals?

Ckeditor's modals are still looking differently (Table for instance).

LewisNyman’s picture

LewisNyman
he/him
CreditAttribution: LewisNyman commented

Ah, if it's a CKeditor form problem then we should open new issues against that component, then we can figure out the best solution.

alvar0hurtad0’s picture

alvar0hurtad0
he/him
Primary language Spanish
Location Cáceres
CreditAttribution: alvar0hurtad0 commented

+1 :D

emma.maria’s picture

emma.maria
she/her
CreditAttribution: emma.maria commented
Issue summary: View changes
FileSize
Screen Shot 2015-01-26 at 17.03.00.png146.29 KB

I have updated the screenshot in the issue summary as the modal looks a little different in core now.

emma.maria’s picture

emma.maria
she/her
CreditAttribution: emma.maria commented
Status: Needs review » Needs work

This issue needs to follow what we are trying to achieve here #1342054: [META] Clean up templates and CSS.

All code for a particular component needs to be kept in it's own component CSS file.

Therefore all of the code for the modal styles including the font declaration in typography.css needs to put into a file called "ui-dialog" in css/components folder.

Please make a follow up patch of this work from style_the_modal_in_bartik_ui_dialog_2226189-28.patch in #28.

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
CreditAttribution: mherchel commented
Status: Needs work » Needs review
FileSize
2226189-style-bartik-modal-33.patch1.37 KB
2226189after.jpeg167.09 KB

Patch submitted. Updated screenshot below:

after

emma.maria’s picture

emma.maria
she/her
CreditAttribution: emma.maria commented

Thanks for the patch @mherchel! The CSS is in the right place now with the right component class.

I have two small requests!

Can we please add...

  1. The Bartik sans-serif font stack to the .ui-dialog please
    font-family: "Helvetica Neue", Helvetica, Arial, sans-serif;

    2.  

  2. Remove the rounded corners of the window modal also on the .ui-dialog level

 
Thanks!

emma.maria’s picture

emma.maria
she/her
CreditAttribution: emma.maria commented
Status: Needs review » Needs work
mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
CreditAttribution: mherchel commented
Status: Needs work » Needs review
FileSize
2226189-style-bartik-modal-36.patch1.47 KB
2226189after.jpeg156.68 KB

Patch and screenshot attached!

mike

emma.maria’s picture

emma.maria
she/her
CreditAttribution: emma.maria commented

I have a beginner reviewing this issue at Drupalcon Latin America today, no one else review this until I say so :)

tmjoseantonio’s picture

tmjoseantonio CreditAttribution: tmjoseantonio commented
FileSize
patch-review.png83.38 KB

Hello,

The patch in comment #36 fixes what it say that it was going to fix, I tested it manually by checking the CSS and I also I checked visually. Please remember to select the theme Bartik as the default for the admin in order to test this issue in the future.

tmjoseantonio’s picture

tmjoseantonio CreditAttribution: tmjoseantonio commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs screenshots
emma.maria’s picture

emma.maria
she/her
CreditAttribution: emma.maria commented
Issue tags: +LatinAmerica2015
webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

Looks much nicer now! :) Great work.

Committed and pushed to 8.0.x. Thanks!

  • webchick committed 0de81e3 on 8.0.x 
    Issue #2226189 by alvar0hurtad0, sivaji@knackforge.com, mherchel, emma....

Status: Fixed » Closed (fixed)

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