- Issue created by @Eduardo Morales Alberti
- Status changed to Needs review
3 months ago 11:17am 10 April 2024 - 🇪🇸Spain omarlopesino
It looks good to me. I've added a question to the MR suggesting an optimization that could be done. Waiting for a reply before deciding merging or moving to needs work. Thanks!
- 🇪🇸Spain frouco
@mistermoper @EduardoMoralesAlberti
I think it will be better to move this feature to a submodule:
- Declare dependencies properly (colorbox)
- YT core code is simpler, containing only what is needed.
- Status changed to Needs work
2 months ago 8:22am 26 April 2024 - 🇪🇸Spain Eduardo Morales Alberti Spain, 🇪🇺
Pending to move the Javascript to a submodule.
- 🇪🇸Spain omarlopesino
I agree with @frouco. I didn't mention that because it is also coupled with the embed integration which doesn't need to be present in every Drupal site. But the Oembed part can be decoupled in a different issue.
To do that I suggest dispatching an event here: https://git.drupalcode.org/project/youtube_cookies/-/merge_requests/31/d... , so that the submodule can hook into the event and show the popup when the cookies are not accepted.
- Status changed to Needs review
2 months ago 9:41am 29 April 2024 - 🇪🇸Spain Eduardo Morales Alberti Spain, 🇪🇺
Moved colorbox to new submodule, ready to review.