Conversation
b7c7159 to
4da188d
Compare
|
Hi, Symfony UX 3.0 has been released, and the Could you please retarget this PR to the Thanks! |
bb5ab31 to
13a1fc5
Compare
📊 Packages dist files size differenceThanks for the PR! Here is the difference in size of the packages dist files between the base branch and the PR.
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Kocal
left a comment
There was a problem hiding this comment.
Thanks for working on this component!
Please ensure to read the whole review before starting anything, my mind changed during the review and I'm sure we don't need a dedicated route (and everything related) for downloading a .ics file.
Thanks you!
| $builder->setParameter('ux_calendar_link.ics.data_uri_threshold', $config['ics']['data_uri_threshold']); | ||
| $builder->setParameter('ux_calendar_link.ics.ttl', $config['ics']['ttl']); | ||
| $builder->setParameter('ux_calendar_link.ics.download_route.enabled', $config['ics']['download_route']['enabled']); | ||
| $builder->setParameter('ux_calendar_link.ics.download_route.path', $config['ics']['download_route']['path']); | ||
|
|
||
| if ($config['ics']['download_route']['enabled']) { | ||
| $builder->setAlias('ux_calendar_link.ics.event_store', 'ux_calendar_link.ics.event_store.cache'); | ||
| $builder->removeDefinition('ux_calendar_link.ics.event_store.in_memory'); | ||
| } else { | ||
| $builder->removeDefinition('ux_calendar_link.ics.event_store.cache'); | ||
| } |
|
Thank you @Kocal for this great review and suggestions, indeed there are bunch of things to simplify here , I will take care of this |
|
The package must also be added in the subtree split configuration file, at the repo's root |
405061c to
8348232
Compare
I did a large cleanup here and the CI is green. |
| $services = $container->services() | ||
| ->defaults() | ||
| ->autowire() | ||
| ->autoconfigure() | ||
| ->private(); |
There was a problem hiding this comment.
We do not configure services defaults in bundles:
| $services = $container->services() | |
| ->defaults() | |
| ->autowire() | |
| ->autoconfigure() | |
| ->private(); | |
| $services = $container->services(); |
| **Symfony UX Calendar Link** generates "Add to calendar" links for Google | ||
| Calendar, Outlook.com, Office 365 and iCalendar (``.ics``) — the format | ||
| consumed by Apple Calendar, Outlook desktop, Thunderbird and every native | ||
| calendar client. |
There was a problem hiding this comment.
| **Symfony UX Calendar Link** generates "Add to calendar" links for Google | |
| Calendar, Outlook.com, Office 365 and iCalendar (``.ics``) — the format | |
| consumed by Apple Calendar, Outlook desktop, Thunderbird and every native | |
| calendar client. | |
| Symfony UX CalendarLink is a Symfony bundle that allows generator of "Add to calendar" links for Google | |
| Calendar, Outlook.com, Office 365 and iCalendar (``.ics``), the format | |
| consumed by Apple Calendar, Outlook desktop, Thunderbird and every native | |
| calendar client. |
| It addresses the long-standing request in | ||
| `symfony/ux#3308 <https://github.com/symfony/ux/issues/3308>`_ for a | ||
| first-party "Add to calendar" component in the Symfony UX initiative. | ||
|
|
| .. code-block:: terminal | ||
|
|
||
| $ composer require symfony/ux-calendar-link | ||
|
|
||
| If you're not using Symfony Flex, enable the bundle manually: | ||
|
|
||
| :: | ||
|
|
||
| // config/bundles.php | ||
| return [ | ||
| // ... | ||
| Symfony\UX\CalendarLink\UXCalendarLinkBundle::class => ['all' => true], | ||
| ]; |
There was a problem hiding this comment.
| .. code-block:: terminal | |
| $ composer require symfony/ux-calendar-link | |
| If you're not using Symfony Flex, enable the bundle manually: | |
| :: | |
| // config/bundles.php | |
| return [ | |
| // ... | |
| Symfony\UX\CalendarLink\UXCalendarLinkBundle::class => ['all' => true], | |
| ]; | |
| Install the bundle using Composer and Symfony Flex: | |
| .. code-block:: terminal | |
| $ composer require symfony/ux-turbo | |
| A note on Apple Calendar and EventKit | ||
| ------------------------------------- | ||
|
|
||
| Apple's EventKit (``EKEvent`` / ``EKEventStore``) is a **native iOS/macOS | ||
| API**, not a web URL scheme. There is no proprietary "Add to Apple Calendar" | ||
| deeplink. The correct way to target Apple Calendar from a web page is to | ||
| serve an RFC 5545 ``.ics`` file — Safari on iOS and macOS recognize the | ||
| ``text/calendar`` MIME type and offers to add the event directly. The ``ics`` | ||
| provider covers Apple Calendar, Outlook desktop, Thunderbird and every other | ||
| native calendar client in a single link. | ||
|
|
||
| ICS delivery | ||
| ------------ | ||
|
|
||
| The ``ics`` provider always returns a | ||
| ``data:text/calendar;charset=utf-8;base64,...`` URL containing the full | ||
| VCALENDAR payload. |
There was a problem hiding this comment.
I don't think these paragraphs are useful
| Reminders and recurrence | ||
| ------------------------ | ||
|
|
||
| :: |
| accepting the usual ``interval``, ``count`` and ``until`` named arguments: | ||
|
|
||
| :: |
There was a problem hiding this comment.
Having :: alone is nonsense:
| accepting the usual ``interval``, ``count`` and ``until`` named arguments: | |
| :: | |
| accepting the usual ``interval``, ``count`` and ``until`` named arguments:: |
| CalendarRecurrence::daily(interval: 2); // every other day | ||
| CalendarRecurrence::monthly(count: 6); // six monthly occurrences | ||
| CalendarRecurrence::yearly(until: new \DateTimeImmutable('2030-01-01')); |
There was a problem hiding this comment.
| CalendarRecurrence::daily(interval: 2); // every other day | |
| CalendarRecurrence::monthly(count: 6); // six monthly occurrences | |
| CalendarRecurrence::yearly(until: new \DateTimeImmutable('2030-01-01')); | |
| // every other day | |
| CalendarRecurrence::daily(interval: 2); | |
| // six monthly occurrences | |
| CalendarRecurrence::monthly(count: 6); | |
| // ... | |
| CalendarRecurrence::yearly(until: new \DateTimeImmutable('2030-01-01')); |
| All-day events | ||
| -------------- | ||
|
|
||
| :: |
74889b4 to
c83ae65
Compare
c83ae65 to
3920086
Compare
Kocal
left a comment
There was a problem hiding this comment.
Thanks for taking care about my previous comments, here some new ones.
Overall it looks fine, but I still want to take some time to try it inside a real project.
Thanks!
| Usage | ||
| ----- | ||
|
|
||
| Start by creating a CalendarEvent object in your controller. Once populated with your event details, pass it to Twig to render the calendar links:: |
There was a problem hiding this comment.
| Start by creating a CalendarEvent object in your controller. Once populated with your event details, pass it to Twig to render the calendar links:: | |
| Start by creating a ``CalendarEvent`` object in your controller. Once populated with your event details, pass it to Twig to render the calendar links:: |
| start: new \DateTimeImmutable('2026-05-14 09:00'), | ||
| end: new \DateTimeImmutable('2026-05-15 18:00'), | ||
| location: 'Cité Universitaire Paris', | ||
| description: 'Annual Symfony conference', |
There was a problem hiding this comment.
| description: 'Annual Symfony conference', | |
| description: 'Annual Symfony conference in France', |
(since we also have SF Con, SF Day, and SF Live Berlin 😉)
| * ``ux_calendar_link(event, provider)`` — generates a link for one provider | ||
| * ``ux_calendar_links(event)`` — generates links for every registered provider |
There was a problem hiding this comment.
| * ``ux_calendar_link(event, provider)`` — generates a link for one provider | |
| * ``ux_calendar_links(event)`` — generates links for every registered provider | |
| * ``ux_calendar_link(event, provider)``: generates a link for one provider | |
| * ``ux_calendar_links(event)``: generates links for every registered provider |
PS: you can add this section to your AGENTS.md / skills :D
### Em-dash
Never use em-dashes (the `—` character). Use parentheses, commas, colons, or restructure the sentence instead.
| Bad | Good |
|-----|------|
| EN: "The tool — which was released last year — is now stable." | EN: "The tool (released last year) is now stable." |
| EN: "This has one major benefit — speed." | EN: "This has one major benefit: speed." |
| FR: "L'outil — sorti l'an dernier — est maintenant stable." | FR: "L'outil, sorti l'an dernier, est maintenant stable." |
| ``CalendarRecurrence`` exposes one static factory per RFC 5545 frequency — | ||
| ``minutely()``, ``daily()``, ``weekly()``, ``monthly()``, ``yearly()`` — each | ||
| accepting the usual ``interval``, ``count`` and ``until`` named arguments: |
There was a problem hiding this comment.
| ``CalendarRecurrence`` exposes one static factory per RFC 5545 frequency — | |
| ``minutely()``, ``daily()``, ``weekly()``, ``monthly()``, ``yearly()`` — each | |
| accepting the usual ``interval``, ``count`` and ``until`` named arguments: | |
| ``CalendarRecurrence`` exposes one static factory per RFC 5545 frequency: | |
| ``minutely()``, ``daily()``, ``weekly()``, ``monthly()``, ``yearly()``, each | |
| accepting the usual ``interval``, ``count`` and ``until`` named arguments: |
| private function uid(CalendarEvent $event): string | ||
| { | ||
| $hash = hash('xxh128', implode('|', [ | ||
| $event->title, | ||
| $event->start->format(\DateTimeInterface::ATOM), | ||
| $event->end->format(\DateTimeInterface::ATOM), | ||
| $event->location ?? '', | ||
| ])); | ||
|
|
||
| return $hash.'@ux-calendar-link.symfony'; | ||
| } |
There was a problem hiding this comment.
I don't really like this method, as two similar events with different description/url/... will have the same generated hash, which is wrong.
And as documented by https://icalendar.org/New-Properties-for-iCalendar-RFC-7986/5-3-uid-property.html, it's recommended to use a UUID to generate the UID field.
There was a problem hiding this comment.
| $this->assertStringContainsString("BEGIN:VCALENDAR\r\n", $ics); | ||
| $this->assertStringContainsString("DTSTART:20260514T090000Z\r\n", $ics); | ||
| $this->assertStringContainsString("DTEND:20260514T100000Z\r\n", $ics); | ||
| $this->assertStringContainsString("SUMMARY:Demo\r\n", $ics); | ||
| $this->assertStringContainsString("END:VCALENDAR\r\n", $ics); |
There was a problem hiding this comment.
Can't we assert the whole string instead?
| EXPERIMENTAL This component is currently experimental and is likely to | ||
| change, or even change drastically. |
There was a problem hiding this comment.
| EXPERIMENTAL This component is currently experimental and is likely to | |
| change, or even change drastically. | |
| **EXPERIMENTAL** This component is currently experimental and is | |
| likely to change, or even change drastically. |
There was a problem hiding this comment.
(please mimic the other README.md files)
| ## Basic usage | ||
|
|
||
| ```php | ||
| use Symfony\UX\CalendarLink\CalendarEvent; | ||
|
|
||
| $event = new CalendarEvent( | ||
| title: 'Symfony Live Paris', | ||
| start: new \DateTimeImmutable('2026-05-14 09:00'), | ||
| end: new \DateTimeImmutable('2026-05-15 18:00'), | ||
| location: 'Disneyland Paris', | ||
| description: 'Annual Symfony conference', | ||
| ); | ||
|
|
||
| return $this->render('event/show.html.twig', ['event' => $event]); | ||
| ``` | ||
|
|
||
| ```twig | ||
| <a href="{{ ux_calendar_link(event, 'google').url }}">Add to Google Calendar</a> | ||
|
|
||
| {% for link in ux_calendar_links(event) %} | ||
| <a href="{{ link.url }}">{{ link.label }}</a> | ||
| {% endfor %} | ||
| ``` |
| This repository is a READ-ONLY sub-tree split. See https://github.com/symfony/ux | ||
| to create issues or submit pull requests. |
There was a problem hiding this comment.
| This repository is a READ-ONLY sub-tree split. See https://github.com/symfony/ux | |
| to create issues or submit pull requests. | |
| **This repository is a READ-ONLY sub-tree split**. See | |
| https://github.com/symfony/ux to create issues or submit pull requests. |
| change, or even change drastically. | ||
|
|
||
| Symfony UX Calendar Link generates "Add to calendar" links for Google | ||
| Calendar, Outlook.com, Office 365 and iCalendar (`.ics`) — the format |
There was a problem hiding this comment.
| Calendar, Outlook.com, Office 365 and iCalendar (`.ics`) — the format | |
| Calendar, Outlook.com, Office 365 and iCalendar (`.ics`), the format |
| <?xml version="1.0" encoding="UTF-8"?> | ||
|
|
||
| <phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" | ||
| xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/9.5/phpunit.xsd" | ||
| backupGlobals="false" | ||
| colors="true" | ||
| bootstrap="tests/bootstrap.php" | ||
| failOnRisky="true" | ||
| failOnWarning="true" | ||
| > | ||
| <php> | ||
| <ini name="error_reporting" value="-1"/> | ||
| <server name="KERNEL_CLASS" value="Symfony\UX\CalendarLink\Tests\Fixtures\TestKernel"/> | ||
| </php> | ||
|
|
||
| <testsuites> | ||
| <testsuite name="symfony/ux-calendar-link Test Suite"> | ||
| <directory>./tests/</directory> | ||
| </testsuite> | ||
| </testsuites> | ||
|
|
||
| <coverage> | ||
| <include> | ||
| <directory>./src</directory> | ||
| </include> | ||
| </coverage> | ||
| </phpunit> |
There was a problem hiding this comment.
This is what PHPUnit configuration file from other UX packages looks like:
| <?xml version="1.0" encoding="UTF-8"?> | |
| <phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" | |
| xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/9.5/phpunit.xsd" | |
| backupGlobals="false" | |
| colors="true" | |
| bootstrap="tests/bootstrap.php" | |
| failOnRisky="true" | |
| failOnWarning="true" | |
| > | |
| <php> | |
| <ini name="error_reporting" value="-1"/> | |
| <server name="KERNEL_CLASS" value="Symfony\UX\CalendarLink\Tests\Fixtures\TestKernel"/> | |
| </php> | |
| <testsuites> | |
| <testsuite name="symfony/ux-calendar-link Test Suite"> | |
| <directory>./tests/</directory> | |
| </testsuite> | |
| </testsuites> | |
| <coverage> | |
| <include> | |
| <directory>./src</directory> | |
| </include> | |
| </coverage> | |
| </phpunit> | |
| <?xml version="1.0" encoding="UTF-8"?> | |
| <!-- https://phpunit.de/manual/current/en/appendixes.configuration.html --> | |
| <phpunit | |
| xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" | |
| xsi:noNamespaceSchemaLocation="vendor/phpunit/phpunit/phpunit.xsd" | |
| colors="true" | |
| bootstrap="tests/bootstrap.php" | |
| failOnDeprecation="true" | |
| failOnRisky="true" | |
| failOnWarning="true" | |
| cacheDirectory=".phpunit.cache" | |
| > | |
| <php> | |
| <ini name="error_reporting" value="-1"/> | |
| <env name="SHELL_VERBOSITY" value="-1"/> | |
| <env name="KERNEL_CLASS" value="Symfony\UX\CalendarLink\Tests\Fixtures\TestKernel" /> | |
| </php> | |
| <testsuites> | |
| <testsuite name="Symfony UX CalendarLink Test Suite"> | |
| <directory>./tests</directory> | |
| </testsuite> | |
| </testsuites> | |
| <source | |
| ignoreSuppressionOfDeprecations="true" | |
| ignoreIndirectDeprecations="true" | |
| restrictNotices="true" | |
| restrictWarnings="true" | |
| > | |
| <include> | |
| <directory>src</directory> | |
| </include> | |
| <deprecationTrigger> | |
| <function>trigger_deprecation</function> | |
| </deprecationTrigger> | |
| </source> | |
| </phpunit> |
This new component address the #3308 idea , it provide twig functions as proposed by (@Kocal) with an easy way to configure calendar links , it supports Google Calendar, Microsoft Outlook and Ics.
I did not include a dropdown in the component since we can use any other dropdown from any Kit or custom Dropdown.