urfuwo / hlx-test

Apache License 2.0
0 stars 3 forks source link

Video with Placeholder image #173

Open mhaack opened 9 months ago

mhaack commented 9 months ago

We need support for video (youtube) player with a placeholder image which are used on todays news pages.

This is implemented via the embed block we can take over from the block collection: https://www.aem.live/developer/block-collection/embed

Sample pages:

See Figma https://www.figma.com/file/oSetT4LbatRmXlcB2A7V8Z/Content-Hubs-2024?type=design&node-id=699-32259&mode=design&t=oGjZrklt5hCErBMt-4

mlangfordsap commented 9 months ago

Updated per conversation with UX team

Needs that are out of scope for this ticket

  1. We need a modal implementation.
  2. We need to investigate Analytics on the YouTube player. It has a robust event model, but I'm not sure if the analytics team has bandwidth to build that if it doesn't already exist.
mhaack commented 9 months ago

Todays news videos are getting imported as embed block already with placeholder image. But only if there is a placeholder image today. Otherwise they get imported as links, this is better for authoring experience. In scripts.js we can change the behaviour and generate embed blocks for links only as well, if needed.

mhaack commented 9 months ago

For self-hosted videos I suggest to create a separate ticket.

mlangfordsap commented 9 months ago

The UX team expressly said they did not want links that were added in-line to the text be embedded - those have to open in a modal (out of scope for this ticket). Embedded video is to be an author-specified situation. The only place that uses YouTube is News, and YouTube videos aren't actually allowed on sap.com so the way News does it now will eventually need to be addressed by Governance, even if that means a sort of phase-out runway. That's not something I want to get in the middle of, but since laying groundwork now isn't hard and knowing that any videos we need for the Insights content will NOT be on YouTube, means we need to be prepared for it.

The DAM-based video player should be in another ticket as that is a custom-built tool that I'll probably need EPAM help to isolate from sap.com, but the framework to allow for multiple embeds is easy enough to implement using the pattern established in the AEM block-collection library even if the actual embed code isn't yet available.

In any case, I could simplify the block in Word to be a full link instead (the existing form came from early interference from the script.js implementation). That's easy enough to walk back since the source can be inferred from the links.

@benpeter

benpeter commented 9 months ago

we'll look at sap.com when we get there. part of the edge delivery philosophy is to remain open but not implement anticipations, rather wait for the manifestation.

couple points

anything that speaks against that approach @mlangfordsap ?

mlangfordsap commented 9 months ago

That sounds right. Video played as an embed uses a Video Embed block as we discussed last night, the inline stuff via modal should be implemented via separate ticket. I don't think this is a case of anticipating - Angie said pretty specifically that their video content isn't in YouTube so that's a requirement that has now manifested (even if we haven't documented it yet). That implementation is out of scope for this ticket, but since the EMBEDS_CONFIG pattern already exists in the block-collection example, I don't see any reason not to include that for now, even if it's not used, since it would be silly to use two different blocks for video embedding when we don't need to. That means this ticket needs to be done and merged before a subsequent SAP DAM implementation can start (I don't see a problem with that if we consider the DAM implementation an iteration on existing video embed features)

mlangfordsap commented 9 months ago

This has been discussed to death in Slack, but I'm documenting this here as well for reference.

Final scope is as follows:

  1. No discrete video embed block is needed.
  2. No placeholder images will be used.
  3. We will implement YouTube and LinkedIn video options.
  4. Links that contain either youtube.com, youtu.be, or linkedin.com hostnames will be converted to an inline embedded video player. All other video use-cases (SAP player, player in modal, etc) is out of scope for this particular issue.
  5. Links placed inline in text is out of scope for this issue.