✨ [AMP Story Paywall] Enable developers to specify a custom subscriptions page index #38175
Conversation
|
Hey @newmuis, @gmajoulet! These files were changed: |
|
/to @mszylkowski |
extensions/amp-story-subscriptions/0.1/amp-story-subscriptions.js
Outdated
Show resolved
Hide resolved
| subscriptionsState !== SubscriptionsState.DISABLED && | ||
| this.subscriptionsPageIndex_ === -1 | ||
| ) { | ||
| return this.blockOnPendingSubscriptionsPageIndex_( |
There was a problem hiding this comment.
Seems like you're using 2 methods that do almost the same thing: wait for the subscription to initialize. I recommend just having 1 promise that resolves when the subscriptions state and index are both resolved using Promise.all([promise1, promise2]) and do if (anything is missing on the subscriptions) {this.blockOnPendingSubscriptionsState(x, y)}
There was a problem hiding this comment.
So two states should be blocked and waited at different times. This is because sub state is a network request and takes much longer than getting the sub index from amp-story-subscriptions. This is why the sub index is blocked on the initial layout switchTo and the sub state is only blocked when user is trying to navigate to the page after sub index.
extensions/amp-story-subscriptions/0.1/amp-story-subscriptions.js
Outdated
Show resolved
Hide resolved
extensions/amp-story-subscriptions/0.1/amp-story-subscriptions.js
Outdated
Show resolved
Hide resolved
| @@ -328,6 +322,14 @@ export class AmpStory extends AMP.BaseElement { | |||
|
|
|||
| /** @private {?number} the timeout to show subscriptions dialog after delay */ | |||
| this.showSubscriptionsUITimeout_ = null; | |||
|
|
|||
| /** @private {?number} the index of the page where the paywall would be triggered. */ | |||
| this.subscriptionsPageIndex_ = this.storeService_.get( | |||
There was a problem hiding this comment.
Getting this number will always be -1, it's not necessary to fetch it from the storeservice
There was a problem hiding this comment.
This is to potentially get the already resolved index from the store service, since the amp-story-subscriptions might already have initialized.
|
|
Agree and this PR is definitely targeting at taking care of both scenarios. Let me update the PR description to avoid confusions. |
…a-menu-images-validator-spec * 'main' of github.com:ampproject/amphtml: (90 commits) 🔥 [Story mediapool] Fix videos from mediapool with `noaudio` don't have audio when reused. (#38216) Hide progress bar on the control group of auto advance experiment (#38215) ✨ Add Bento Autocomplete Component (#37837) 🌐 [Story subscription] Subscription localization async (#38204) Dable: add new optional parameter "channel" (#38199) ✨ [AMP Story Paywall] Enable developers to specify a custom subscriptions page index (#38175) SwG Release 0.1.22.217 (#38187) amp-script: implements new size limits for sandboxed scripts (#38185) 🖍 Hide the system layer and progress bar in preview mode (#38163) added minItems (#38177) Prevent expandTemplate from ReDOSing (#38178) Change amp-story-subscriptions attribute name to reflect its flexibility (#38176) 🐛 [Story Preview] Enable amp-video to play in preview mode (#38149) Added the possibility to get page count to story messaging api (#38170) SwG Release 0.1.22.216 (#38168) Allow @newmuis to update OWNERS files (#38169) ✨ Add Richaudience to RTC callout vendors (#38160) 🚀 SunMedia: Update amp-ad (#38128) Remove option to deploy PR artifacts to a static website (#38152) added some vars and requests in gfksensic.json (#37722) ...
Block the story if the index is not resolved yet. After amp-story-subscriptions retrieve the index specified by the developers, it would update it in store service for amp-story to unblock and conditionally show the paywall on the specified page.
From the product perspective, we decided that the index provided by developers has to be larger than 2 and not the last page of the story.