MukurtuCMS / mukurtucms

GNU General Public License v2.0
83 stars 36 forks source link

Imported image files not displaying in Slick carousel - display as file names with links #123

Closed mTeej closed 4 years ago

mTeej commented 4 years ago

I emailed Michael Wynne about this last week. I'm pasting below Greg Tunink's information on the issue. He is the member of the UNL CDRH Dev. team who has looked at the issue. Unfortunately Greg is leaving the country in just a few short days, and won't be able to work on this until he returns in a couple of weeks. Perhaps Michael can use his sandbox account to the project website to at least see the issue in action.

Greg's notes from our GitHub account are below:

It looks like it's trying to use an image carousel called Slick. It is throwing errors in the browser console. That's what led me to find it. I think this is it: https://www.drupal.org/project/slick

Yup. And Mukurtu is using a version 1.9 which looks like it was reverted: kenwheeler/slick@a3131ca

Will look into this further tomorrow

I haven't found any way to customize or disable this Slick carousel/slider. So it looks to me like it is pretty firmly baked into Mukurtu.

I did find that both the theme and subtheme were trying to run JavaScript to initialize the Slick slider: double-slick

I didn't see any easy way to just disable it in the theme, so I emptied the duplicate JS file in the subtheme. After doing that, I no longer see the error in the browser console, but the page buttons / thumbnails still don't seem to behave.

I think it's broken and we're better off asking the Mukurtu folks about it. My guess is that it's supposed to stay on the page and just update the image. I think that's why we see the loading animation when one clicks one of the page / thumbnail links. It is probably supposed to do a preventDefault to stop navigating to a whole new page, but because something is broken the browser navigates through to the fallback URL.

taylor-steve commented 4 years ago

It's difficult to address your specific issue without seeing exactly what changes you've done to the theme/sub-theme, but I can describe exactly how the Slick carousel is baked it.

It's not baked in to Mukurtu specifically, but it is a part of the stock Mukurtu theme.

There are CSS and JS files specific to Slick in the Mukurtu theme:

Those CSS and JS files are loaded conditionally in the Mukurtu theme by the template.php file for the following content types:

The HTML display of the Slick carousel in the Mukurtu theme is controlled by the "templates/fields/field--field-media-asset.tpl.php" file.

If you are creating a sub-theme from the Mukurtu theme, it will be important that:

There are no front-end configurations for altering/disabling Slick in the Mukurtu theme. If you wanted to do so I would either:

Hope that's helpful, let me know if you have additional questions.

mTeej commented 4 years ago

I thought I replied to this comment here--apologies! I've forwarded this information to one of our developers, Greg Tunink, but he's currently out of the country and won't return until next week. Due to the workload after his return and that next week is a short week for us here due to Thanksgiving, I anticipate he won't be able to dive deeper into this until the first week in December. I will be sure to reach back out again if there's anything he may need to consult with the Mukurtu team about.

Thanks very kindly for your time and support! We are grateful.

techgique commented 4 years ago

Hey @steve-taylor-wsu, thumbnails for the Slick carousel were not rendering on "digital-heritage" multi-page documents, but I figured out a fix.

I followed what you said about the subtheme's use of those template files. I moved template.php from the root of the subtheme into a dont-use/ directory and that fixed the duplicated JS/CSS inclusion. Next I tried moving templates/fields/field--field-media-asset.tpl.php to my dont-use/ directory as well. This didn't fix the thumbnail rendering problem but had no apparent negative side effects either. I tried moving the entire templates/ directory into my dont-use/ directory. That actually broke the page display, so I restored all but the root template.php file back from my dont-use/ directory.

I tried modifying templates/fields/field--field-media-asset.tpl.php to see if it had any effect, but I determined this file was not actually affecting the page we're having trouble with.

From there I used CSS class names to search the subtheme files for where HTML was being rendered in the PHP. This led me to templates/system/page--node--digital-heritage.tpl.php. I tried a simple text print and it indeed was outputting the related HTML. These are the responsible lines of code:

So then I searched around for examples of how to properly output a thumbnail URL. I ended up replacing both of the above lines with this:

        $atom = scald_atom_load($sid);
        if($atom && !empty($atom->scald_thumbnail)) {
            $thumburl = image_style_url('carousel_thumbnail', $atom->scald_thumbnail[LANGUAGE_NONE][0]['uri']);
        } else {
            $thumburl = url(drupal_get_path('module', 'ma_digitalheritage') . '/images/no_media_available_150.jpg');
        }
        print "<img class=\"img-responsive\" src=\"$thumburl\">";

Thumbnails are now rendering appropriately, but I wanted to inform you in case this is a general bug that y'all might want to fix and to see if there is a better fix in the code to output the thumbnail image.

Please advise :smile:

techgique commented 4 years ago

Sorry, another related question. Do y'all have documentation on which PHP files specifically should and should not be included in a subtheme? We'll be focusing on more theme-related work in the near future and we just want to make sure we're doing things in the best way to make future upgrades the least painful after we customize our subtheme.

Thanks!

taylor-steve commented 4 years ago

Thanks for the detailed notes! We will take a look and see if there are adjustments we can make to the theme in general.

Regarding documentation, no we don't have any specifically addressing what each PHP file in the theme does (they are almost all named using the standard Drupal theme suggestion format). For a sub-theme, you would not want to copy template.php or theme-settings.php (your sub-theme could have its own versions of those files, but not identical).

For all the PHP files in the templates directory, in general I'd copy only the files you are making changes to. That way you are inheriting as much as possible from the parent theme.

techgique commented 4 years ago

Good to know! I'm not very familiar with Drupal and sub-theming so this helps. Thanks for the advice, Steve.

techgique commented 4 years ago

Any ideas on whether the changes I made are necessary for others? Or if there is an ideal long-term way to apply them, whether the theme file override in our sub-theme or otherwise?

The sub-theme file fixes worked pretty smoothly with the recent Mukurtu release upgrade, but that may not always be the case :man_shrugging:

techgique commented 4 years ago

I determined this was due to our sub-theme being broken. Once I re-created our sub-theme from scratch my fixes were no longer necessary. :v: