jQuery UI Accordion settings form does not let you set the heightStyle attribute which replaced autoHeight in jQuery UI Accordion version 1.9 and up.

Here is the documentation about this change and how to use autoHeight:

https://jqueryui.com/upgrade-guide/1.9/#deprecated-autoheight-clearstyle...

I propose we:

  1. Change the "Set to autoheight" option on the settings form to "Height style" which is a set of radios with three options: "Auto", "Content" and "Fill" and some explanation of what that means
  2. Check for the obsolete "autoHeight" setting and translate it to the new setting, if the new setting hasn't been defined (so we don't need to to an update hook - similar to #2530966: Accordion style setting "-1" on active doesn't work with jquery ui => 1.9)
CommentFileSizeAuthor
#3 panopoly_theme-jquery_ui_accordion_autoheight-2541838-3.patch3.8 KBhart0554

Comments

dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
commented
Issue summary: View changes
Related issues: +#2530966: Accordion style setting "-1" on active doesn't work with jquery ui => 1.9

Thanks! I updated the issue summary with some documentation about this change and a proposal.

dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
commented
Title: jQuery UI Accodion » Convert accordion "heightAuto" to "heightStyle" for jQuery UI >= 1.9
Version: 7.x-1.25 » 7.x-1.x-dev
Category: Task » Bug report
Related issues: +#2235081: Update jquery_update to version 2.5

Also, this is related to the issue that switched us to jQuery UI 1.10: #2235081: Update jquery_update to version 2.5

hart0554’s picture

hart0554 commented
Status: Active » Needs review
StatusFileSize
new panopoly_theme-jquery_ui_accordion_autoheight-2541838-3.patch3.8 KB

Here's a crack at a patch. I mapped from the previous settings fillSpace to fill, clearStyle to content, and autoHeight to auto. If there was nothing checked in the original settings it goes to auto since that's the default according to jQUI.

dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
commented
Issue summary: View changes
Status: Needs review » Needs work

@hart0554: Thanks for the patch! I reviewed and tested it and the form changes look great. :-)

However, we're also going to need to update theme_panopoly_theme_panopoly_accordion_style_render_region() to catch the old settings and convert them to heightStyle - otherwise it'll be necessary to edit and save the style settings everywhere an accordion is used to fix existing sites.

alex.bukach’s picture

alex.bukach commented
Status: Needs work » Needs review

#3 works for me.

@dsnopek if you check the patch it deals with converting old settings to the new ones, so one doesn't need to update the settings everywhere.

dsnopek’s picture

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

Ah, you're right! I'm not sure what I was thinking when I wrote that comment. :-/ In any case, looks good and works in my testing! Committed. :-)

  • dsnopek committed 5567d0b on 7.x-1.x 
    Update Panopoly Theme for Issue #2541838 by hart0554: Convert accordion...

Status: Fixed » Closed (fixed)

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