🚮 Remove <amp-story>-specific check for <amp-audio> actions
#35643
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
caroqliu
approved these changes
Aug 12, 2021
gmajoulet
approved these changes
Aug 12, 2021
zoharnyego
added a commit
to apester-dev/amphtml
that referenced
this pull request
Aug 12, 2021
commit b9e41a2 Author: Ryan Cebulko <ryan@cebulko.com> Date: Thu Aug 12 16:47:01 2021 -0400 ♻️ Define helper for hash params and drop barely-used `getMode().log` (ampproject#35628) * Add getHashParams helper * Add win param * Update mode.js to use helper * Add tests * Update callsites * Remove getMode().log uses * Remove log from ModeDef type * Lint fixes * Fix tests * Split up isModeDevelopment commit 0e66e28 Author: Alan Orozco <alanorozco@users.noreply.github.com> Date: Thu Aug 12 13:25:34 2021 -0700 🚮 Remove `<amp-story>` check for `<amp-audio>` actions (ampproject#35643) Removes check from `<amp-audio>` that prevents actions when descending from an `<amp-story>` element. `<amp-story>` now has an action allowlist, so the check was redundant. The action is still prevented, triggering the message '[Action] "AMP-AUDIO.play" is not allowlisted []'. commit 6b46842 Author: honeybadgerdontcare <honeybadgerdontcare@users.noreply.github.com> Date: Thu Aug 12 13:19:24 2021 -0700 Update link to modifications best practices (ampproject#35647) * Update link to modifications best practices * Update dead links commit a59d776 Author: Raghu Simha <rsimha@amp.dev> Date: Thu Aug 12 14:31:55 2021 -0400 🏗 Move Edge testing from GH Actions to CircleCI (ampproject#35630) commit b6476c3 Author: Khoi Doan <khoid@google.com> Date: Thu Aug 12 13:47:16 2021 -0400 ✨ Add bitness to User Agent Client Hint Params in ad requests (ampproject#35612) * Add User Agent Client Hint Params to Google Ads. * Update the param names. * Add timeout for UACH promise (1 second). * Use fake timers for timeout test. * Lint * Add exception for dependency on promise.js. * Add bitness field to User Agent Client Hints params * Fix merge * Fix merge again * Lint commit 87d5d14 Author: Ryan Cebulko <ryan@cebulko.com> Date: Thu Aug 12 13:43:51 2021 -0400 ♻️ Remove `version` from legacy mode object (ampproject#35624) * Remove `version` from legacy mode object * Restore version to mode-object for now * Remove version entirely * Lint fixes * Fix transformer test fixtures * Fix test commit 00b8e7e Author: Brandon Suen <brandonbsuen@google.com> Date: Thu Aug 12 10:31:56 2021 -0700 📖 [Story interactive] Add Documentation for Image Quizzes and Polls (ampproject#35618) * Documentation text added * Add images and source code * Reorganize in alphabetical order * Reordering * Add additional attributes * Fix typo * Fix typo * Fix typo * Update screenshots * Add note about image sizes * Revise note about image size commit 9813cea Author: Ryan Cebulko <ryan@cebulko.com> Date: Thu Aug 12 13:27:55 2021 -0400 ♻️ #core form, modal, amp-element-helpers ("real" diff in desc) (ampproject#35608) * Update src/form code * Update src/modal code * Update src/video-interface code * Move src/{form,modal,amp-element-helpers} -> #core/dom * Fix types * update dep-check config * Remove bad assert * Update imports of form,modal,amp-element-helpers * Fix rebase error commit 4ed4b92 Author: Philip Bell <philip.hunter.bell@gmail.com> Date: Thu Aug 12 12:38:05 2021 -0400 Null check on orientation lock. (ampproject#35639) commit 5ff32ad Author: Chris Antaki <ChrisAntaki@gmail.com> Date: Wed Aug 11 15:12:58 2021 -0700 SwG Release 0.1.22.179 (ampproject#35629) commit 2106624 Author: Matias Szylkowski <mszylkowski@google.com> Date: Wed Aug 11 17:13:54 2021 -0400 📖 [Story sidebar] Removed sidebar from docs (ampproject#35616) * removed sidebar and bookend docs * removed unused import * revert test removals * Add newline on visual test html commit d21ecc0 Author: Ryan Cebulko <ryan@cebulko.com> Date: Wed Aug 11 17:01:02 2021 -0400 ♻️ #core: error helpers (ampproject#35621) * Define core/error helpers * Drop createErrorVargs * Restore name assignment in Log#error * Typechecking/extern * Remove unused error_ helper * Typo * Add @ returns commit 6404407 Author: Brandon Suen <brandonbsuen@google.com> Date: Wed Aug 11 13:33:02 2021 -0700 ✅ [Story interactive] Rename Example Image Quiz and Poll Stories to Use .amp.html Extension (ampproject#35511) * Rename examples to use .amp.html extension * Remove confetti from results
AnuragVasanwala
added a commit
to rtCamp/amphtml
that referenced
this pull request
Aug 13, 2021
dethstrobe
pushed a commit
that referenced
this pull request
Apr 12, 2022
* Initiate amp-audio bento component * Make Audio component functional * ♻️ Initial Test Component * 🧪 Experimental Test Case Implementation * 🧪 Refactoring * ♻️ Comments and Prettify * 🐛 Minor fixes * ♻️ Minor fix for test case * ♻️ `propagateAttributes` moved to `amp-audio.js` * 🚮 Removed duplicate entry for `amp-date-display` in `bundles.config.extensions.json` * ♻️ `isStoryDescendent` is moved to isolated AMP layer * ♻️ `toggleFallback` refactored into `onLoad` and `onError` `toggleFallback` can be on `onLoad` / `onError` / etc. instead. The Preact layer does not care what logic is passed here, only when it should be called. Co-authored-by: Caroline Liu <10456171+caroqliu@users.noreply.github.com> * 🚮 Removed unnecessary `toggleFallback` from `component.js` * 🚮 Removed unnecessary comments * 🐛 Fixed support to load from `<source>` Added support to load audio from multiple `<source>` tags. Co-authored-by: Caroline Liu <10456171+caroqliu@users.noreply.github.com> * 🚮 Removed unnecessary commented code-block Co-authored-by: Caroline Liu <10456171+caroqliu@users.noreply.github.com> * ♻️ Initial value updated for `audioRef` `useRef` should be initialised with `null`. Co-authored-by: Caroline Liu <10456171+caroqliu@users.noreply.github.com> * ♻️ `props` destructured into inline * 🚮 Removed unused prop `children` from dependency * ♻️ `<ContainWrapper>` added * 🐛 Fixed bug related to `ARIA` attributes * ♻️ Type for `ARIA` attributes added * ♻️ Type definition are sorted for readability * ♻️ Test-case correction Co-authored-by: Caroline Liu <10456171+caroqliu@users.noreply.github.com> * ♻️ Fix for `ARIA` attributes * ✅ 🐛 Fix for `should load audio through sources` This will not be defined once we have sources prop defined in `base-element.js`. Instead assert `expect(audio.childNodes).to.have.lengthOf(2)`since the third child is disregarded due to being not a `<source>` tag. * ✅ ♻️ `element` instead of `audio` By using `<ContainWrapper>`, `offsetWidth` and `offsetHeight` should be evaluated on `element` instead of child `<audio>`. * 🚮 `propagateAttributes` removed `propagateAttributes` is passing attributes from `<amp-audio>` to the `<audio>` element it created. The Bento version does not have to use this utility because the `BaseElement['props']` definition already takes care of this. * ✅ ♻️ Fix for API function `play` and `pause` Play-Pause should not work when `amp-audio` is a direct decendent of `amp-story`. * ✅ ♻️ Refactored `amp-audio` import Co-authored-by: Caroline Liu <10456171+caroqliu@users.noreply.github.com> * ♻️ 🚮 Removed unnecessary comment for test Co-authored-by: Caroline Liu <10456171+caroqliu@users.noreply.github.com> * ✅ 🚮 Removed unnecessary import Co-authored-by: Caroline Liu <10456171+caroqliu@users.noreply.github.com> * ♻️ Removed unnecessary `isPlaying` dependency * ♻️ `triggerAnalyticsEvent` moved to AMP layer * ✅ ♻️ `getAmpAudio` improvised for `1.0` test suite This helper was taken from the `0.1` test suite and is improvised for `1.0` test suite. Co-authored-by: Caroline Liu <10456171+caroqliu@users.noreply.github.com> * ♻️ Fixed test case import * 🏗 Updated validation rules * ✅ Added preact component test-cases * ♻️ `lint` and `prettify` * ♻️ `listen` helper replaced by `onPlaying` `listen` helper is imperative way of attaching event handlers. We can instead use the `onPlaying` prop. * 📖 Markdown YAML updated for experimental `bento` * 🐛 Corrected bad type error * ✅ ♻️ Simplified selector for `audio` * ✅ ⏪ Uncommented failed test-cases for review * 🏗 Updated validation rules Also, copyright year updated to 2021 for validator files * ♻️ Added default media `METADATA` * ♻️ Added default value for `autoplay, loop, muted` * ♻️ Minor code fixes Co-authored-by: Caroline Liu <10456171+caroqliu@users.noreply.github.com> * ♻️ Simplify `playCallback` and `pauseCallback` To simplify object name, rename: `playCallback` -> `play` `pauseCallback` -> `pause` * ♻️ `...rest` should be attached to `ContainWrapper` `rest` should be attached to `ContainWrapper` so defined styles dimensions can be applied to the container. Co-authored-by: Caroline Liu <10456171+caroqliu@users.noreply.github.com> * 🐛 `realWin` required instead of `sandboxed` for Preact Test For the `should load audio through sources`, because we are measuring the component, it needs to be attached to the DOM. In order to attach it, we need to run the test under a `realWin` and access DOM via the given env. Co-authored-by: Caroline Liu <10456171+caroqliu@users.noreply.github.com> * 🐛 Need to attach mounted component to the document It is needed to have the mounted component attach to the document by providing the `attachTo` option. Co-authored-by: Caroline Liu <10456171+caroqliu@users.noreply.github.com> * 🐛 Use `.getDOMNode()` for `.offsetWidth` * 🐛 Fixes for failing `audio` props These were failing because the component actually renders an outer `ContainWrapper`, which was not given any of these props. So we have to actually get the `<audio>` instance to assert the props we want. Co-authored-by: Caroline Liu <10456171+caroqliu@users.noreply.github.com> * 🏗 ⏪ Bento components do not support `AMP4HTML` Bento components are only for `html_format: AMP` for now. They might support `html_format: AMP4HTML` in future! * ♻️ Fixes for `style` Co-authored-by: Caroline Liu <10456171+caroqliu@users.noreply.github.com> * ♻️ 🚮 Removed unnecessary `unlayout` block * ♻️ Added interface `AudioDef.AudioApi` API Action * ♻️ Propagate `<source>` children for preact `<source>` will only work if we also modify `component.js` to propagate children. * ♻️ Cleanup, lint and prettify * 🐛 Fixed preact test case error * ♻️ Formatting and minor fixes Co-authored-by: Alan Orozco <alanorozco@users.noreply.github.com> * ♻️ `audioPlaying` renamed to `onPlaying` With `metadata` name fixes and prettify. * 📖 ⏪ `ads` added back to YAML * ♻️ 🚮 `isInvocationValid` removed from the Bento implementation Reference: #35643 * ♻️ Cleanup, lint and prettify * 🚮 `amp-story` descendant check TestCase removed Refer: #35643 * ♻️ Cleanup, lint and prettify * ✨ Added support for `loading` attribute By default, `loading` is set to `lazy`. * ♻️ `source` is concidered instead of `children` Let's use `sources` or `children`, but not both. `sources` would be consistent with the video players. * ♻️ Formatted html with `<!-- prettier-ignore -->` Formatted with ` <!-- prettier-ignore -->` right above the opening `<head>` tag. This will prevent the boilerplate code from formatting. * ⏪ Removed `validateMediaMetadata` for component The validation step for video components on Bento are excluded for now. * 🏗 Add a new `bento_supported_version` tag Remove `deprecated_allow_duplicates: true` and `requires_usage: EXEMPTED` in new `1.0` versions. Use new `bento_supported_version` tag. Co-authored-by: Caroline Liu <10456171+caroqliu@users.noreply.github.com> * ♻️ Improved type definition Use `?SampleTypeDef` instead of `!SampleTypeDef | null` Co-authored-by: Caroline Liu <10456171+caroqliu@users.noreply.github.com> * ♻️ Improved event handler attachment Co-authored-by: Caroline Liu <10456171+caroqliu@users.noreply.github.com> * 🚮 Removed unnecessary comments * ♻️ Safer way to trigger `onPlaying` and `onPause` handlers Contributor: @alanorozco A safer way to trigger `onPlay` and `onPause` props is to call them from event handlers like audioPlaying. Please move into handlers. The idea is right, except for one small detail. play and playing are different events on the HTMLMediaElement spec, so let's rename the onPlay prop to onPlaying. * 🏗 Fixes for validation Co-authored-by: Alan Orozco <alanorozco@users.noreply.github.com> nit: This quirky formatting is another inherited property, we could format this more nicely: 1. Temporarily, add `<!-- prettier-ignore -->` right above the opening `<head>` tag. This will prevent the boilerplate code from formatting. 2. Format with prettier locally, or on `prettier.io/playground/` (choose html parser on the options on the left side). 3. Remove `<!-- prettier-ignore -->` * ♻️ Extra space removed for validation file * 🚮 Removed copyright header from all `*.js` files Co-Authored-By: Alan Orozco <alanorozco@users.noreply.github.com> Co-Authored-By: honeybadgerdontcare <1340565+honeybadgerdontcare@users.noreply.github.com> * 🚮 Removed copyright header from `protoascii` Co-Authored-By: Alan Orozco <alanorozco@users.noreply.github.com> * ♻️ Removed `loading` from `BaseElement` We should not expose control of `loading` at this level—let's remove the attribute. (The prop on `component.js` should stay as-is.) Co-authored-by: Alan Orozco <alanorozco@users.noreply.github.com> * ♻️ Removed `EMPTY_METADATA` for `artist` and `artwork` `EMPTY_METADATA.artwork` is an Array, which does not make sense as a string. Let's set it to an empty string instead. It's okay to remove the default for `artwork` and and `artist`. Co-authored-by: Alan Orozco <alanorozco@users.noreply.github.com> * ♻️ Removed `default` for `loop` and `muted` from `BaseElement` Remove `default` from booleans at this level, set on component props instead. Co-authored-by: Alan Orozco <alanorozco@users.noreply.github.com> * ♻️ Removed `autoplay` for now, `useInOb` will be used in future To be clear - does `amp-audio:0.1` begin playing immediately once it enters viewport if given autoplay? If so, I think we should consider using the useInOb hook @dmanek is working on to do this feature, perhaps in a separate PR. Having it `autoplay` immediately is not a good UX, and short of (potentially) implementing the viewport based logic, should be removed from the AMP layer for now. Co-Authored-By: Alan Orozco <alanorozco@users.noreply.github.com> Co-Authored-By: Caroline Liu <10456171+caroqliu@users.noreply.github.com> * 🚮 Removed `EMPTY_METADATA` Co-Authored-By: Alan Orozco <alanorozco@users.noreply.github.com> Co-Authored-By: Caroline Liu <10456171+caroqliu@users.noreply.github.com> * ♻️ Minor fixes for TestCase Co-authored-by: Alan Orozco <alanorozco@users.noreply.github.com> * 🚮 Removed copyright header from validator, ♻️Re-created validator Co-Authored-By: Alan Orozco <alanorozco@users.noreply.github.com> * ♻️ Fix to trigger `element.toggleFallback` for `onError` Co-Authored-By: Alan Orozco <alanorozco@users.noreply.github.com> * ♻️ Fixes for TestCase Recently updated Classic `amp-audio` tests to remove this odd creation function and use HTML templates instead on #35650 Co-Authored-By: Alan Orozco <alanorozco@users.noreply.github.com> * Latest updates to * ♻️ Removed unnecessary var `ampAudio` * ♻️ `lint` and `prettify` * 🏗 Switched to latest LTS version of node Node version v14.17.5 (latest LTS) python version 2.7.16 npm version 6.14.14 * ♻️ Fix for preact component TestCase * Fix bento amp-audio unit test * 🧪 Experimental test for `done() called multiple times in hook` Two consecutive test run resolves this error on local machine. As I do not have capability to restart CircleCI, I need to push this commit just to trigger re-run CircleCI to check. * 🏗 Updated validation rules Running `amp validator --update_tests` have updated `package-lock.json` files. Co-Authored-By: honeybadgerdontcare <1340565+honeybadgerdontcare@users.noreply.github.com> * ♻️ Fix import alias * ♻️ Update derived class extends using `setSuperClass` * Fix test case and build issue * Remove addon-knobs dependency from storybook files * Update bundles config with new properties * 🖍 Add CSS (as per #32180) * ♻️ Prefix `Bento` to Preact Component (`BentoAudio`) Co-authored-by: Edi Amin <to.ediamin@gmail.com> Co-authored-by: Caroline Liu <10456171+caroqliu@users.noreply.github.com> Co-authored-by: Alan Orozco <alanorozco@users.noreply.github.com> Co-authored-by: honeybadgerdontcare <1340565+honeybadgerdontcare@users.noreply.github.com> Co-authored-by: Deepak Lalwani <deepak.lalwani81@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Removes check from
<amp-audio>that prevents actions when inside an<amp-story>element.<amp-story>now has an action allowlist, so the check is redundant. Removing the check still prevents the action, with the message: