search

localgovdrupal / localgov_base

The base theme for LocalGov Drupal websites.
8 stars 15 forks source link

adds add-to-calendar functionality for events #546

Closed markconroy closed 3 months ago

markconroy commented 5 months ago

Closes #532

=== Thanks to Big Blue Door for sponsoring my time to work on this.

msayoung commented 5 months ago

I notice testing on FF and Chrome with NVDA screenreader that when the dialog opens the entire thing is read out. I think because focus is moving to the dialog rather than a component within it. But it may just be an issue with NVDA.

https://adrianroselli.com/2020/10/dialog-focus-in-screen-readers.html

The dialog probably wants a name of some sort. A heading, or an aria-label or aria-labelledby would be good.

markconroy commented 4 months ago

@ekes

ical uses CRLF line endings so it should be \r\n instead of \n but then that doesn't mean the same thing as a line break. So text might will end up in a single line.

Any preference on what we do here? \r\n or \n? I guess we want to be right, so \r\n but we also don't want everything on one line, so \n. My preference would be to leave it as is if it's working and figure out something better if a problem presents itself.

Any text including HTML should be an additional version using ALTREP

I'm not fully sure exactly what we need to do here? I don't like having the body field used as the event description. So perhaps it's best to just remove that section? I can imagine an event with a very long body field being not exactly what a user is expecting in their calendar. Maybe we should just use the summary field (which is just plain text with no HTML) as our value here instead?

markconroy commented 4 months ago

Should we merge this? The only thing outstanding is whether to use twig or PHP to construct our URLs. This seems more like an enhancement than a blocker to me.

finnlewis commented 3 months ago

Discussing this in Merge Tuesday:

Enabling by default on existing sites, for sites that have not overridden this template, will automatically add the button to 'Add to calendar' to event node full view.

We're happy with this (@willguv, @markconroy, @ekes, @stephen-cox, @FinnERLewis )

@andybroomfield would we consider moving this to the events module, to a pseudo field? This can be a follow up issue.