Date Picker - make field selection names clearer

[Venutius]

When you install the date selector as a profile field, it is displayed in the user registration page with each of the three fields shown as "---", but there is no indication as to which is Day, Month and Year.

Whilst the user can work this out by clicking on the fields, it's less than intuitive. a suggested fix would be to replace "---" with Day, Month and Year thus guiding the user as to what input is required.

[Venutius]

Replying to Venutius:

When you install the date selector as a profile field, it is displayed in the user registration page with each of the three fields shown as "---", but there is no indication as to which is Day, Month and Year.

Whilst the user can work this out by clicking on the fields, it's less than intuitive. a suggested fix would be to replace "---" with Day, Month and Year thus guiding the user as to what input is required.

[Venutius]

Sorry thought I'd selected enhancement first time round.

[DJPaul]

Great idea, @Venutius! Thanks for sharing.

[mercime]

Easiest way to help members is by adding a description to the date fields, like "e.g. 28 April 2017"

The other way is to remove the bp-screen-reader-text and screen-reader-text classes surrounding the <label> for the front and back end screens. While the change doesn't affect the backend screens--xProfile and Extended Profile-- it does affect the front end as <labels> are displayed as a block in BP Legacy and BP Nouveau.

Extended Profile Screen

xProfile Screen

Front end - <labels> are displayed as a block

The first option mentioned is safe layout-wise if hopefully, translators take care of the new text there before 3.0 rolls out. Otherwise, there'll be english words before the day, month, year selections.

[mercime]

if second option chosen

[DJPaul]

I asked @rianrietveld about the select/option element question, in the context of screen readers: "If the text of the label and the first option is the same, (VoiceOver) only announces it once. Leaves you with the possibility that people can select and submit the first option." I was also told that "hiding the label and using the first item as placeholder/label is in fact a hack", so this is a good opportunity to correct this.

I think using block elements here is going to be too big a change for most themes to support, so displaying them inline (albeit with the label now showing) is least invasive. I also prefer how it looks. I have run with @mercime's patch and made 7665-2.patch which adds CSS fixes for the above.

Outstanding tasks for feedback:

1) Do we change --- to Day, Month, Year (on each label appropriately), so we think the VoiceOver screen reader won't say (based on above screenshot) "Date of Birth, Day, dash dash dash" -> "Date of Birth, Day".

...or...

Do we come up with a phrase to replace ---, perhaps "Unset" (needs to be something short, but basically "No year selected"), so the VoiceOver might go "Date of Birthday, Day, Unset" or "Date of Birthday, Day, No year selected".

I prefer the latter though we'd have to use a very short text otherwise we will force the width of the select boxes to grow -- typically German is my go-to example because its words tend to be very long).

2) I'm not sure "bp-datefield" is the best idea of a CSS class here. @hnla can I use "inline-label" as a class name or do you have a better idea?

[hnla]

Not sure I mind bp-datefield it is however quite specific we perhaps need a more generic class that can be used to hook styles on inline-label perhaps goes too far the other way, too generic? Maybe xprofile-label?

[hnla]

Overall though the question of a default option is a thorny one, option elements are meant to be selectable values not labels, suggestions such as setting to disabled not really a satisfactory approach.

I'm not for doing much for legacy here due to upsetting layouts - the only true option is to display labels, but we should revisit the Nouveau template and remove the screen reader classes.

[DJPaul]

The relevant parts of the template is in core files, we shouldn’t distinguish there based on template pack. I’ll probably test this on the handful of themes that I have.

[hnla]

The relevant parts of the template is in core file

True, good point. It's a sadness though, it's one of those areas I would like to unpick and build better, it's always felt a struggle dealing with those edit profile functions.

[hnla]

Find myself once more trawling around those xprofile field type classes :(

Specifically edit_field_html() we can pass an optiona= $raw_properties to this function and render form control attr however the function is responsible for more than just the controls e.g the label elements where we hard code the 'screen-reader-text' It would help if we expanded the $args passed through to include ? 'show_label' => true or whatever way round makes sense as a default state. This way I can enable the labels visually for Nouveau which I think I would prefer tbh, at the least we have an option to play around with.

[hnla]

<digressing-rambling> However an issue I have always had with these edit fields is the manner in which they have been separated into class functions so changes need to be made across multiple files, then when it comes to working in the template as we loop things I have access to only one instance of $field_type->edit_field_html( array( 'show_label' => true ) ); so have a problem identifying what field I want to target. It has always felt like xprofile was written for the convenience of the backend not frontend.

Edit: just re-factored the datebox class method to take an additional param and changed the hardcoded 'screen-reader-text' class to a var, this at least now allows me the option in the template to turn on/off, that's one class though, the problem remains that I would need to re-factor all class functions to accept same param thus all profile fields would be shown/hidden without being able to be selective.

[hnla]

#6710 references an issue here and where I had to port some styles over to twentysixteen.scss, from what earlier style sheet I'm not sure yet and still not 100% sure why we hid this.

[DJPaul]

I need to test but if this change is accepted, I might change the proposed patch to use #buddypress label.bp-datefield instead of #buddypress .standard-form label.bp-datefield so the overall selector weight is less.

[mercime]

I might change the proposed patch to use #buddypress label.bp-datefield

+1. Tested on Windows. Looks good.

[hnla]

Lets commit this then, I did attempt to check over myself but browser vendors saw fit to wreck my localhost dev sites so can't localhost dev until I figure out how to tell myself who I am.

I'm in favour of using labels as intended though, Nouveau we can change with impunity at this stage and we'll adjust layout if necessary.

[DJPaul]

Fixed in r11905