Follow-up to: #1812724: Consolidate all form element templates and add theme_hook_suggestons

Problem

As contained and clarified at the bottom of the change notice:

  • INPUT form element #types have a #pre_render callback, which translates the render element into theme variables.
  • The registered #pre_render callback for each form element in system_element_info() is essential, since it processes the render element #properties into #attributes.

  • Due to the way default element #properties are merged into element definitions, a form element definition like this omits the #pre_render callback:

      $form['foo'] = array(
    '#type' => 'textfield',
    '#pre_render' => array('mymodule_pre_render_foo'),
  );

    You have to manually ensure to include the required #pre_render:

      $form['foo'] = array(
    '#type' => 'textfield',
    '#pre_render' => array_merge(array('mymodule_pre_render_foo'), element_info_property('textfield', '#pre_render', array())),
  );

Goal

  • Introduce a new render element #property that defines a single callback to prepare and translate a render element for #theme.
CommentFileSizeAuthor
Screen Shot 2013-01-17 at 14.11.50.png108.79 KBswentel
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Issue summary: View changes

Updated issue summary.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Priority: Major » Normal

The issue summary doesn't make this clear why it is major. Nothing in core has to use the above pattern, if its a problem we should have a test module to illustrate it.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Priority: Normal » Major

To spell out the problem in KISS:

  1. $form['foo'] = array(
  '#type' => 'textfield',
  '#pre_render' => array('mymodule_prerender_foo'),
);
  2. Kaboom.
tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Version: 8.x-dev » 9.x-dev
tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Issue summary: View changes

Updated issue summary.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Version: 9.x-dev » 8.1.x-dev
Issue summary: View changes
Issue tags: -API clean-up +API addition

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.0-beta1 was released on March 2, 2016, which means new developments and disruptive changes should now be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented
Version: 8.2.x-dev » 9.x-dev
Status: Active » Postponed

May want to reword the issue title because it sounds like it's related to the translation module but it really doesn't sound like it is. Maybe transform? something more specific?

Regardless it would be way easier to deal with OOP render API because we could add/append/delete callbacks with methods on the Renderable object.(crosses fingers)

Pushing to 9.x unless we can get more details on what this should be doing. It's understood that the #pre_render would clobber unless you get the previous ones...

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Version: 9.x-dev » 8.3.x-dev
Priority: Major » Normal
Status: Postponed » Postponed (maintainer needs more info)
Issue tags: +Needs issue summary update

API additions can go into minor releases, but I don't get this issue at all, so moving to needs more info.

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented
Status: Postponed (maintainer needs more info) » Closed (outdated)

This has been waiting for feedback for some time. I'm closing this to triage the issue queue. Feel free to re-open if you still think this would be useful proposal. I can't think of how this would work from the issue summary proposal.