[buddypress-trac] [BuddyPress Trac] #5500: Date xprofile field enhancement

[buddypress-trac]

#5500: Date xprofile field enhancement
-------------------------------------+------------------
 Reporter:  sooskriszta              |       Owner:
     Type:  enhancement              |      Status:  new
 Priority:  normal                   |   Milestone:  2.7
Component:  Extended Profile         |     Version:
 Severity:  normal                   |  Resolution:
 Keywords:  has-patch needs-testing  |
-------------------------------------+------------------

Comment (by DJPaul):

 @boonebgorges `do_settings_section_field_types` etc - looks good. I
 understand why you're using `null` (implicitly) to see if the value is not
 set. How about setting the default to `null` explicitly to match the other
 properties, and also update the PHPDoc block? It's not always a bool.

 In `admin_save_settings`, the `default` block section:

 1) `wp_unslash` already happens in the change in
 `xprofile_admin_manage_field` where we call that function.
 2) New values for settings other than `range_relative_start` and
 `range_relative_end` are not being sanitised... or, is the later call to
 `validate_settings` meant to take care of that? If so, maybe add a comment
 explaining where it happens, because I first read that `intval` section as
 the input sanitisation. I only noticed `validate_settings` on my third
 read-through.

 (If I'm following that right!)

 In `admin_new_field_html`, there's some `<br>` being used presumably to
 space out the inputs. This is not semantically correct, and we should
 remove those and add custom CSS instead. (Also, we can remove the inline
 `margin-top` on one of the elements).

 Also in `admin_new_field_html`, the string `'From %s to %s'` needs
 attention. It needs a translation comment explaining what the `%s` values
 are. I also have no idea how that kind of structure will work for things
 like screen readers, but we can test after. I assume this was copied from
 WordPress, but that doesn't mean its good. :D

 In `get_date_formats`, the date string is missing a textdomain.

 Other than those niggles, I think this is ready to go!

--
Ticket URL: <https://buddypress.trac.wordpress.org/ticket/5500#comment:57>
BuddyPress Trac <http://buddypress.org/>
BuddyPress Trac