It would be nice if subscriptions ui had a setting to completely disable the fieldset wrapper around the form. Sometimes you just want to display the form as a block in a panel or perhaps in a modal window and the additional fieldset just gets in the way.

Patch to follow which will add an option for this.

CommentFileSizeAuthor
#5 subscriptions-disable-fieldset-wrapper-2269169-5.patch2.64 KBtimwood
#3 subscriptions-disable-fieldset-wrapper-2269169-2.patch2.63 KBtimwood
#1 subscriptions-disable-fieldset-wrapper-2269169-2.patch2.63 KBtimwood

Comments

timwood’s picture

timwood
Primary language English
Location Rockville, Maryland
commented
StatusFileSize
new subscriptions-disable-fieldset-wrapper-2269169-2.patch2.63 KB

Here is the patch!

timwood’s picture

timwood
Primary language English
Location Rockville, Maryland
commented
Status: Active » Needs review

Please review if you are interested. Thanks!

timwood’s picture

timwood
Primary language English
Location Rockville, Maryland
commented
StatusFileSize
new subscriptions-disable-fieldset-wrapper-2269169-2.patch2.63 KB

First patch had a typo and incorrect logic. Fixed in this patch.

timwood’s picture

timwood
Primary language English
Location Rockville, Maryland
commented
timwood’s picture

timwood
Primary language English
Location Rockville, Maryland
commented
StatusFileSize
new subscriptions-disable-fieldset-wrapper-2269169-5.patch2.64 KB

Sorry, uploaded same file. Here is the correct one.

salvis’s picture

salvis
he/his
commented
Status: Needs review » Closed (won't fix)

Thank you for your suggestion and patch!

Two comments:

1. I don't think this provides enough value for meriting an additional configuration option. We already have too many of them, and this can easily be accomplished with hook_form_alter().

2. Options should always be positive. A "completely disable" option that needs to be OFF to enable the fieldset (= normal behavior) is confusing. It would have to be something like "Wrap the Subscriptions options in a fieldset" and be ON by default.

timwood’s picture

timwood
Primary language English
Location Rockville, Maryland
commented

#2 implies you might be open to another variation of the configuration option and patch but #1 (and won't fix status) implies you aren't. Should I spend time on another patch or not bother?

salvis’s picture

salvis
he/his
commented

No, please don't. #1 is the overriding one.

#2 was just a thought that came up when I looked at your patch. Double negatives should be avoided wherever possible.

Adding such an option would double the number of possible display configurations, and you are the first person to ask for this option during my 7 years of maintaining Subscriptions.

Since you're already doing interesting things...

Sometimes you just want to display the form as a block in a panel or perhaps in a modal window and the additional fieldset just gets in the way.

... I'm sure you're not challenged to write a straight-forward hook_form_alter() implementation.

timwood’s picture

timwood
Primary language English
Location Rockville, Maryland
commented

Thanks for clarifying and for the feedback, oh and this awesome module.