🧪 Experiment: story auto advance and new cta button #38097
Conversation
|
Hey @Jiaming-X! These files were changed: Hey @jeffkaufman! These files were changed: Hey @gmajoulet, @newmuis! These files were changed: Hey @mszylkowski! These files were changed: |
calebcordry
left a comment
calebcordry
left a comment
There was a problem hiding this comment.
Experiment setup LGTM. Will defer to the other folks on the best way to disable animation
| ); | ||
| if ( | ||
| (!isExperimentOn(this.win, 'story-ad-auto-advance') && this.isAd()) || | ||
| ((!storyAdSegmentBranch || |
There was a problem hiding this comment.
nit: these complex if statements here and below are a bit hard to parse, maybe pulling to a named var will help.
powerivq
force-pushed
the
launch-auto-advance-agin
branch
from
b57e029 to
d0ec5d2
Compare
powerivq
force-pushed
the
launch-auto-advance-agin
branch
from
d0ec5d2 to
50fd083
Compare
| animation: i-amphtml-tap-scale var(--i-amphtml-page-attachment-ui-animation-duration) var(--i-amphtml-page-attachment-ui-animation-delay) both !important; | ||
| } | ||
|
|
||
| .i-amphtml-story-page-open-attachment-outlink-no-animation-exp.i-amphtml-story-page-open-attachment-outlink-no-animation-exp[active] { |
There was a problem hiding this comment.
If you want to avoid the animations above, it's better to do .i-amphtml-story-page-open-attachment-outlink[active]:not(.i-amphtml-story-page-open-attachment-outlink-no-animation-exp) {animation: i-amphtml-tap-scale...} instead of overriding it with animation: none on a new property. Same below.
| const openImgAttr = attachmentEl.getAttribute('cta-image'); | ||
|
|
||
| const noAnimation = | ||
| getExperimentBranch(getWin(pageEl), 'story-ad-auto-advance') == 'c'; |
There was a problem hiding this comment.
Where is this constant c from? Do you need to replace it with the correct value from StoryAdSegmentExp.AUTO_ADVANCE_NEW_CTA_NOT_ANIMATED?
4 participants
Experiments:
Experiment IDs need to be updated before merging.