CVS edit link for stijndm

I have been working with Drupal since 2007 and I feel I am long overdue to contributing back to the community. I've tried following the issue queue but I easily forget to check back.

I've written lotst of modules, but often to project specific to be interesting for the community.

More recent I've been working on a Quick Edit block to work together with the next version of the Admin module. (http://m.sken.be/post/293599831/spelen-met-de-nieuwe-admin-toolbar-van)

I also have a working version of an Addemar plugin for Davy Van Den Bremt's Email Marketing Framework (emf) which currently is under internal review. But most likely will become a sponsored subset of emf.

But today I would like to contribute a module I have written that allows you to place form error messages inline with the corresponding form element (called: Inline Form Errors). You can see it in action on my contact page (http://sken.be/contact). A demo site is being setup where you can login an see how it works.

I'm aware of the following modules:
- Inline Messages (http://drupal.org/project/inline_messages)
- Inline Errors (now Inline Messages)

After some research these modules were not what we were looking for because of the following reasons:

1) Goals/result:
Inline Messages taks the messages set by Drupal and groups them above the form you submitted, creating a more clear connect between the errors and the forms.

As far as I can tell I isn't possible to place the errors in line with the corresponding form element. This is what Inline Form Errors does. It takes the error message from a certain form element and puts it directly below the highlighted field.

2) Approach:
Inline Messages uses jquery to pull the messages from the message container and then places them above the form.

Inline Form Errors does not use any jquery. With a form alter the elements are styled with an override-able theme function. In this theme function the element errors are fetched and placed beneath the themed element.

The original error messages (in $messages) can either be 1) replaced by a generic 'Please correct errors' message, 2) be removed entirely or 3) can be left in place together with the inline errors

It currently has been tested with some of the drupal core forms, custom built forms and with Webform.

It's a fairly simple module, but one that provides often asked functionality.
Hopefully it sounds interesting enough for a positive review.

CommentFileSizeAuthor
#7 stijndm-ife.zip6.91 KBstijndm
#5 ife.zip6 KBstijndm
#3 ife.zip5.09 KBstijndm
#1 ife.zip5.08 KBstijndm

Comments

stijndm’s picture

StatusFileSize
new5.08 KB

The code for Inline Form Errors

stijndm’s picture

Status: Postponed (maintainer needs more info) » Needs review

status change

stijndm’s picture

StatusFileSize
new5.09 KB

Forgot to define hook_perm :)

avpaderno’s picture

Issue tags: +Module review
stijndm’s picture

StatusFileSize
new6 KB

Some minor changes
hope to receive some feedback soon

Jo Wouters’s picture

stijndm,
I like the module you propose. It is a valid use case, and your module looks like a good solution for it.

Some issues i noticed while running through the code:
* maybe the name should be longer ? That way you reduce the chance on naming collisions. ex. "inline_error" instead of "ife"

* function form_id_load() should be renamed
Better: _yourmodule_form_id_load()

Coding standards:
* the title of a menu item should not be in a t()-funciton See http://drupal.org/node/140311

Performance:
* could you add some static caching in form_id_load() ?
Put all the form_ids in a static variable, so we don't call the select for every forms rendered on a page
(You might even consider putting the data in a caching bin)
* why an "ORDER BY form_id" in the query ?
* Did you consider limiting your query on form_id, instead of loading all form_ids ?

Update: I had a look at the code from #1 maybe some of these issues are allready resolved in #5

stijndm’s picture

StatusFileSize
new6.91 KB

New version of Inline form errors.

In response to Jo Wouters' issues:
- ife is shorthand for the full module name 'Inline Form Errors'. There are more modules out there that use a shorthand version in code. I'm not sure what the general opinion is, but form the moment I'm leaving it as is. This is up for debate.

- form_id_load() has been renamed

- t() removed from titles in hook_menu()

- static caching and a caching bin has been set up. This is now the working order: data pulled from static => if no static pulled from cache => if no cache pulled from database

- The function to load all form_ids is also used on the settings page, one query with multiple purposes. I thought it might be easier to administer the settings if the forms would be listed alphabetically.
- Again, I wanted to keep the different types of queries and functions to a minimum.

Now that everything is cached I don't think these last two points are an issue anymore.

Added two new features per request:
- CCK support (previously only core form elements would be recognized)
- An option to show form_ids with the form, none-devs were having trouble finding out what the form_id is

avpaderno’s picture

ife is shorthand for the full module name 'Inline Form Errors'. There are more modules out there that use a shorthand version in code. I'm not sure what the general opinion is, but form the moment I'm leaving it as is. This is up for debate.

I don't think the chosen name could create any problem, or confusion. As long as the name is at least three characters (even though there aren't rules about that), I think the name is fine.

stijndm’s picture

As a side note: I applied to take over the contest module, which the current owner approved. http://drupal.org/node/358802

avpaderno’s picture

Status: Needs review » Fixed

Status: Fixed » Closed (fixed)
Issue tags: -Module review

Automatically closed -- issue fixed for 2 weeks with no activity.

avpaderno’s picture

Component: Miscellaneous » co-maintainer application
Assigned: Unassigned » avpaderno
Issue summary: View changes