ampproject / amphtml

The AMP web component framework.
https://amp.dev
Apache License 2.0
14.89k stars 3.89k forks source link

✨ Add TargetVideo Alias To Brid AMP Player #40151

Closed danijel-ristic closed 2 months ago

danijel-ristic commented 2 months ago

Closes #40130

Talked to team and decided to go other way around then the issue suggests.

powerivq commented 2 months ago

@danijel-ristic Validator test failed. You need to update https://github.com/ampproject/amphtml/blob/main/extensions/amp-brid-player/validator-amp-brid-player.protoascii to have the validator recognize the new tag.

Also I will suggest you to revert changes to examples/brid-player.amp.html for now. Once the validator is updated and published, you will be able to change the example then.

danijel-ristic commented 2 months ago

@powerivq Done.

powerivq commented 2 months ago

Adding @banaag for validator rule approval.

powerivq commented 2 months ago

@danijel-ristic can you try doing a rebase? It should fix the failed test.