This patch makes it so you can just add text to a banner w/o background images.

CommentFileSizeAuthor
#5 rotating_banner_div_zero.patch1.75 KBjames.elliott
rb_no_background.patch1.59 KBJacobSingh

Comments

AimAdvantage’s picture

AimAdvantage commented

Thanks Jacob,

Does this still enable the text to be linked to a URL?

Thanks for keeping up on this module as it is clearly the easiest solution for a non-flash rotating banner in Drupal and can still be styled fairly easily with CSS.

Aaron
http://www.aimadvantage.com

james.elliott’s picture

james.elliott commented
Status: Needs review » Needs work

I don't think this patch will work. You're initializing $smallest as 0 and then only updating it if the filesize is less or the variable is NULL. That means that $smallest will never change for the initialized 0 unless you have a file with a negative size.

JacobSingh’s picture

JacobSingh commented

If you have no file at all, the $smallest var will not be initialized, so there will be a PHP notice. You have to set it to something.

JacobSingh’s picture

JacobSingh commented

I also added a conditional below to not include a size if $smallest is 0.

james.elliott’s picture

james.elliott commented
Status: Needs work » Needs review
StatusFileSize
new rotating_banner_div_zero.patch1.75 KB

You're also initializing the variables at the beginning of the foreach() loop. That means that rather than being storage for whichever slide meets the criteria to overwrite those values, they're getting thrown out at each iteration so that the last slide in the banner will always be the smallest and tallest.

I think the real problem is that when there is no file, we still try to calculate the width/height ratio. I think that is where the div by 0 error is coming from. I've attached a patch to that effect.

JacobSingh’s picture

JacobSingh commented

Hey, $smallest is undefined (even outside the loop). So if there are no files in the banner, it will still throw a notice.

james.elliott’s picture

james.elliott commented
Status: Reviewed & tested by the community » Needs review

I've tested this a bunch of times and I'm not getting any div by 0 notices when viewing the rotating banner. I am however, getting 18 warnings and notices on the block config page.

https://skitch.com/jameselliott/rqss6/rotating-banner-my-banner-block-d7...

JacobSingh’s picture

JacobSingh commented
Status: Needs review » Reviewed & tested by the community

sorry, didn't see that $smallest was defined.

james.elliott’s picture

james.elliott commented
Status: Needs review » Fixed

Committed

Status: Fixed » Closed (fixed)

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

Mark Linden’s picture

Mark Linden commented
Status: Closed (fixed) » Needs review

Could not get this working under 7.x-7.x-dev or 7.x-2.x-dev.
It still requires an image to be assigned when creating a new banner.