Closed shushugah closed 8 months ago
Name | Link |
---|---|
Latest commit | 0b085ad003376f3321ceea3e96e105dab19fe053 |
Latest deploy log | https://app.netlify.com/sites/techworkersberlin/deploys/65d7d3dbfc67380008d7c404 |
Deploy Preview | https://deploy-preview-273--techworkersberlin.netlify.app |
Preview on mobile | Toggle QR Code...Use your smartphone camera to open QR code link. |
To edit notification comments on pull requests, go to your Netlify site configuration.
Generally, I think it's a good idea.
However, I think it looks odd when the first events are in the past.
Maybe only show events from today onwards?
And if there are no such events, show past events in reverse order?
@rxxx03 since Jekyll timestamps are run at build time, let's assume for this example that website was built on 21 February.
Since the website isn't automatically updated each day (we could...trigger a daily rebuild job) what is considered future/or past...is dependent on build timestamp, not actual dynamic time. But do scenarios 1-4 make sense as is? If so, you can finish code review and approve merge request.
but we could also say, only filter the future events with "earliest event", otherwise show the "latest event" on top, regardless of whether it's limited or not?
I think this addresses the 4 scenarios. In case you'd prefer to show order by "earliest" event, regardless of whether there's future events or not, but also filter future events when they exist, below code would address that
{% assign events = site.events | filter_tags: include.tags | reverse %}
{% if include.limit %}
{% assign future_events = events | where_exp: "event", "event.date >= site.time" %}
{% if future_events.size > 0 %}
{% for post in future_events reversed limit: include.limit %}
{% include event-card.html %}
{% endfor %}
{% else %}
{% for post in events reversed limit: include.limit %}
{% include event-card.html %}
{% endfor %}
{% endif %}
{% else %}
{% for post in events %}
{% include event-card.html %}
{% endfor %}
{% endif %}
Hi Yonatan, I wanted to play around with it a bit, e.g. to see if it's possible to get the current date by javascript. But there seems to be a problem:
Ok, looks good now.
I made one more small comment on the code.
But I like the new sorting: if there are events in the future, show only those, in ascending order; and if there are only events in the past show them descending order. <3
If there is no limit specified, show the latest events on top; if there is a limit filter (e.g limit: 5) query the latest events, limit them and then again reverse the order, for easier chronological reading.
Currently on the home page, as well as /learning and /works-councils the latest event was shown on top, even though it might be more intuitive to show the oldest events first (when there's a small number).
The changes
Before this PR on home page (7 May on top)
After this PR on home page (7 May on bottom)
The /events remains exactly the same as before