A React-based PWA for displaying AMP content #52
Conversation
|
@cramforce PTAL |
cramforce
changed the title
cramforce
left a comment
cramforce
left a comment
There was a problem hiding this comment.
I'm getting lots of AMP related errors in the console. Is that expected? @dvoytenko
amp-pwa/manifest.webmanifest
Outdated
| "type": "image/png" | ||
| } | ||
| ] | ||
| } No newline at end of file |
There was a problem hiding this comment.
Add trailing newlines here and elsewhere.
amp-pwa/src/components/home.js
Outdated
| } | ||
| Home.propTypes = { | ||
| documents: React.PropTypes.arrayOf(React.PropTypes.object), | ||
| } |
|
The demo doesn't work on safari - if this is intended, we should mention it in the limitations. |
|
Could you fix the images? When I'm going to a deep link directly it serves the AMP doc (yaihh), but then SW installation fails with a 404. |
|
@cramforce I noticed errors yesterday and fixed in a separate PR. Unfortunately, this part broken on resources refactoring. We'll need to setup integration tests for this part. |
dreamofabear
force-pushed
the
amp-pwa
branch
from
8a6fbe1 to
b13933d
Compare
@sebastianbenz Not intended -- I'm investigating a problem in Safari with the Closure-compiled version of the AMP-shadow runtime,
Done. I also worked around the shadow bug by bundling a local build, although it breaks
That was probably the |
dreamofabear
changed the title
|
Can you make URLs like On Tue, Sep 20, 2016 at 5:28 PM, William Chou [email protected]
|
|
That should already work and I just tested it again on desktop Chrome. If you see the nav bar at the top, then the shell was rendered. |
|
Yeah, never mind. It is working now. On Wed, Sep 21, 2016 at 11:07 AM, William Chou [email protected]
|
|
One request, though: The leaf pages should use amp-install-serviceworker to On Wed, Sep 21, 2016 at 11:09 AM, Malte Ubl [email protected] wrote:
|
|
Few more comments from my side:
|
|
@sebastianbenz We're looking at actually getting good newsy-content soon. The ABE stuff is just to have something at all. |
Done.
Fixed, thanks!
@sebastianbenz Could you elaborate on what you mean by this? E.g. have the list of articles in the nav bar, or in the AMP documents themselves? |
|
@choumx I think what @sebastianbenz meant is something like this:
Clicking this should trigger the PWA router and navigate on the client side. |
| } | ||
|
|
||
| componentDidMount() { | ||
| this.fetchAndAttachAMPDoc_(this.props.src); |
There was a problem hiding this comment.
Generally, we do full camel-case style guide. E.g. fetchAndAttachAmpDoc_. If it doesn't conflict with React - let's switch.
|
|
||
| /** | ||
| * `window.AMP` is set by the AMP runtime when it finishes loading. | ||
| * @private |
|
|
||
| /** | ||
| * Child element that the AMP document will be added as a shadow root to. | ||
| * @private |
|
|
||
| /** | ||
| * XMLHTTPRequest that fetches the AMP document. | ||
| * @private |
| * Fetches and parses HTML at `url`. | ||
| * @private | ||
| * @param {string} url | ||
| * @return {Promise} If fetch succeeds, resolved with {Document}. |
There was a problem hiding this comment.
Should be {!Promise<!Document>}
| */ | ||
| fetchDocument_(url) { | ||
| return new Promise((resolve, reject) => { | ||
| const xhr = (this.xhr_ = new XMLHttpRequest()); |
There was a problem hiding this comment.
Looks like we also need to reset this.xhr_ = null in the either outcome of this promise.
|
Added cross-ref links to AMP documents and corresponding react-router support in Haven't updated the live demo with this yet -- ironically, prod AMP works for this feature but HEAD has issues. |
|
I'm good here. How about everyone else? |
|
@cramforce Any other feedback? |
cramforce
left a comment
cramforce
left a comment
There was a problem hiding this comment.
Lets merge this and then flesh out details in follow up!
4 participants
A simple React-based progressive web app that displays Accelerated Mobile Page (AMP) content. Built on create-react-app for minimal build configuration.
Live demo: http://choumx.github.io/amp-pwa
This project adds modern web features to
create-react-app:Compared to
create-react-app, this project adds a small number of new dependencies:sw-precachegenerates a production-ready service worker for precaching and other progressive enhancements.expressis used to run the development API server,server.js.bootstrapandreact-bootstrapcan be easily removed for your choice of UI framework.