Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[amp-story] Publisher identity #33353

Closed
Enriqe opened this issue Mar 18, 2021 · 4 comments · Fixed by #33725
Closed

[amp-story] Publisher identity #33353

Enriqe opened this issue Mar 18, 2021 · 4 comments · Fixed by #33725

Comments

@Enriqe
Copy link
Contributor

Enriqe commented Mar 18, 2021

Describe the new feature or change to an existing feature you'd like to see

We would like to add support for adding publisher identity in stories. This would come in the form of a logo and publisher name that when clicked would take to the publisher's site.
IMG_1380

Requirements

  • This should be an opt-in at the player level, since we don't want to break existing implementations (both in players - creating double attribution, and in stories that already contain attribution baked into the story).
  • Should be updated for every story (stories could come from different publishers/entities inside the same player).

Source of data

[Selected] Option 1: Story's metadata

We will extract the publisher's logo and name from the existing required fields publisher and publisher-logo-src. If provided, we will use entity-url to navigate when clicking on the attribution. Otherwise, we will fallback to the document's canonical URL's domain.

<amp-story standalone
    title="Top 5 AMP stories"
    publisher="AMP Project"
    publisher-logo-src="https://amp.dev/icon.png"
    publisher-href="https://amp.dev"
    poster-portrait-src="portrait.jpg"
    poster-square-src="square.jpg"
    poster-landscape-src="landscape.jpg"
    entity-url="https://amp.dev/creator-profile.html"></amp-story>

Option 2: amp-story-player level markup

  • The metadata would be declared in the player's markup.
  • Not ideal since metadata would be spread between the amp-story and amp-story-player markup.
<amp-story-player layout="fixed" width="360" height="600">
  <a href="story1.html"
      publisher="AMP Project"
      publisher-logo-src="https://amp.dev/icon.png"
      publisher-href="https://amp.dev">
  </a>
  <a href="story2.html"
      publisher="AMP Project"
      publisher-logo-src="https://amp.dev/icon.png"
      publisher-href="https://amp.dev">
  </a>
</amp-story-player>

Option 3: Player's JSON config + APIs

Would require:

  • Introducing a player.updateControl() API to update the controls dynamically.
  • Adding support for "textContent" label in custom controls.
<amp-story-player layout="fixed" width="360" height="600">
  <script type="application/json">
    {
      "controls": [
        {
          "name": "logoIcon",
          "backgroundImageUrl": "https://amp.dev/icon.png",
          "position": "start"
        },
        {
          "name": "logoText",
          "textContent": "AMP Project",
          "position": "start"
        },
        {
          "name": "close",
          "position": "end"
        },
      ]
    }
  </script>
  <a href="story1.html"></a>
  <a href="story2.html"></a>
</amp-story-player>

<script>
  const player = document.body.querySelector("amp-story-player");
  const allPublisherDetails = [
    {
      icon: 'https://amp.dev/icon.png',
      text: 'AMP Project',
      url: 'https://amp.dev/'
    },
  ];
  let publisherDetails;
  function goToPublisher() {
    location.href = publisherDetails.url;
  }

  player.addEventListener('amp-story-player-logoIcon', goToPublisher);
  player.addEventListener('amp-story-player-logoText', goToPublisher);
  player.addEventListener('navigation', (event) => {
    // Parallel array to stories <a> tags in the player
    const publisherDetails = allPublisherDetails[event.index];
    player.updateControl('logoIcon', {
      backgroundImageUrl: publisherDetails.icon
    });
    player.updateControl('logoText', {
      backgroundImageUrl: publisherDetails.text
    });
  });
</script>

Tasks

The following high-level tasks must be completed:

@ampproject/wg-stories
@ampproject/wg-viewers

@gmajoulet
Copy link
Contributor

gmajoulet commented Mar 18, 2021

Option 2 and 3 can't work as we can't make a runtime feature available to only one player. It'd have to be a general Player API, nothing tied to how we configure the open source Player we distribute. We shouldn't make references to the amp-story-player in this ticket.
Option 1 has the advantage of having the data available within the Story, so the identity can be displayed on first render. This is a much better UX than Option 3, where swiping would not have the next story identity visible.

However, I believe the answer to this question should be in our PRD: do we want the Player to control the identity information, or each individual story? This is a product question and not an eng question, we can then adapt our design based on what's in the PRD.

About Option 1, do we want the only option to be linking to the publisher homepage? In other words, if Platform decides to use this on their Web Stories, would their only option be showing <a href="platform.com>Platform and never <a href="platform.com/user">User on Platform?

@newmuis
Copy link
Contributor

newmuis commented Mar 18, 2021

Thanks for chiming in, Gabriel!

Strong preference for option 1, though I think 2 and 3 can work (though, yes, they would be amp-story-player features, and not have anything to do with the story).

do we want the Player to control the identity information, or each individual story?

The intention is that the identity is per-story (i.e., there can be a mix of publishers in the same player). As for which code is actually rendering the information, I agree with the question (and cc @raovs). My personal opinion is that it would be a good precedent to set that the story controls it (that's why we ask for metadata). Letting the player control it is a great way for someone to fake ownership (which, of course there are other ways to do). I think of this as analogous to a YouTube embed pulling the creator information from YouTube, rather than letting you specify an arbitrary creator when embed it.

do we want the only option to be linking to the publisher homepage?

Again /cc @raovs, but I think it would make sense for us to allow them to link to the authoritative webpage for the publisher. We have both publisher and entity metadata; I think it makes sense to allow specifying a URL for both (a la publisher-href attribute in Option 1 above), but giving precedence to the entity's URL when specified. I don't know if we need to do enforcement on the URL besides making sure it's not nefarious (e.g. javascript:alert('hello world')).

@raovs
Copy link
Contributor

raovs commented Mar 18, 2021

Summarize recent discussions:

  • We'll focus on option Add commit hooks #1 for badging - pulling badge info from the publisher and entity metadata
  • We'll add a URL field to the metadata (TBD - one for publisher and one for entity or just a single badge field)
  • We'll have a few different possible layouts:
    • Badge positioning (start with badge in the top right, eventually support footer)
    • Badge layout (publisher, entity, mix of publisher and entity)
      • TBD if the badge layout is defined by the Story or the player
      • Final UX mocks in progress

@Enriqe
Copy link
Contributor Author

Enriqe commented Mar 19, 2021

We'll have a few different possible layouts:

Badge positioning (start with badge in the top right, eventually support footer)

nit: top left, right?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants