Closed cpauwels closed 4 years ago
Hi @shishirm, are there any updates on this issue? Anything else we should do to get the PR #4298 merged?
Hello @cpauwels, thank you so much for filing this. We've reviewed the various Stories and before we list on amp.dev, some fixes are requested,
Must-Fix
The Stories are currently not self-canonical i.e. the link rel canonical tag currently points to the non-Story version. For example, https://stories.tappable.co/bbc-micro?story=true points to https://stories.tappable.co/bbc-micro as canonical but should instead point to https://stories.tappable.co/bbc-micro?story=true. Could you kindly update all Stories with this fix. We suggest providing this feature within the tool itself.
Nice-To-Haves
These are NOT required to list on amp.dev but we'd like to pass along these recommendations to incorporate in the tool based on reviewing the submitted Stories,
Also, thank you for filing the PR. However, I will create a parallel PR to submit as it's easier for us to test and merge in. Once your canonical fixes are in place, I'll go ahead and push the PR. Please feel free to update this issue when ready.
Thank you!
Some additional feedback,
Hi @shishirm,
regarding the first issue, our backend detects wether it needs to return a web story or wether it needs to return the player experience you see.
So running https://stories.tappable.co/bbc-micro
through the AMP Test tool produces valid results which you can see here. The ?story=true
query param is only used to force the delivery of the web story regardless of the experience.
The same is true when using the linter with npx @ampproject/toolbox-cli lint https://stories.tappable.co/bbc-micro
, the test will PASS Story is self-canonical
.
Let me know if anything is unclear.
PS: I've also advocated in a Stories WG meeting for dropping the self-canonical requirement now that the amp-story-player
exists. Happy to discuss this if needed! cc @newmuis
Thanks for the details @cpauwels. Running this up the flagpole to confirm nothing untoward and will then create a PR to update amp.dev.
Filed https://github.com/ampproject/amp.dev/pull/4599 to close this out.
🛠️ Request to Add Third-Party Story Creation Tool/CMS/Platform
Story creation resource name
Tappable
Type
Use [x] to mark a selection:
URL
https://tappable.co
Image
tappable-amp-dev-logo.svg.zip (Used a zip since GH doesn't allow svg images upload)
Description
The collaborative Web Stories builder. Design immersive Stories, publish on a blazing fast CDN and measure engagement with zero-config analytics.
List of Stories
BBC Micro Bot Our World From Space Diversity in Television Fortnite Brand Collabs Gestalt Theory Stay Cultured at Home How Contact Tracing Works Stop the Spread of False Information Ramadan in Times of COVID-19 Microsoft Flight Simulator 2020 Brussels Beer Project Mushroom Carbonara
Notes
We implemented blurred placeholders for images that are not loaded yet but the amp linter doesn't seem to like them, complaining about
FAIL All <amp-img> have reasonable width and height
.