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.

CommentFileSizeAuthor
#5 1565812-admin-suppress.patch719 bytesdave reid
#4 media-suppress_admin_module-1565812.patch854 bytesothermachines
#1 media-media_browser_margins-1565812.patch776 bytesothermachines

Comments

othermachines’s picture

othermachines commented
Status: Active » Needs review
StatusFileSize
new media-media_browser_margins-1565812.patch776 bytes

Patched...

RobW’s picture

RobW commented

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

RobW commented

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

othermachines commented
StatusFileSize
new media-suppress_admin_module-1565812.patch854 bytes

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

dave reid
he/him
Primary language English
Location Nebraska USA
commented
StatusFileSize
new 1565812-admin-suppress.patch719 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

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Status: Needs review » Fixed

Tested and confirmed #5. Committed to 7.x-2.x and 7.x-1.x.
http://drupalcode.org/project/media.git/commit/ef8c386
http://drupalcode.org/project/media.git/commit/7c4ecb5

Thanks!

RobW’s picture

RobW commented

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

RobW commented

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.