Skip to:
Content

BuddyPress.org

Opened 9 years ago

Closed 7 years ago

#6549 closed enhancement (fixed)

xprofile field type for phone number

Reported by: danbp's profile danbp Owned by: djpaul's profile DJPaul
Milestone: 3.0 Priority: normal
Severity: normal Version: 2.3.2
Component: Extended Profile Keywords: has-patch
Cc:

Description

As suggested by @boone on Slack, i open a ticket for this bug.

Description

When this field type is selected to let users enter a phone number, and this number starts by 0 (like most french phone numbers), the value who is shown once the user saved his number doesn't show the zero. Example: enter 0610919191. Save and you'll see 610919191 on edit screen. This can let the user think he forgot the zero, and he will redo and redo... Weird on a preview !

That said, on the profile View, the number is correct.

Other remark

That field comes also with an incorporated auto-increment counter. On some themes, like 2012, it's even difficult to see.

Seems to me that this field is not like a date field coming with a calendar, just a textarea which should contain a numercic data.

Is this overcomplicated thing necessary ? I fairly sure a user can enter a number manually, as he does for his name within a text field, without any prebuilt alphabetics, huh ? Less is more. ;-)

+1 for a specific phone field if not to complicated to buit-in. If not, stay basic, a very simple "numeric only" field would be enought.

The zero disucussion started on Slack is for devs. End users have to use the edit preview and to get there a correct information.

Thanks for attention.

Attachments (2)

6549-01.patch (7.6 KB) - added by DJPaul 7 years ago.
6549-2.patch (8.9 KB) - added by DJPaul 7 years ago.

Download all attachments as: .zip

Change History (12)

#1 @DJPaul
9 years ago

  • Component changed from General - UX/UI to Component - XProfile
  • Milestone changed from Awaiting Review to Future Release
  • Summary changed from xprofile numeric field to xprofile field type for phone number
  • Type changed from defect (bug) to enhancement

We absolutely do need to have a discussion about future profile field types that we could or should add, and this is one of them.

#2 @DJPaul
7 years ago

  • Milestone changed from Awaiting Contributions to 3.0
  • Owner set to DJPaul
  • Status changed from new to assigned

@DJPaul
7 years ago

#3 @DJPaul
7 years ago

  • Keywords has-patch added

I've added a patch for a phone number profile field type. Here's a copy of the development branch on my Github: https://github.com/paulgibbs/bptemp/compare/6549-phone-number

For practicality, it accepts any value as valid (same rules as the textbox), but on output, it transforms the value using a tel:// protocol link. Modern web browsers e.g. Safari, iOS, Android, will trigger that device's native calling functionality when such a link is touched.

Code review appreciated.

#4 @boonebgorges
7 years ago

This is neat. I didn't know about tel://.

What is the purpose of the -1 here? https://github.com/paulgibbs/bptemp/compare/6549-phone-number#diff-4b65738138aa6b4805151204f4f9e6bfR68 Should this be dynamic, in case there's more than one field of the type? There's a couple more hardcoded suffixes later in the same method too.

When concatenating regex I usually use preg_quote(). Probably doesn't matter here because of the limiter character set of the scheme, but might be a best practice. https://github.com/paulgibbs/bptemp/compare/6549-phone-number#diff-4b65738138aa6b4805151204f4f9e6bfR138

A question: I can imagine a site owner wanting to implement stricter validation, such as forcing the phone number format of a given country. This can be done pretty well with a client-side library, so maybe it's not relevant here. Do you think it's also worth considering a more obvious way to do that here as well, without having to register a new field type? I guess this'd be a filter on the format? https://github.com/paulgibbs/bptemp/compare/6549-phone-number#diff-4b65738138aa6b4805151204f4f9e6bfR31 This suggestion might be overengineering/overthinking.

#5 @DJPaul
7 years ago

What is the purpose of the -1

No idea about the -1 thing, I assume it's from the original profile field implementation pre-2.0. It's used in all other field types. It looks like the assumption made was that bp_the_profile_field_input_name() would be unique. If I introduced this in 2.0, I would need to go look at the commits logs and hope I left a clue. You also see -3 used for select boxes, etc.

concatenating regex

Good tip, I'll check that out!

stricter validation

We do have similar logic for, I think, date fields? We copied WP's logic from Settings > Reading? We could? I'm not against implementing something similar, though the default'll have to be (anything), and we could pick a handful of countries' phone number formats?

Last edited 7 years ago by DJPaul (previous) (diff)

#6 @boonebgorges
7 years ago

No idea about the -1 thing, I assume it's from the original profile field implementation pre-2.0. It's used in all other field types.

Ah, interesting. I guess maybe this should be examined as part of a separate ticket which would touch all field types.

I'm not against implementing something similar, though the default'll have to be (anything), and we could pick a handful of countries' phone number formats?

Honestly, I'm not sure we even need to go this far, at least not for v1 of this feature. I'd only suggest that we make it possible for plugin authors (or the BP team, in the future) to add more specific validation down the road. (I'm guessing phone number validation is an absolute nightmare, so I'm not confident about taking on the mantle.)

#7 @DJPaul
7 years ago

The -1 thing was introduced in #7348 I haven't looked into the details to (re-)understand the specifics (cc @mercime).

#8 @DJPaul
7 years ago

phone number validation is an absolute nightmare

Yes.

we make it possible for plugin authors

add_action( 'bp_xprofile_field_type_telephone', function( $field_type_obj ) {
  $field_type_obj->set_format( '/your_phone_number_regex_here/', 'replace' );
} );

Building a UI would be harder in a plugin, but AFAIK you could extend BP_XProfile_Field_Type_Number to create a bespoke phone number field type, de-register BP's, and look at BP_XProfile_Field_Type_Datebox for inspiration how to build the UI and validation elements.

Last edited 7 years ago by DJPaul (previous) (diff)

#9 @Offereins
7 years ago

I think @mercime will confirm that the -1 and -3 suffixes are for the references used in the related ARIA attributes (aria-labelledby and aria-describedby). They are most likely unique, since bp_the_profile_field_input_name() is always unique as long as the field only appears once on the page. However, one can argue whether it would be more clear/semantic/meaningful if the suffixes were actual words describing the elements which they identify.

Concerning the number-format input-or-filter issue, I'm with you guys.

Version 0, edited 7 years ago by Offereins (next)

@DJPaul
7 years ago

#10 @djpaul
7 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 11849:

xprofile: add telephone number field type.

Fixes #6549

Props danbp, boonebgorges

Note: See TracTickets for help on using tickets.