Skip to content

Move video_state macro#26212

Merged
micajuine-ho merged 6 commits intoampproject:masterfrom
micajuine-ho:move_video_state
Feb 14, 2020
Merged

Move video_state macro#26212
micajuine-ho merged 6 commits intoampproject:masterfrom
micajuine-ho:move_video_state

Conversation

@micajuine-ho
Copy link
Copy Markdown
Contributor

@micajuine-ho micajuine-ho commented Jan 5, 2020

Project Tracker: #26091

Moving VIDEO_STATE macro to amp-analytics

Since VIDEO_STATE macro is referenced by ID, we can't move it from url-replacements. However we can still decrease v0 size by moving the implementation of the macro to video-manager-impl.
This will also improve the cohesiveness of the video-manager analytics.

Behavior is unchanged (can still use the VIDEO_STATE macro as a part of the url substitution and also in amp-analytics vars).

@micajuine-ho micajuine-ho force-pushed the move_video_state branch 2 times, most recently from 4b8b5ec to 539f57f Compare February 12, 2020 21:35
Copy link
Copy Markdown
Member

@alanorozco alanorozco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@micajuine-ho micajuine-ho force-pushed the move_video_state branch 2 times, most recently from 3c05e69 to 83bd4fb Compare February 13, 2020 04:19
@micajuine-ho micajuine-ho requested a review from zhouyx February 13, 2020 04:36
@micajuine-ho micajuine-ho merged commit 755ff40 into ampproject:master Feb 14, 2020
robinvanopstal added a commit to jungvonmatt/amphtml that referenced this pull request Feb 17, 2020
* master: (181 commits)
  🏗 Ensure valid flag usage for `gulp` tasks (ampproject#26814)
  build-system: Fix autocomplete error response (ampproject#26824)
  application/json is ab allowed type for <script> (ampproject#26815)
  🚮 Removing amp-consent-v2 experiment logic (ampproject#26162)
  Fix more arrow functions that are passed in as "constructors" (ampproject#26795)
  Variable substitution tester  (ampproject#26695)
  Turn on restrict fullscreen canary (ampproject#26766)
  Mock variableService getMacros  (ampproject#26300)
  Sync from Google (ampproject#26805)
  Sync from Google (ampproject#26803)
  Move video_state macro (ampproject#26212)
  🚀 Move ads variables to amp-analytics (ampproject#25113)
  Sync from Google (ampproject#26800)
  Sync from Google (ampproject#26798)
  Sync from Google (ampproject#26792)
  Another set of example.com change (ampproject#26753)
  Add PWA multidoc loader to examples (ampproject#26680)
  🐛Check for window null before creating tracking pixel (ampproject#26749)
  trying to update Sauce timeouts (ampproject#26737)
  🐛Fixes swipe to dismiss badly ordered swipes on `amp-lightbox-gallery` (ampproject#26788)
  ...

# Conflicts:
#	extensions/amp-accordion/amp-accordion.md
#	extensions/amp-bind/amp-bind.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants