localgovdrupal / localgov_events

Events for LocalGov Drupal.
GNU General Public License v2.0
1 stars 0 forks source link

Refactor the events listing view to use a view modes #34

Open markconroy opened 3 years ago

markconroy commented 3 years ago

Most of our views use view modes to create their listings. The events view is using fields.

Can we reset this to use a teaser view mode instead? This will make theming much easier, and will also be more consistent with how the rest of our distro is set up.

willguv commented 3 years ago

Not sure who this question is to. There’s nothing from a product point of view that would dictate a special way of building this format. It’s much better if everything is consistent.

Might be a historical thing. Events was built in the last couple of sprints of Alpha, and influenced by Lambeth’s events work. Don’t know if any code was reused.

andybroomfield commented 3 years ago

I think there is an issue with the recurring date field and displaying the correct date which is why events uses fields instead.

markconroy commented 3 years ago

@andybroomfield Thanks for that info. I'd like to perhaps explore that a bit more at one of the technical group meetings. I'm not sure why views would output the correct field any better than a view mode would.

ekes commented 3 years ago

iirc it's because it makes a join on a table of the calculated repetitions in views. There's presently no way of passing the context to the rendered entity, only to the view field which is actually 'from' that repeat table. Making a way to pass context would also be handy for displaying different dates on the node page itself (at the moment it's either the first, or the next, with optional repeat description).

markconroy commented 3 years ago

I think we have a juicy topic for the next Technical Group meeting!

markconroy commented 2 years ago

Thinking about this again, I'd like us to at least put a wrapper around the fields, probably with an article tag.

Then at least we'd have something like this:

<article class="teaser">
  {{ field_1 }}
  {{ field_2 }}
  {{ field_3 }}
</article>

Also, a class of .lgd-teaser-list on the view (for inheriting default layout).

And finally, attach the teaser library to the view (so the CSS is picked up).

andybroomfield commented 2 years ago

Was this ever picked up? Still seems strange to have to use fields (I know its to support the recurring date with the right date).

markconroy commented 2 years ago

It hasn't been picked up yet. I don't mind us using fields to be honest (well, I kind of do, but ...).

I'll try get a PR created soon for at least rewriting the output we get from fields to mimic our teaser template markup. That will make things a little more maintainable.

markconroy commented 2 years ago

Actually, now that I think of it a bit more, views has a "force using fields" checkbox when you use a view mode.

This means we can use a view mode for the rendered out put, but fields within the view itself. That would be worth looking into.