Skip to content
This repository has been archived by the owner on Nov 3, 2023. It is now read-only.

Discrepancy with $this->date between EventReader and EventList #8012

Closed
fritzmg opened this issue Sep 14, 2015 · 19 comments
Closed

Discrepancy with $this->date between EventReader and EventList #8012

fritzmg opened this issue Sep 14, 2015 · 19 comments
Assignees
Labels
Milestone

Comments

@fritzmg
Copy link
Contributor

fritzmg commented Sep 14, 2015

All the event templates use the variable $this->date. However, the Event modules do not fill this variable with the same values. While the EventList module fills this variable with just the date, e.g. 2015-09-14 the EventReader module on the other hand fills this variable with the date wrapped within an HTML <time> tag, e.g.

<time datetime="2016-05-23T00:00:00+02:00">2016-05-23</time>

So, if you select the default event_list template in the EventReader (for whatever reason) it will produce invalid HTML:

<h2><a href="" title="The Contao NRW-Day (<time datetime="2016-05-23T00:00:00+02:00">2016-05-23</time>)"></a></h2>

This is also reproducible in the Contao Online Demo.

The reason why the EventReader module produces a different value for $this->date is probably because the default event_full template has this line in it:

<p class="info"><?= $this->date ?></p>

which in the end will produce

<p class="info"><time datetime="2016-05-23T00:00:00+02:00">2016-05-23</time></p>

But in the teaser and list template, the variable is used within the title attribute of the link.

See also 3.5.3 Event Reader zeigt nur Teaserinhalt und seltsame Zeichen

@leofeyer leofeyer added this to the 3.5.4 milestone Sep 15, 2015
@leofeyer
Copy link
Member

leofeyer commented Oct 2, 2015

Although this is easy to fix, I wonder if the problem will ever really occur. If you chose any event_ template but event_full in the event reader module, the output will be broken anyway (e.g. the href attribute will be empty, there will be no link text etc.).

@leofeyer
Copy link
Member

leofeyer commented Oct 2, 2015

@contao/developers /cc

@aschempp
Copy link
Member

aschempp commented Oct 5, 2015

Unifying the templates would certainly be a good thing, but I wouldn't consider this a bugfix…

@leofeyer
Copy link
Member

leofeyer commented Oct 5, 2015

It is a bugfix without a doubt. The question is if this case will ever really occur, because if you chose any event_ template but event_full in the event reader module, the output will be broken anyway (e.g. the href attribute will be empty, there will be no link text etc.).

@aschempp
Copy link
Member

aschempp commented Oct 5, 2015

That's what I mean. Fixing the date only does not make sense for the moment. Adjusting the reader and list so all templates work with all modules makes sense, but I would not consider that a bugfix.

@leofeyer
Copy link
Member

leofeyer commented Oct 5, 2015

Adjusting the reader and list so all templates work with all modules makes sense

It does not make sense at all, because a list template has different requirements than a reader template!

@aschempp
Copy link
Member

aschempp commented Oct 5, 2015

In my opinion both show an event. Maybe people want to show the full event details in a list.

@Toflar
Copy link
Member

Toflar commented Oct 5, 2015

Maybe people want to show the full event details in a list.

Happened often to me.

@leofeyer
Copy link
Member

leofeyer commented Oct 5, 2015

I it really happened often, you would have noticed that it is not possible :)

@fritzmg
Copy link
Contributor Author

fritzmg commented Oct 5, 2015

@Toflar @aschempp The problem isn't using the event_full template in an event list (this won't cause any invalid html at least), but the other way around.

However, I also think it's very rare that someone wants to use the event_list template in the event reader.

I would consider it a known limitation right now and maybe fix it for Contao 5?

@aschempp
Copy link
Member

aschempp commented Oct 5, 2015

I it really happened often, you would have noticed that it is not possible :)

Well one just needs to rewrite the template in that case. Guess we always did that :D

@leofeyer
Copy link
Member

leofeyer commented Oct 5, 2015

Guess we always did that :D

No, I guess you never had this case before. See @fritzmg's comment to understand which case we are discussing:

use the event_list template in the event reader.

@leofeyer
Copy link
Member

Fixed in a6c674f.

@xchs
Copy link
Contributor

xchs commented Nov 25, 2015

How do we handle date ranges (e.g. 25/11/2015 – 30/11/2015)? We might want to mark up the date/time with two separate <time> elements:

<time datetime="2015-11-25">25/11/2015</time><time datetime="2015-11-30">30/11/2015</time>

@leofeyer
Copy link
Member

That would be wrong, because the <time> tag indicates the start date of an event. Only schema.org allows to distinguish between itemprop="startDate" and itemprop="endDate".

@aschempp
Copy link
Member

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

This element is intended to be used presenting dates and times in a machine readable format. This can be helpful for user agents to offer any event scheduling for user's calendar.

I can't see any reason why it should not be used multiple times?

@leofeyer
Copy link
Member

It can be used multiple times but it should not be used for both the event start date and end date without additional markup such as schema.org.

@aschempp
Copy link
Member

Ok, so how about we add that markup? Would make sense anyway?

@leofeyer
Copy link
Member

Yes, but that's a separate ticket then (feature request in https://github.com/contao/calendar-bundle). We will have to add some more variables to the template and move all of this logic to the template. Not nice but possible.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

5 participants