Optimize theme fieldset functions - vertical tabs broken [#589918]

Hi mario,
..found some more time :-)

You should write the fieldset in "function theme_pollfield" like that:

...     
$form = array(); 
  $form[$fieldname] = array(
  '#type' =>'fieldset',
        '#title' =>t('Extra cck fields'),
        '#collapsible' =>TRUE,
        '#collapsed' =>TRUE,

because if someone uses vertical tabs this cck field is better included. ( Now the fieldset is not displayed as fieldset in vertical tabs)

Cheers Kai

Comment	File	Size	Author
#8	pollfield_589918.patch	841 bytes	hadsie
#7	pollfield_589918_2.patch	605 bytes	hadsie
#5	pollfield_fieldset.patch	727 bytes	hadsie

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

Apfel007 CreditAttribution: Apfel007 commented 16 October 2009 at 14:31

Hi,
I'm back from holiday.

Did you have a look on it?

Cheers

Comment #2

mario_prkos CreditAttribution: mario_prkos commented 16 October 2009 at 16:28

I am glad you back. I hope you enjoyed your holiday. Yes I did check out about this issue. As I understand theme function their output should be some kind of html code which can bu used to print content of node. Did you have in mind using Form API to build fieldset and then using drupal_render to generate html or you have something else in mind. Pleas, correct me if I misunderstand something. Looking forward to see more comments and contributions.

Cheers, Mario

Comment #3

Apfel007 CreditAttribution: Apfel007 commented 18 October 2009 at 20:56

Hi mario,

I'm at the start of coding drupal.. :-) but yes - if I understood everything right, I would suggest the form api..
This should be the right way..

/**
 * Implementation of hook_form_alter().
 */
function pollfield_form_alter(&$form, $form_state, $form_id) {
  $form['pollfield'] = array(
    '#type' => 'fieldset',
    '#title' => t('Pollfield settings'),
    '#collapsible' => TRUE,
    '#collapsed' => TRUE,
  );
  $form['pollfield']['question'] = array(
    '#type' => 'textfield',
    '#title' => t('Question'),
    '#rows' => 2,
    '#default_value' => isset($element['#value'][$field]) ? $element['#value'][$field] : '',
    '#description' => t('The question this poll will ask.'),
  );
  $form['pollfield']['status'] = array(
    '#type' => 'radios',
    '#title' => t('Status'),
    '#default_value' =>isset($element['#value'][$field]) ? $element['#value'][$field] : 1,
    '#options' => $_active,
    '#description' => t('When a pollfield is closed, visitors can no longer vote for it.')
  );
   $form['pollfield']['duration'] = array(
    '#type' => 'select',
    '#title' => t('Duration'),
    '#default_value' => isset($element['#value'][$field]) ? $element['#value'][$field] : 1,
    '#options' => $_duration,
    '#description' => t('After this period, the pollfield will be closed automatically.')
  );
  //anonymous voting
  $form['pollfield']['anonymous'] = array(
    '#type' => 'select',
    '#title' => t('Anonymous voting'),
    '#default_value' => isset($element['#value']['anonymous']) ? $element['#value']['anonymous'] : 1,
    '#options' => array(
	'anon_non' => t('Not allowed'),
    'anon_by_ip' => t('Allowed, filtered by user IP'),
	'anon_by_cookie'=>t('Allowed filtered by user cookie'),
    'anon_all' => t('Allowed')),
    '#description' => t('Allow or disallow anonymous users to vote on this poll. Filtering by user IP or cookie ensures only one vote is allowed per user IP.')
  );
}

Cheers Kai

Comment #4

Apfel007 CreditAttribution: Apfel007 commented 8 December 2009 at 17:21

What did you think about this?

Comment #5

hadsie CreditAttribution: hadsie commented 15 January 2010 at 20:05

Status:

Active

» Needs review

File	Size
pollfield_fieldset.patch	727 bytes

I've attached a quick patch that turns the theme_pollfield into a proper themed fieldset. I don't think it's necessary in this case to drop the fieldset into form_alter, since this is actually displaying the pollfield field, not additional settings as other modules often add.

The other issue is that the title is being overridden by a new title "Pollfield settings", but I don't think that makes sense, it should use the title set on that particular field. What if you have two different pollfield fields on a single node? You wouldn't be able to differentiate the two.

I think you could also add the fieldset properties right into the $element in pollfield_pollfield_process() and that would probably be a more ideal way. All your theme_pollfield module would need to do then is return drupal_render($element);

Note that this will should also take care of #651066: Node form : "Pollfield settings" is untranslateable

Comment #6

mario_prkos CreditAttribution: mario_prkos commented 17 January 2010 at 20:47

Yes, I agree with you and I will fix this in dev version soon as possible. Thank you for your contribution. Soon dev version will be release. Also, I will probably do some other changes so than I will do it this in second way you mention. For now I will use your patch you provide for us, thanks again.

Comment #7

hadsie CreditAttribution: hadsie commented 22 January 2010 at 16:08

File	Size
pollfield_589918_2.patch	605 bytes

I've made a slight modification with this, and patched against the latest dev version. This patch allows the developer to also override the collapsible / collapsed fieldset settings if they run a hook_form_alter.

Comment #8

hadsie CreditAttribution: hadsie commented 22 January 2010 at 16:41

File	Size
pollfield_589918.patch	841 bytes

I missed the #attributes in the last patch.

Comment #9

hadsie CreditAttribution: hadsie commented 2 February 2010 at 13:40

Just looking through the latest dev version and this looks to be fixed now. Thanks Mario!

Still testing it but looks good.

Comment #10

mario_prkos CreditAttribution: mario_prkos commented 2 February 2010 at 14:31

Great, let me know is there any problem so I can put this in release version!
I appreciate very much any help on testing because it takes me lots of time.

Mario.

Comment #11

igorik CreditAttribution: igorik commented 3 June 2010 at 21:06

Status:

Needs review

» Needs work

Hi, last patches nor the first code doesn't work for me. (and it is mainly already implemented in pollfield module)
but i still can't got pollfield fieldset into vertical tabs...

any help?
thanks

Igor

Comment #12

mario_prkos CreditAttribution: mario_prkos commented 5 June 2010 at 14:33

I think you have to declare it in settings.php it is explained in vertical tabs docs. Try it let me know how is going.

Comment #13

nibblebot CreditAttribution: nibblebot commented 18 February 2011 at 00:13

fixed?

Comment #14

bryan kennedy CreditAttribution: bryan kennedy commented 29 May 2011 at 21:04

Assigned:

Unassigned

» bryan kennedy

This is still not working fully, with the D6 vertical tabs backport.

Comment #15

bryan kennedy CreditAttribution: bryan kennedy commented 14 June 2011 at 17:25

Title:	fieldset function should optimize	» Optimize theme fieldset functions - vertical tabs broken
Issue tags:		+theme

Just clarifying the title.

Comment #16

bryan kennedy CreditAttribution: bryan kennedy commented 21 June 2011 at 04:21

I spent some time on this tonight and got a whole lot of nowhere. The latest dev code does have the patches that Apfel007 suggested. however, I am still not seeing any pollfield fieldsets in the vertical tabs. I must say I am no expert on the form API. If anyone else has time to look at this code and suggest some changes I would appreciate it.