Just a small issue: Admin module (used by Build Kit) will set the left margin on the tag to 260px when the administration menu is expanded. This messes with the display of the media browser. Issue can be resolved by taking over margin setting in media.css. Patch to follow.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

othermachines’s picture

Status: Active » Needs review
FileSize
776 bytes

Patched...

RobW’s picture

Is there a way to resolve this without !important? There should be no important declarations in any Drupal core or contrib css, especially if that css isn't divided into base.css, admin.css, and theme.css files.

RobW’s picture

Actually, since Admin module creates the conflict, I think this might be better in their issue queue.

I didn't do too much testing, but:

  1. It looks like Admin is probably adding the admin-ah class to all Drupal pages, whether they're an iframe or not. Fixing this would fix the style conflict.
  2. As long as you're in there, the menu expanding transition would be better done with css on the toggled class instead of adding a style attr to the body tag itself. Something like admin-expanded { margin-left: 260px; } in the stylesheet and then animating with a css transition. Better performance, less js, less dom manipulation, more separation between style and presentation, easier to maintain or alter, degrades gracefully, etc.
othermachines’s picture

Your comments are appreciated.

Having done some further investigation, it appears that the same issue arose with the admin_menu module. This was met with a "temporary fix" in media_page_alter():

Media issue queue: Issue #914834: Double admin menu...what does it mean?

Here is the related issue from the Admin Menu queue: Issue #914786: Best way to disable admin_menu on iframe popups

Discovered an issue posted in Admin (unfortunately no follow-ups as of yet): Issue #1268324: CSS Compatibility with Media module browser

Since it was seen as fit to provide a temporary fix for admin_menu, does it make sense to do the same for admin module?

If so, I've attached a patch (no !important necessary!). Works well in my installation.

Next step, I guess, is to mosey over to Admin and give them a poke...

Cheers -

Dave Reid’s picture

FileSize
719 bytes

After reading a couple of other related issues, I think this is the correct patch that should help resolve this.

Dave Reid’s picture

Status: Needs review » Fixed
RobW’s picture

Thanks for the expanded background. So is this just a general Drupal issue, that some modules render a whole page and can't tell everyone else they've done it in an iframe? Wonder if D8 has any plans for some sort of core hook iframe create, or better yet a core non-iframe modal API.

RobW’s picture

Ah, While I was writing David Reid replied and showed us the way. Thanks.

Status: Fixed » Closed (fixed)

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