Tickets :: Comment on [#3421] Add click2dial support to phone number renderers

* Your Email Address
* Spam protection	Enter the letters below: . .. ..__..__ __. \| \|\|\ \|\| \|\| \(__ \|__\|\| \\|\|__\\|__/.__)
Comment	>> Here you go :-) I didn't remove / rename Horde_Form_Type_cellphone as > >> cellphones > >> will also accept SMS messages. I modified Horde_Form_Type_cellphone to call > >> Horde_Form_Type_phone and to also add a link to the SMS engine (if > >> some Horde app > >> provides SMS). > > > > Good thinking, that makes sense. Some comments: > > - this doesn't apply against latest CVS, can you make sure you have > HEAD and re-generate the patch? > > > > - you shouldn't set nicetitle attributes directly - set them as title="" > > > > - you use $html for a few too many different things, I thought it was > buggy; on second reading it looks like it isn't, but it's confusing. > :) > > > > - you need to either attach the voip-call.png image, or remove that > part from the patch. > > > > - _renderVarInput_phone should get the current body of > _renderVarInput_cellphone, and _renderVarInput_cellphone should just > call _renderVarInput_phone. If you think both should be replaced with > _renderVarInput_text, I can see that, too, but then you should just > remove both methods. > > > > - you should check for errors from the registry call.
Attachment
Watch this ticket