Making a phone field required does not present error message [#1928662]

Comment	File	Size	Author
#6	phone-required-error-1928662-6.patch	2.3 KB	cdale
#5	phone-required-error-1928662-5.patch	2.27 KB	cdale
#4	phone-required-error-1928662-4.patch	2.26 KB	cdale
#1	phone-libphonenumber-required-error-issue.patch	752 bytes	cdale

Comment #1

cdale commented 27 February 2013 at 04:26

Status	File	Size
new	phone-libphonenumber-required-error-issue.patch	752 bytes

Here's a patch that corrects the issue, though, I'm not sure if we need to do more to support phone usage as a stand alone FAPI element. Thoughts? How would this normally be solved?

Log in or register to post comments

Comment #2

cdale commented 27 February 2013 at 04:39

Above patch is broken. Working on a fix.

Log in or register to post comments

Comment #3

cdale commented 27 February 2013 at 04:39

Status:

Needs review

» Needs work

Log in or register to post comments

Comment #4

cdale commented 27 February 2013 at 05:23

Status:

Needs work

» Needs review

Status	File	Size
new	phone-required-error-1928662-4.patch	2.26 KB

Updated patch. I think this is a better approach, and solves the issue.

Log in or register to post comments

Comment #5

cdale commented 27 February 2013 at 06:37

Status	File	Size
new	phone-required-error-1928662-5.patch	2.27 KB

One of those days. Couldn't sworn I added the !empty($error) check....

Log in or register to post comments

Comment #6

cdale commented 27 February 2013 at 06:38

Status	File	Size
new	phone-required-error-1928662-6.patch	2.3 KB

Definitely one of those days...

Log in or register to post comments

Comment #7

Nephele commented 27 February 2013 at 06:56

Just to quickly chime in with a different option. It's possible to provide a '#title' to the original element, but then specify '#title_display' => 'invisible'. That makes the title not appear on the edit form, but yet be available for error messages. I'm not sure whether that ends up being a better alternative.

Log in or register to post comments

Comment #8

cdale commented 27 February 2013 at 09:53

That would be ideal, except the $element['#title'] that gets provided to us in hook_widget_form(), is empty when it the cardinality is not 1. So what we get, is no #title, which, we can still output a message but I like the idea of making people aware which field is actually causing the issue, especially if they for some reason have many phone fields on the one form. I.m open to suggestions. I'm not 100% happy with the bubbles code either, but before that, I actually had an if(!empty($element['#title'])) display one message, or show something slightly different otherwise. The only difference, was that one included a name, and one did not.

This does seem somewhat related to a core issue I found, #980144: Issues with "required, multiple" fields in forms. Some of the first patches here make sense, but the final ones look to be rather broken for us, as for example, they remove the "ext" label. There has not been movement in that issue for sometime however.

I'm torn which way to go here, the code currently in place works for all use cases I tried, which I like. It's just a question of whether that is acceptable.