Add a UI for adding a session time on "add session" pages

[melchoyce]

Right now, you have to add times manually:

This is hard, time consuming, and not at all user friendly. It would be so much easier for WordCamp organizers if it was given the default jquery UI calendar we include in core, plus some sort of time input field.

[iandunn]

related ​#wordpress7665

[iamfriendly]

Just a quick note that the inbuilt jquery datepicker in WP doesn't have a CSS theme. ​https://core.trac.wordpress.org/ticket/18909 is the ticket for that so when that gets done, this will be a really good first patch for someone.

[nvwd]

split the dateTime field into date, time( hours, minutes, meridiem ) fields

[nvwd]

removed the date time format hint

[iandunn]

Thanks Nowell, the UI from the patch is a big improvement :)

I am seeing a bug while testing on my sandbox, though; when I load a post that already has a time saved, the date/time is displayed correctly, but if I change the time and save it, the wrong date/time is shown after the refresh. It looks like the meta field is being saved as an empty string instead of the new time.

Let me know if you can't reproduce that on your sandbox and I'll dig into it to figure out what's going on.

The rest of the things I noticed are just minor tweaks:

• Normally it's best to keep any JavaScript and CSS in external files to keep the separation of concerns and maintainability, but since there's just a small snippet for this, it's not really worth the extra HTTP request. I think it'd be a little better to just have an admin_print_footer_scripts callback function that prints it.
• jquery-ui.min.css should use 1.11.2 as the cachebust parameter to wp_enqueue_scripts(), since it's an external project with an actual version. Cachebusters like 20141028 are just used when we don't have a version to tie it to.
• We should add a note to the line that enqueues jquery-ui.min.js to remind us that we won't need to bundle it anymore once it's included in Core in ​wordpress:#18909
• printf() is a simpler way of doing echo sprintf()
• It'd good to use esc_attr() and the other escaping functions, even when you know the value is your outputting is safe (like the hardcoded 1-12 for hours). It may be a bit overkill, but there are a few benefits:
  • 1) It saves time for other devs who are looking at the code, because they don't have to trace the value back to where it's assigned to make sure it's safe to echo without escaping; and
  • 2) It protects against situations where the value is changed to something unsafe in the future and the developer doesn't realize they now need to escape the value. It's not a huge deal, but it's a good habit to get into.
• The meridiem select options can just be hardcoded instead of having a loop, since there's only 2 of them. I think they'd be a bit more readable that way.
• I like using sprintf() in many situations, but when creating the markup I think it'd be more readable output the HTML and use inline PHP:

  <select name="wcpt-session-hour" aria-label="Session Start Hour">
  	<?php for ( $i = 1; $i <= 12; $i++ ) : ?>
  		<option value="<?php esc_attr( $i ); ?>" <?php selected( $i, $session_hours ); ?>>
  			<?php echo esc_html( $i ); ?>
  		</option>
  	<?php endfor; ?>
  </select>
  

• When saving the new time, we're only using the input values once, so we don't really need to store them in variables, which makes it a bit cleaner:

  $session_time = strtotime( sprintf(
  	'%s %d:%d:00%s',
  	sanitize_text_field( $_POST['wcpt-session-date'] ),
  	sanitize_text_field( $_POST['wcpt-session-hour'] ),
  	sanitize_text_field( $_POST['wcpt-session-minutes'] ),
  	sanitize_text_field( $_POST['wcpt-session-meridiem'] )
  ) );
  

• It shouldn't be an issue in this situation, but as a general rule, it's good to use absint() instead of sanitize_text_field() when expecting a number. In some situations, malicious input could be crafted in a way that exploits the fact that the input is evaluated as a string.

[nvwd]

@iandunn,

Thanks for the feedback. Looking into it now to see what I can find.

[nvwd]

made the updates suggested by @iandunn

[iandunn]

In 968:

WordCamp Post Types: Add a datepicker and time dropdowns for Sessions.

This is a much better UX than asking users to type out Y-m-d H:i:s.

Fixes #205
props nvwd

[iandunn]

Great work on this Nowell, I know there are lots of organizers who will love this :)

Also, kudos for using aria-label on the hour/minute dropdowns :)

One potential drawback is that now there isn't any way for organizers to leave the date/time blank. I don't think that'll be an issue for anyone, but if it is then we can think about adding a checkbox for a null timestamp, or something to that effect.

It looks like the bug I mentioned earlier was caused by the lack of padding in $1 during the for loop for the minutes. selected() was comparing (int) 5 against (string) 05. I changed it to:

for ( $i = '00'; (int) $i <= 55; $i = sprintf( '%02d', (int) $i + 5 ) )

Loosely-typed languages are fun sometimes :)

I made a few other minor tweaks:

• Added the optional year/month dropdowns to the datepicker, so users don't have to click a lot of times to go to a date a few months ahead of the current one.
• Removed the defaultDate because it seemed unnecessary, since the picker defaults to the value of the input field already.
• Removed the escaping on the am / pm strings. The note I made early about escaping safe values only applies to variables; it's overkill for hardcoded strings, since it's much safer to assume that any dev in the future who changes something hardcoded to user-input would then think about escaping it.

One final suggestion: if your IDE has a setting to truncate whitespace from the end of lines only on modified lines, then that will cut down on some of the diff noise that you can see in r968. I know PHPStorm does, but I'm not sure about others.

That's all small stuff, though; I'm only mentioning because I know you're interested in getting lots of feedback :)

[nvwd]

Thanks for the feedback.

Great idea adding the month/year dropdowns.

I did have Sublime Text trimming trailing white space.