Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use WordPress date format for Lesson table #150

Merged
merged 2 commits into from Mar 8, 2019

Conversation

Projects
None yet
3 participants
@alexsanford
Copy link
Contributor

commented Mar 7, 2019

Closes #133

This changes the date display for the Lessons table to match the format specified in the WordPress settings.

Note that it does not change the date format in the meta box, which was specified in #133. There are a few reasons for this:

  • Since we are using a datepicker, the displayed format is probably not quite as important.
  • We explicitly tell our users what format should be used in the input, and it is locale-independent.
  • The available date format specifiers for the datepicker and the WordPress setting do not match. It seems that it would be very difficult and error-prone to try to make the displayed format (which must work with the datepicker) exactly match the WordPress format.

I'm open to coming up with a solution for this if we feel that it is important enough.

Testing Instructions

  • Set a specific date for a lesson to drip.
  • Load the Lessons table. Ensure the date is displayed in the format specified by the WordPress settings.
  • Change the date format in the WordPress settings and reload the Lessons table. Ensure the date format in the table updates.

@alexsanford alexsanford added this to the 1.1.0 milestone Mar 7, 2019

@alexsanford alexsanford self-assigned this Mar 7, 2019

@alexsanford alexsanford requested review from jom and donnapep Mar 7, 2019

@donnapep
Copy link
Contributor

left a comment

Nothing that this change does actually change the format of the date that displays in the meta box as well.

@jom

This comment has been minimized.

Copy link
Contributor

commented Mar 8, 2019

re: @donnapep's comment, it seems like that also makes it so the date is no longer selected by the date picker. Doesn't seem to break on save, though.

@alexsanford

This comment has been minimized.

Copy link
Contributor Author

commented Mar 8, 2019

@donnapep Good catch!

Ok, so the way it works currently is:

  • The initial date that is displayed in the metabox is in the WP format.
  • As @jom said, that date is not selected in the datepicker.
  • When a new date is selected in the datepicker, the newly selected date displays in the Y-m-d format.

Personally, I think this is a bit confusing, and I would prefer the date to always display as Y-m-d within the metabox. I've made that change in 38d29fe but am open to suggestions! 🙂

@donnapep

This comment has been minimized.

Copy link
Contributor

commented Mar 8, 2019

Woah, didn't even catch that the date was no longer selected in the date picker. Credit to @jom for that one. 👍 Approving again. 😆

@jom

jom approved these changes Mar 8, 2019

@alexsanford alexsanford merged commit 4909b8c into master Mar 8, 2019

@alexsanford alexsanford deleted the fix/date-format branch Mar 8, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.