Fixes #877 - Updates date format #3240
Conversation
|
Added update to convert jobs table date: |
|
@anselmbradford This looks good, but I have one question--what about May? Will the period be removed or will it look like "May. 9, 2017"? |
|
Pushed the
|
|
Fixed the tests. |
| @@ -41,3 +41,20 @@ | |||
| {% endif %} | |||
| </span> | |||
| {% endmacro %} | |||
|
|
|||
| {% macro render_date_no_markup(datetime, | |||
There was a problem hiding this comment.
ehhhh, I'm not crazy about it. @richaagarwal, do you know a better way to write this code?
There was a problem hiding this comment.
It's awesome! Best code I've ever written! Look at all the brackets!
|
Why is this necessary? :) And/or did anyone request this? There are probably ways to write this a little more cleanly and I am happy to pair or brainstorm, but wanted to understand the need first. |
|
@richaagarwal See the "dates and date ranges" section in the content strategy doc linked via https://[GHE]/UX/Content-Strategy |
| {{ 'Sept. {dt.day}, {dt.year}'.format(dt = datetime) }} | ||
| {% else %} | ||
| {{ '{dt:%b}. {dt.day}, {dt.year}'.format(dt = datetime) }} | ||
| {% endif %} |
There was a problem hiding this comment.
I spent some time thinking about this and looking at possible alternatives. I couldn't find a Python library that has built-in support for formatting months like this (conditionality around when a month is abbreviated and to what character-number).
At the very least, I don't think this should be in the template itself, it should be a filter or something similar that can be called from any template (particularly if this is the standardized way to refer to dates) and can be properly unittested.
It would also simplify the logic greatly to use a mapping (like Python's build-in datetime library) of month number to abbreviation instead of ifs/elses.
There was a problem hiding this comment.
Thinking about it further, a custom wrapper around strftime that either introduces our own format code or handles %b differently from the standard library would mean we could actually use date formatting here (instead of dt.month, dt.day, dt.year) and in all the other places in this changeset like we should be doing.
There was a problem hiding this comment.
This all sounds well and good and over my head 😄
|
@anselmbradford Thanks for the background! That's interesting. I didn't see anyone reply to your comment -- was consensus reached elsewhere? I don't think we should be making these changes unless someone really wants them. If we do proceed, I agree with @willbarton that a wrapper around |
|
Closing this astoundingly stale PR, but leaving the associated issue open and updating the issue with context that a wrapper around |
Fixes #877
Changes
Testing
Screenshots