Ticket #333 (closed Feature Request: fixed)

Opened 6 months ago

Last modified 2 months ago

[Modules:Forge] Form 'id' attribute support - new constructor argument?

Reported by: sbeattie Assigned to: Shadowhand
Priority: minor Milestone: 2.1
Component: Modules:Forge Version: SVN HEAD
Keywords: Cc:
SVN Revision (if applicable):

Description

Suggested minor amend to Forge_Core::construct() to add support for <form> element id attribute in resulting markup (maybe there's a better way?): 

	public function __construct($action = '', $title = '', $method = NULL, $id = 'form', $class = 'form')
	{
		// Set form attributes
		$this->attr['action'] = $action;
		$this->attr['id'] = $id;
		$this->attr['method'] = empty($method) ? 'post' : $method;

		// Set template variables
		$this->template['title'] = $title;
		$this->template['class'] = $class;
	}

thanks

Change History

01/24/08 19:46:42 changed by sbeattie

I think it also makes more sense to use the $class argument as an attribute of the resulting <form> tag too as anything can be referenced in CSS below this tag: 

public function __construct($action = '', $title = '', $method = NULL, $id = 'form', $class = 'form')
	{
		// Set form attributes
		$this->attr['action'] = $action;
		$this->attr['id'] = $id;
		$this->attr['method'] = empty($method) ? 'post' : $method;
		$this->attr['class'] = $class;

		// Set template variables
		$this->template['title'] = $title;
		
	}

01/28/08 13:59:35 changed by sbeattie

  • owner changed from Shadowhand to - No owner -.

In addition to setting an 'id' attribute on a form it makes sense to also set the 'name' attribute too.

For now, I'm using the following class to override the default Forge_Core::constructor() from within my application.

my_application/libraries/MY_Forge.php: 

<?php defined('SYSPATH') or die('No direct script access.');

class Forge extends Forge_Core {

	public function __construct($action = '', $title = '', $method = NULL, $id = 'form', $class = 'form')
	{
		// Set form attributes
		$this->attr['action'] = $action;
		$this->attr['name'] = $id;
		$this->attr['id'] = $id;
		$this->attr['method'] = empty($method) ? 'post' : $method;
		$this->attr['class'] = $class;

		// Set template variables
		$this->template['title'] = $title;
	}
}

01/28/08 16:46:53 changed by Shadowhand

  • status changed from new to assigned.
  • owner changed from - No owner - to Shadowhand.

01/28/08 17:16:10 changed by Shadowhand

  • status changed from assigned to closed.
  • resolution set to fixed.

Fixed in r1853.

02/04/08 20:48:01 changed by sbeattie

  • status changed from closed to reopened.
  • resolution deleted.

The changes in r1853 make sense but shouldn't the extra attributes be added to the $this->attr array and not the $this->template array?

The change in r1853 makes the extra attributes variables for the form template view. This is really useful functionality in itself but perhaps better served by a new method - Forge::setTemplateProps( $arr ) or similar. I was looking for a way to add more HTML attributes to the opeining form tag.

Changing line 36-37 of Forge.php as follows seems to work. 

  // Append extra attributes
  $this->attr += $attr;

thanks

02/04/08 20:55:02 changed by Shadowhand

  • status changed from reopened to closed.
  • resolution set to fixed.

Fixed in r1916. Thanks!

05/30/08 13:36:36 changed by airforce1

thanks get a cool air force ones