More [and improved] HTML helper functions #1664

Closed
igravity opened this Issue Jul 28, 2012 · 11 comments

Projects

None yet

5 participants

@igravity

I think the HTML helper functions are really sparse, more can be added. As at 2.1.2, only br() , heading() , img() , link_tag() , nbs() , ol() and ul() , meta() and doctype(). Support for img hopefully following an image_path method, alongside other tags.

Also, pertaining to improving the HTML helper, is the CodeIgniter team okay with end users writing HTML as an argument for a helper function? My initial thought was more of an array approach. Now I understand CodeIgniter prides itself in being fast so a string HTML would be ideal to pass in as arguments for a helper function. However, the array approach will also be helpful. I don't think it would be much overhead to add an option for an array-style argument list.

Current implementation of the heading() function forces users to use it thus

<?php 

// for a normal h1 tag -->
heading("Fried Beans");

// heading levels can also be specified
heading("Ingredients", 3);

// assuming classes and ids have to be added too...
heading("Step 1", 3, "class='step' id='step-1'");

What I am proposing would allow users the option to write the last example thus...

<?php
heading("Step 1", 3, array('class' => 'step', 'id' => 'step-1'));

And if there is a branch already working on this, someone please point me to it. I am currently working on the develop branch. The HTML5 issue is closed, and I think so is all work on that branch

@alexbilbie

Not a bad idea

@igravity

I would like to help out on this. Can I just submit a pull request on the development branch, or is there a specific branch for this sort of stuff?

@ericbarnes

I just pushed a branch for this:
https://github.com/EllisLab/CodeIgniter/tree/issue1664-htmlhelper

I only did the heading, list, and img functions so far. Still needs more unit testing and probably cleaning. But feel free to jump in.

@igravity

Sweetness. Thanks!

@igravity

I just submitted a pull request for this issue. As of now, $attributes can be added to any helper function and _stringify_attributes($attributes) can be called to return a string version of the attributes if they are passed as arrays or strings.

@philipptempel

There's already a function called _parse_attributes($attributes, $javascript = FALSE) in the system's url_helper. Maybe you can incorporate that function in your heading-helper? Of course it would be ugly to load one helper from another helper, but why create two functions for basically the same task? That's what helpers are all about: keeping it DRY ;)

@ericbarnes
@igravity

I took a look into the functions mentioned. _parse_form_attributes seem to be specific to forms. I also saw _attributes_to_string in form_helper.php that looks like it's dedicated to form_open only. I think _stringify_attributes is a method all these can "inherit" from to build their respective behaviours upon.

If any function should be moved to common.php, is should be _parse_attributes in url_helper.php. Right now, it is only dedicated to variations of the anchor function but I think it had way more potential than that.

@ericbarnes

yeah good ideas. You want to send a pull for that?

@philipptempel

I'm working on it right now (made a helpers/common branch on my fork). During the next few days I'll look through every file where _parse_attributes is used and may change it to use the function in common_helper.php, if that's okay with the team?
Also I may look through all other helpers and search for duplicate helper-functions and move all of them to common_helper.php.

@narfbg

There's no point in keeping this open. Anybody is free to submit a PR with new additions.

@narfbg narfbg closed this Jan 8, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment