this patch allows users to setup image paths used by the slideshow.
please review (and commit).

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Summit’s picture

Status: Needs review » Reviewed & tested by the community

Hi, working great. thanks!
greetings, Martijn

gmreed’s picture

Hi, I see that this patch is designated for 7.x-1.x-dev and when I tried it against 7.x-1.9 the business.info hunk failed to apply, so I'm assuming it shouldn't work for that version (please correct me if I'm wrong here). I also tried to manually add the rejected lines to business.info but then got the following error when I selected a field in the drop-down menu for the "Automatic Slide Image from a node's field":

Notice: Undefined property: stdClass::$type in _business_file_field_path_extract() (line 166 of /Users/powerbook/Sites/test/test717dec2012/sites/all/themes/business/theme-settings.php).

Please excuse my ignorance, but where can I download the dev version that this patch works with? (as I don't see it linked from the project page)

Thanks very much,
Graham

drzraf’s picture

all hunks succeed, even if some fuzzying is needed. I regenerated the whole anyway.
The notice could be fixed... it could also be safely ignored.
Caches may (or may not) need to be cleared after the patch is applied.
Ensure the state of checkboxes is consistent too and save the Business form at least once.

Hopefully, the maintainers will come back here one of these days.

gmreed’s picture

I agree it would be great if the maintainers integrated this into the theme!

Especially since I don't seem to be able to get things working properly on my site... Not sure what I'm missing - on the theme config page (.../appearance/settings/business) I have selected the image field to use for the slides, saved the settings and also cleared the site cache (all multiple times) but the slider still displays the default images. Which checkboxes need to be consistent?

drzraf’s picture

At the very least, filling the field Slide Image (from path) with an URI should get you done.
The other possibility is:

  • empty Slide Image (from path)
  • fill Slide URL with an existing node path (eg: node/2)
  • save.
  • The next time the form is loaded Automatic Slide Image from a node's field) contains available image-compatible fields
  • select one
  • save

Obviously the patch must be applied first and Show slideshow should be checked.

hope this helped

gmreed’s picture

FileSize
24.41 KB

I really appreciate your time with this.

The field Slide Image (from path) is indeed working.

Initially when trying to use Automatic Slide Image from a node's field I hadn't first cleared out Slide Image (from path) because of the help text for Automatic Slide Image from a node's field saying "If set this will override the below image path." But even when I clear it out and then select my image field from the drop-down menu, all I get is a slideshow without any images (with my settings as you describe above). [As an aside, after clearing Slide Image (from path) and saving the form, the field did not remain empty but instead showed the default image path again (despite those image paths not being used for the slideshow). Not sure if that is expected behaviour.]

But it may just be my faulty application of the patch (using the regenerated version in #3 above) because I still got a failed hunk when I applied it from Terminal in Mac OSX as follows:

patch -p1 < 1855926-3-business-theme-slideshow-images-UI-config.patch
patching file business.info
Hunk #1 FAILED at 41.
1 out of 1 hunk FAILED -- saving rejects to file business.info.rej
patching file templates/page--front.tpl.php
patching file theme-settings.php

I have attached a screenshot of the business.info.rej file that was created. It looked to me like the following lines just needed to be added at 44-49:

settings[slide1_imgfield] = "none"
settings[slide2_imgfield] = "none"
settings[slide3_imgfield] = "none"
settings[slide1_imgpath] = 0
settings[slide2_imgpath] = 0
settings[slide3_imgpath] = 0

Which is what I did, but since it isn't working perhaps I have messed things up...

drzraf’s picture

- didn't tried further, but that patch must applies unless you have local changes to your personal version of Business theme.
- patch must applies cleanly against 7.x-1.x
- if it fails you should merge the changes manually.
- you set "node/X": ok
- after a 1st validation, the image-compatible fields of node/X are shown: ok
- Then you'll need to find the issue yourself by doing var_dump inside business_form_system_theme_settings_submit :

  • does business_node_load succeeded ? if not, why ?
  • does _business_file_field_path_extract succeeded ? if not, why ?

Drupal structure are a bit unpredictable, so finding the fid in a random struct (image, file, ...) isn't a piece of cake. That's why the patch relies on a RecursiveIteratorIterator to find it out.

kspal’s picture

Works perfectly here. I've added the patch to my business' subtheme.
Thanks a lot

[Edit:] Actually, to use this script in a subtheme, you have to modify it a little. In theme-settings.php, remove the string 'business' in the calls to theme_get_setting (as well as its preceding comma), and replace the line

drupal_get_path('theme', 'business')

with

drupal_get_path('theme', $GLOBALS['theme'])
wonder95’s picture

I've been using this theme on a small project, and in the course of using it I made a bunch of enhancements to make it much more flexible and clean up the unnecessary code. I was about to post it as a separate issue, but I thought I'd post it here first, since there is a little overlap between my patch and this one.

The things I did in this patch are:

  1. Use loop in theme-settings.php to generate fieldsets for slideshow slides.
  2. Move fields to ['theme_settings'] in theme-settings.php
  3. Move creation of slideshow variables from page templates to template.php
  4. Added use of theme_image_style to take advantage of automatic resizing if images so that slideshow images don't have to be manually resized prior to upload.

Let me know what you think.

drzraf’s picture

while your patch truly brings enhancements it also provides differently a similar feature.
Most notably, while you allow to setup url, decription and image as text fields, I made the choice to grab the image from a node path. In this regard they match different use-cases while I must concede I chose a more complex one.

In the end, what I'm mainly looking for is that saran.quardz and guys from Devsaran dot com get back to their code and repositories. or will this theme stay orphaned during the coming years has it already happened so many times ?

drzraf’s picture

Status: Reviewed & tested by the community » Needs work

I forgot to say that both patch would actually conflict + back to "needs work".

I'm not willing to rewrite mine and do useless effort until maintainer state clearly what they intend to do with their theme. (I'd rather prefer to spend my time on another theme, maintained in the long run)
If only we can see what to expect in the near future; then we could try to grab the best of both patches.

my 2c

kspal’s picture

drzraf,

While I agree that we've not seen saran.quardz at lot recently on drupal.org, please consider that he has released 9 versions in 2012, which is pretty good. And should he stop maintaining this theme, we have the solution to create a fork since it is released under the GPL (that's more or less what I've started to do by creating a subtheme). The pitiful part is that we are all working in parallel on similar subjects, so wasting a little our time...

Saran, if you read us, what's about putting the theme's files on bitbucket or github, so that we can all contribute to it?

Regards
Kspal

ikhmer’s picture

Issue summary: View changes

Hello , I am newbie to drupal sorry if i asked strange question or maybe question already answer.

I want to create slideshow using views and I have installed all modules needed like views slideshow,circle...
when i display slideshow it show only 1 photo that i uploaded while i add my content.
I placed my slideshow folder in /site/default/files/sideshow. Do you think this path is right???

Thanks for you help!!