Skip to content

FEATURE: Extends Eel DateHelper with machine-readable formats#1301

Closed
putzwasser wants to merge 1 commit intoneos:masterfrom
putzwasser:extend-eel-datehelper
Closed

FEATURE: Extends Eel DateHelper with machine-readable formats#1301
putzwasser wants to merge 1 commit intoneos:masterfrom
putzwasser:extend-eel-datehelper

Conversation

@putzwasser
Copy link
Copy Markdown

@putzwasser putzwasser commented Apr 28, 2018

Adds methods which format a DateTime into machine-readable formats to use it e.g. in the datetime attribute of a <time> tag.

  • htmlDateTime formats a DateTime into the W3C format "Y-m-d\TH:i:sP"
  • htmlDate formats the date part of a DateTime into "Y-m-D"
  • htmlTime formats the time part of a DateTime into "H:i:sP"

@putzwasser
FEATURE: Extends Eel DateHelper with machine-readable formats
363fdcb 
Adds methods which format a DateTime into machine-readable formats to use it e.g. in the `datetime` attribute of a `<time>` tag.

* htmlDateTime formats a DateTime into the W3C format "Y-m-d\TH:i:sP"
* htmlDate formats the date part of a DateTime into "Y-m-D"
* htmlTime formats the time part of a DateTime into "H:i:sP"
bwaidelich
bwaidelich reviewed May 16, 2018
View reviewed changes
Neos.Eel/Classes/Helper/DateHelper.php
*/
public function htmlDateTime(\DateTimeInterface $dateTime)
{
return $dateTime->format(\DateTime::W3C);
Copy link
Copy Markdown
Member

@bwaidelich bwaidelich May 16, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a particular reason why you chose the \DateTime:W3C format?
I know that we use this as default in the DateTimeConverter but this was a mistake IMO because it lacks some crucial features.

@bwaidelich
Copy link
Copy Markdown
Member

bwaidelich commented May 16, 2018

@putzwasser Thanks a lot and congratulations to your first Pull Request!
I'm a bit skeptical though to be honest because this seem to use rather random formats (unless I miss something here).
Did you know that you can achieve the above already by using the 2nd argument of the format helper?

timeTag = Neos.Fusion:Tag {
  tagName = 'time'
  attributes.datetime = ${Date.format(someDate, 'Y-m-d\TH:i:sO')}
  content = ${Date.format(someDate, 'Y-m-d')}
}

(untested)

@putzwasser
Copy link
Copy Markdown
Author

putzwasser commented May 21, 2018

Thanks for your comment.
Actually, the formats are not random. The output can be used in time tags—or the datetime attribute to be more precise. Basically they are shorthands to get machine readable datetime/date/time formats.
So, yes one could simply use Date.format but since there are shorthands for hour/minute/second i figured this could be handy because Y-m-d\TH:i:sO is easy to be confused with d/m/Y\TH:i:sO and other possibilities to format a date.
At least it's easier to confuse than e.g. Date.format('H') and this has the Date.hour(datetime) shortcut ;)

albe
albe reviewed Oct 13, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@albe albe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the possible formats for the time tag, I'm a bit unsure about how useful it is to add specialized helpers for three formats. Why not also add one for year, month, time without fraction/seconds and local datetime? Those are all equally valid and probable use cases. I'd rather not increase API surface needlessly.

https://developer.mozilla.org/en-US/docs/Web/HTML/Element/time#Valid_datetime_Values

@stale
Copy link
Copy Markdown

stale bot commented May 30, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions!

@stale stale bot added the Stale Issues and PRs with no recent activity, about to be closed soon. label May 30, 2019
@stale
Copy link
Copy Markdown

stale bot commented Jun 13, 2019

This issue has been automatically closed because it has not had activity for some time. But that does not need to be final. If you find the time to work on it, feel free to open it again. Thanks again for your contributions!

@stale stale bot closed this Jun 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bwaidelich bwaidelich bwaidelich left review comments

+1 more reviewer

@albe albe albe left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Stale Issues and PRs with no recent activity, about to be closed soon.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@putzwasser @bwaidelich @albe