Originally, the Spotlight widget had a per-slideshow "Rotation time" field, which was switched to a global variable in this commit in 2012:

https://cgit.drupalcode.org/panopoly_widgets/commit/?id=581cc963707dd22b...

Then, in 2014, it got moved back to a per-slideshow "Slide duration" field:

#2330667: Add per-spotlight setting for slide rotation duration

So, if you have an old Panopoly site that you've updated over time, you probably have a lot of junk in there.

Also, we have a "Slide duration" field that isn't required, and if a user doesn't fill it out, it uses 4 seconds (or whatever you had set the global variable to before that was removed) which is not great usability since it's pretty unexpected behavior. It'd be better to give the field a default value, and set it to the actual value, rather than having this implied value that can't be changed anymore.

I've also been told by knowledgeable accessibility persons (@cboyden!) that 5 is a better default value :-)

So, let's normalize all this for the future!

Comments

dsnopek created an issue. See original summary.

dsnopek’s picture

Status: Active » Needs review
StatusFileSize
new2.96 KB

Here's a patch!

dsnopek’s picture

StatusFileSize
new3.05 KB
new1.59 KB

This fixes a bug in setting the default value and removes some debug code that accidentally snuck in

cboyden’s picture

There's still some code in the spotlight widget that falls back to a value of 4. In panopoly_widgets_field_formatter_view:

  // Include the Spotlight Rotation time setting
  $duration_field = field_get_items('fieldable_panels_pane', $entity, 'field_basic_spotlight_duration');
  if ($duration_field) {
    $duration_field = reset($duration_field);
    $duration = $duration_field['value'];
  }
  if (empty($duration)) {
    $duration = variable_get('panopoly_widgets_spotlight_rotation_time', 4);
  }

Should this be removed or changed to 5 to match the default field value?

dsnopek’s picture

StatusFileSize
new3.65 KB
new622 bytes

Ah, yes, thanks! If the the update hooks run, it shouldn't matter (because the value will be updated from blank to 5) but we should update that for consistency.

cboyden’s picture

Status: Needs review » Reviewed & tested by the community

We've been using this patch for a while and it's working well.

  • dsnopek committed 20c78c7 on 7.x-1.x
    Update Panopoly Widgets for Issue #2973185 by dsnopek, cboyden:...
dsnopek’s picture

Status: Reviewed & tested by the community » Fixed

Committed!

Status: Fixed » Closed (fixed)

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