Hi,

After upgrading to PP 1.24 I've encountered this issue. It's similar to #2506371 but not the same as the modal has the close button in this case. But the modal seems to fail to load the necessary JavaScript and CSS, resulting in what you see below.

Much grateful for advice.

IMG

CommentFileSizeAuthor
#15 close_button_not_working.png33.42 KBsolipsist
#7 panopoly_core-overriding-jquery-ui-2525208-7.patch1.78 KBdsnopek
media-browser-broken.png88.4 KBsolipsist

Comments

solipsist’s picture

solipsist commented
Issue summary: View changes
dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
commented
Category: Support request » Bug report
Priority: Normal » Critical
Issue tags: +sprint

Thanks for the bug report!

What theme and admin theme are you using? Does one of them include a jQuery UI theme?

Also, just in case (while I'm sure you already tried this), can you try clearing all the Drupal caches? Either by going to Admin -> Configuration -> Development -> Performance or using drush cc all.

Thanks!

dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
commented

Out of curiosity, are you hosted on Pantheon? The issues on #2506371-19: Media dialog missing styles seem to have resolved themselves and @hfarrier thinks it might have been related to varnish caching on Pantheon.

hfarrier’s picture

hfarrier commented

I was mistaken about what I thought were issues with varnish caching. The admin theme we were using (Shiny) includes its own css_alter function on line 112 of template.php which was replacing the panopoly jquery ui css.

  // Use shiny's jQuery UI theme style instead of the default one.
  if (isset($css['misc/ui/jquery.ui.theme.css'])) {
    $css['misc/ui/jquery.ui.theme.css']['data'] = drupal_get_path('theme', 'shiny') . '/css/jquery.ui.theme.css';
  }

You may want to check if your admin theme is doing the same.

dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
commented

@hfarrier: Thanks for the update! In Panopoly 1.22 and earlier, we were replacing ANY jQuery UI theme with the one in panopoly_core, however, that prevented anyone from creating any theme that included a custom jQuery UI theme, so we switched it to just target Seven specifically which was the only one we knew was problematic.

However, if there are several problematic themes... I'm not sure what the right course of action is. Maybe we could replace all jQuery UI themes by default and provide a hook that users can use to allow their theme's jQuery UI theme to NOT be replaced?

hfarrier’s picture

hfarrier commented

For our needs, it makes most sense if the Panopoly admin experience is consistent for our content editors. So, we would prefer to have Panopoly replace all jQuery UI themes with the option of a theme hook to specify our own custom jQuery UI theme. However, other Panopoly users may have a different opinion.

dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
commented
Status: Active » Needs review
StatusFileSize
new panopoly_core-overriding-jquery-ui-2525208-7.patch1.78 KB

I've been thinking about this since yesterday, and this is what I'd like to propose:

  1. We keep overriding Seven's jQuery no matter what
  2. We add a configuration option called "Override jQuery UI theme provided by any theme" which defaults to Enabled
  3. If that option is enabled, we'll override all jQuery UI themes, just like we did in Panopoly 1.22 and earlier

Attached is a patch that implements this. I tested it with Shiny as the admin theme and it worked! Please let me know what you think.

dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
commented

@hfarrier: This issue might interest you as well #2528248: Text on tabs on Media Browser is lower than it is on other interfaces - the problem is much worse under Shiny than it is with Seven.

solipsist’s picture

solipsist commented

dsnopek: Thanks for responding. Yes, the site is hosted on Pantheon. Shiny is the admin theme. But I have the same problem on my local Vagrant box (Kalastack). The issue is still present on the Pantheon hosted site.

hfarrier: Thanks for the idea. I will definitely look into it!

Awesome help guys. Thank you! I'm on vacation right now so I can't follow the thread constantly but I'll try what you suggested and get back.

dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
commented

@solipsist: Thanks! It'd be great if you could test the patch when you get back. :-)

hfarrier’s picture

hfarrier commented

@dsnopek: We don't use shiny theme for the media browser but I tested it and can see the tab issues.

The patch works perfectly for us. Thank you for your work on this.

dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
commented
Status: Needs review » Fixed

I've committed this patch because it had a +1 from @hfarrier and we're being forced to release today because of #2529416: Update to admin_views 1.5 for SA-CONTRIB-2015-132 and I don't want this regression to live in another release of Panopoly. Hopefully, this solves @solipsist's problems too!

  • dsnopek committed 0471576 on 7.x-1.x 
    Update Panopoly Core for Issue #2525208 by dsnopek, hfarrier: Media...

Status: Fixed » Closed (fixed)

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

solipsist’s picture

solipsist commented
StatusFileSize
new close_button_not_working.png33.42 KB

Hi guys,

Back from vacay land. Had a chance to test this now. Great job! I can only spot one problem. The CSS for the button to close seems to have stopped working. Perhaps it's just an old version of jQuery UI that I've forgotten to update and unrelated to this issue.

Image

solipsist’s picture

solipsist commented
Status: Closed (fixed) » Active
dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
commented
Status: Active » Postponed (maintainer needs more info)

Hm, I'm not seeing that in my testing. What theme and admin theme are you using? You mentioned that there might be an old version of jQuery UI in the mix, can you check that? Thanks!

  • dsnopek committed 0471576 on 8.x-1.x 
    Update Panopoly Core for Issue #2525208 by dsnopek, hfarrier: Media...

  • dsnopek committed 0471576 on 8.x-2.x 
    Update Panopoly Core for Issue #2525208 by dsnopek, hfarrier: Media...
solipsist’s picture

solipsist commented

The jQuery UI tabs aren't working so all the tab content is rendered at once. I haven't been able to dig deeper into it but I found a similar issue here:
https://www.drupal.org/node/1974774

dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
commented

What theme and admin theme are you using?

solipsist’s picture

solipsist commented

Theme is based off Kalatheme.
Admin theme is Shiny.

nlambert’s picture

nlambert commented

I think this issue is related: #1974774: Media Browser renders all tabs visible

A simple css tweak fixes the problem for me.
https://www.drupal.org/files/issues/media-browser-render-all-tabs-visibl...

solipsist’s picture

solipsist commented

nlambert: See my comment #20 above :) Thanks though.

solipsist’s picture

solipsist commented

nlambert: That patch did it.

The 'x' close button is still not working however.

dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
commented

@solipsist: I haven't had a chance to test this yet (sorry!) but it's possible that the 'x' close button issue is in Kalatheme. There is a similar issue in Radix, which has to do with the order that jQuery UI and Bootstrap's javascript are loaded. See:

#2546874: jQuery UI dialog missing close icon

You might need to patch Kalatheme (or your child theme) to do something similar to the patch there.

solipsist’s picture

solipsist commented

Thank you for the suggestion. Admin parts of the site use Shiny though.

solipsist’s picture

solipsist commented

I ended up adding these lines of CSS at the beginning of custom.scss in Shiny:

/* Fixes issue with close button not working on the Media Browser modal */
.ui-dialog-titlebar-close.ui-corner-all {
  float: right;
  margin: 10px;
}

There's likely some change in Panopoly's CSS files that caused this in conjunction with some of the modules installed. I'll try and revisit and file a proper patch.

mpotter’s picture

mpotter commented
Priority: Critical » Normal

Lowering this to Normal because the main issue was fixed in #12 and the remaining issues are only with specific themes that are not part of the Panopoly defaults.

dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
commented
Status: Postponed (maintainer needs more info) » Closed (fixed)

I'm going to set this back to fixed, because I'm still unable to reproduce this.

@solipsist: if you come up with a patch to fix this generally in Panopoly, can you please open a new issue? Thanks!

solipsist’s picture

solipsist commented

@dsnopek: Ok. My CSS patch above fixed it as far as my needs are concerned.