So when using the dialog token browser (specifically in Metatag module) the modal loads in my site's main CSS file breaking the admin theme layout. This seems to happen with or without overlay turned on.

Files: 
CommentFileSizeAuthor
#6 1859452-fix-dialog-page-theme.patch1.59 KBDave Reid
PASSED: [[SimpleTest]]: [MySQL] 347 pass(es).
[ View ]
#4 token_dialog-loads-theme-styles-4.patch1.01 KBkalabro
PASSED: [[SimpleTest]]: [MySQL] 347 pass(es).
[ View ]
#1 token_dialog-loads-theme-styles_1859452.patch1.17 KBjnettik
PASSED: [[SimpleTest]]: [MySQL] 345 pass(es).
[ View ]
break-with-overlay.png127.25 KBjnettik
break-wo-overlay.png213.7 KBjnettik
markup.png255.95 KBjnettik

Comments

jnettik’s picture

Version:7.x-1.4» 7.x-1.x-dev
Status:Active» Needs review
StatusFileSize
new1.17 KB
PASSED: [[SimpleTest]]: [MySQL] 345 pass(es).
[ View ]

It seems that drupal_get_css is loading in the theme CSS because /token/tree isn't viewed as an admin page. Not sure if this is the correct approach but I added admin to the path and that seems to fix the issue for me. Patch against dev attached.

jm.federico’s picture

There the problem would be that when loaded on a non admin page, the admin CSS would end up breaking the default theme.
There must be a way of telling the system which CSS we need.

jm.federico’s picture

Status:Needs review» Needs work
kalabro’s picture

Status:Needs work» Needs review
StatusFileSize
new1.01 KB
PASSED: [[SimpleTest]]: [MySQL] 347 pass(es).
[ View ]

I suggest to check if we are in admin section or not.
Works like a charm for me:

yannickoo’s picture

That makes sense will have a review on that later.

Dave Reid’s picture

StatusFileSize
new1.59 KB
PASSED: [[SimpleTest]]: [MySQL] 347 pass(es).
[ View ]

Actually I think emulating what the ajax system does is probably a better approach since it will work with any theme.

Dave Reid’s picture

Tested #6 and it seems to work really really well. We get the added benefit of the dialog not duplicating any CSS or JS that is already included on the parent page as well!

kalabro’s picture

Status:Needs review» Reviewed & tested by the community

Works really fast!
Thanks, @DaveReid!

Dave Reid’s picture

Status:Reviewed & tested by the community» Fixed

Committed #6 without the CSS/JS additions because without using ajax_deliver() it doesn't actually do anything. At least with this quickfix we don't get any conflicts since it's loading the same files.

I created a followup at #1925688: Attempt to load dialog window using AJAX system to see if we can actually switch this dialog loading functionality to the core AJAX system which would remove the need for this 'hack'.

Status:Fixed» Closed (fixed)

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