codeforamerica / ohana-web-search

A mobile-friendly website for finding human and social services in your community
http://ohana-web-search-demo.herokuapp.com
BSD 3-Clause "New" or "Revised" License
69 stars 74 forks source link

Location details page is not displaying certain phone number attributes #415

Open monfresh opened 10 years ago

monfresh commented 10 years ago

There are a few phone attributes that are not being displayed on the details page.

monfresh commented 9 years ago

Extension has been added, but not vanity_number. Here is a location that should have its vanity number displayed: http://ohana-web-search-demo.herokuapp.com/locations/admin-test-org/admin-test-location

anselmbradford commented 9 years ago

What's up with the languages spoken value on http://ohana-web-search-demo.herokuapp.com/locations/admin-test-org/admin-test-location? Is that an Ohana Web Search bug?

anselmbradford commented 9 years ago

Before this is implemented in the view format_phone in app/helpers/detail_format_helper needs to be updated to handle vanity numbers, or a new helper needs to be created to handle them (@monfresh, preference on combined vs separate helpers for formatting the numbers?)

anselmbradford commented 9 years ago

(Referencing https://github.com/codeforamerica/ohana-web-search/issues/172)

monfresh commented 9 years ago

Rails provides a number_to_phone formatter, so we don't have to write our own. It works best when the input only contains digits, so we can either ensure that format on the client side, or update the API app to either enforce that format, or have the API make the transformation.

To achieve the format we have now, you would do something like this:

number_to_phone(phone.number.gsub(/[^\d]/, ''), area_code: true)

Vanity numbers cannot be handled because they are not formatted consistently. They usually contain words of varying length that are meant to be together. For example, you would not want to format 800-FLOWERS into (800) FLO-WERS. There is no way of knowing in advance what letters are in the vanity number, so it would be futile to try to come up with a formatter that works for all of them.

So, as far as this issue is concerned, simply adding HTML to display the vanity number as it is returned by the API is what's needed.

anselmbradford commented 9 years ago

Rails provides a number_to_phone formatter, so we don't have to write our own. It works best when the input only contains digits, so we can either ensure that format on the client side, or update the API app to either enforce that format, or have the API make the transformation.

Are you saying this should be used in place of https://github.com/codeforamerica/ohana-web-search/blob/master/app/helpers/detail_format_helper.rb#L26 and that helper should be removed? (Perhaps in this PR.)

Vanity numbers cannot be handled because they are not formatted consistently. They usually contain words of varying length that are meant to be together. For example, you would not want to format 800-FLOWERS into (800) FLO-WERS. There is no way of knowing in advance what letters are in the vanity number, so it would be futile to try to come up with a formatter that works for all of them.

Ah yes, makes sense. Is there any consistency in the area code? That would always be digits, right? The example vanity number you referenced earlier looks like 703555-ABCD, it would be great to at least format the area code like (703) 555-ABCD. So then if there was an entry that had 800FLOWERS, it would become (800) FLOWERS.

Right now the demo entry looks like:

703555-ABCD 
(703) 555-1212 x1223
CalFresh

Without the area code formatting its confusing to even tell that first number is a phone number because it's formatted to look like a serial number or something.

anselmbradford commented 9 years ago

Hm, I guess the area code wouldn't HAVE to be digits. But I think this could still be formatted. For example given GETFLOWERS a basic formatter could format it (GET) FLO-WERS but it doesn't seem too terribly complicated to also have a check that:

monfresh commented 9 years ago

Let's not overthink this. People should be free to format phone numbers however they want, so yes, we should replace our current helper with the one Rails provides. That way, if one instance wants to format the phones like 703.555.1212, all they have to do is specify the delimiter, like this: number_to_phone(phone.number, delimiter: '.'). If people want to enforce the same or a separate style for vanity numbers, they can do that in the admin interface.

703555-ABCD is not how a typical vanity number would be formatted. I added that manually a while back, but I updated it earlier today to 800-45NOFUME, which is an actual vanity number in SMC.