[development] Re: [Privatemsg bug] Incorrect approach in theme_privatemsg_list

mixel info at mixel.be
Sat Sep 30 14:38:47 UTC 2006


I'm on vacation, will be back to check my mails on 30 september


On 26-sep-06, at 16:38, Duke <drupal-devel at drupal.org> wrote:

> Issue status update for http://drupal.org/node/86090
> Post a follow up: http://drupal.org/project/comments/add/86090
>
> Project:      Privatemsg
> Version:      4.7.0
> Component:    Code
> Category:     bug reports
> Priority:     normal
> Assigned to:  Anonymous
> Reported by:  Duke
> Updated by:   Duke
> Status:       active
> Attachment:   http://drupal.org/files/issues/privatemsg_list.patch  
> (1.31 KB)
>
> Hello.
>
>
> You may consider this to be a minor bug, but it I decided to tell  
> about
> it because it breaks Drupal form API idea.
>
>
> It is incorrect to create a theme_privatemsg_list function and build a
> form component model in it which is then displayed with help of
> drupal_get_form('privatemsg_list_form', $form). A part of the form is
> outputed in theme_privatemsg_list and another part in  
> drupal_get_form()
> which is inconvinient - we need to override both theme functions to
> customize output, difficult to create an accurate layout.
>
>
> drupal_get_form() function already has an ability to call a theme()
> function depending on form_id. So, you just need to build a form model
> and call drupal_get_form('privatemsg_list_form', $form). Then you can
> override theme_privatemsg_list_form() function to customize output of
> the whole form.
>
>
> My patch is very simple, it does 2 main things:
> 1. Change theme("privatemsg_list",...) call to privatemsg_list() call.
> 2. Remove $out = theme('links', $folder_list); from privatemsg_list()
> function and change it to:
>  $form['folders'] = array(
>      '#type' => 'markup',
>      '#value' => theme('links', $folder_list)
>      );
> which is the main fix which gives us a single form with all data in  
> it.
>
>
> Please review my patch.
> Thank you for your good and usefull module.
>
>
>
>
> Duke


More information about the development mailing list