From 52948a41654df69d67140c46dd8cac80772fdaa6 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Mon, 3 Apr 2017 16:51:14 -0600 Subject: [PATCH 1/3] Added support for a plugin which disables lazy assets --- client/coral-embed/src/index.js | 70 ++++++++----------- .../graphql/queries/streamQuery.graphql | 2 +- graph/resolvers/root_query.js | 5 -- routes/assets/index.js | 1 + routes/index.js | 1 + views/article.ejs | 3 +- views/embed-stream.ejs | 48 ------------- 7 files changed, 35 insertions(+), 95 deletions(-) delete mode 100644 views/embed-stream.ejs diff --git a/client/coral-embed/src/index.js b/client/coral-embed/src/index.js index e3350e0d3..9b1b29879 100644 --- a/client/coral-embed/src/index.js +++ b/client/coral-embed/src/index.js @@ -1,5 +1,7 @@ import pym from 'pym.js'; +import {stringify} from 'querystring'; + const snackbarStyles = { position: 'fixed', cursor: 'default', @@ -26,30 +28,21 @@ const Coral = {}; const Talk = Coral.Talk = {}; // build the URL to load in the pym iframe -function buildStreamIframeUrl(talkBaseUrl, asset_url, comment, asset_id) { - let iframeArray = [ +function buildStreamIframeUrl(talkBaseUrl, query) { + let url = [ talkBaseUrl, (talkBaseUrl.match(/\/$/) ? '' : '/'), // make sure no double-'/' if opts.talk already ends with '/' - 'embed/stream?asset_url=', - encodeURIComponent(asset_url) - ]; + 'embed/stream?' + ].join(''); - if (comment) { - iframeArray.push('&comment_id='); - iframeArray.push(encodeURIComponent(comment)); - } + url += stringify(query); - if (asset_id) { - iframeArray.push('&asset_id='); - iframeArray.push(encodeURIComponent(asset_id)); - } - - return iframeArray.join(''); + return url; } // Set up postMessage listeners/handlers on the pymParent // e.g. to resize the iframe, and navigate the host page -function configurePymParent(pymParent, asset_url) { +function configurePymParent(pymParent) { let notificationOffset = 200; let ready = false; let cachedHeight; @@ -117,8 +110,8 @@ function configurePymParent(pymParent, asset_url) { if (ready) { window.clearInterval(interval); - // @todo - It's weird to me that this is sent here in addition to the iframe URL. Could it just be in one place? - pymParent.sendMessage('DOMContentLoaded', asset_url); + // TODO: It's weird to me that this is sent here + pymParent.sendMessage('DOMContentLoaded'); } }, 100); }); @@ -166,42 +159,39 @@ Talk.render = function (el, opts) { } opts = opts || {}; - // @todo infer this URL without explicit user input (if possible, may have to be added at build/render time of this script) + // TODO: infer this URL without explicit user input (if possible, may have to be added at build/render time of this script) if (!opts.talk) { throw new Error('Coral.Talk.render() expects opts.talk as the Talk Base URL'); } - // ensure el has an id, as pym can't directly accept the HTMLElement + // Ensure el has an id, as pym can't directly accept the HTMLElement. if (!el.id) { el.id = `_${Math.random()}`; } - let asset_url = opts.asset_url; - if (!asset_url) { - try { - asset_url = document.querySelector('link[rel="canonical"]').href; - } catch (e) { - console.warn('This page does not include a canonical link tag. Talk has inferred this asset_url from the window object. Query params have been stripped, which may cause a single thread to be present across multiple pages.'); - asset_url = window.location.origin + window.location.pathname; + // Compose the query to send down to the Talk API so it knows what to load. + let query = {}; + + query.comment_id = window.location.hash.slice(1); + query.asset_id = opts.asset_id; + + if (!query.asset_id) { + query.asset_url = opts.asset_url; + if (!query.asset_url && !query.asset_id) { + try { + query.asset_url = document.querySelector('link[rel="canonical"]').href; + } catch (e) { + console.warn('This page does not include a canonical link tag. Talk has inferred this asset_url from the window object. Query params have been stripped, which may cause a single thread to be present across multiple pages.'); + query.asset_url = window.location.origin + window.location.pathname; + } } } - let comment = window.location.hash.slice(1); - - let query = { + configurePymParent(new pym.Parent(el.id, buildStreamIframeUrl(opts.talk, query), { title: opts.title, - asset_url: asset_url, id: `${el.id}_iframe`, name: `${el.id}_iframe` - }; - - if (opts.asset_id && opts.asset_id.length > 0) { - query.asset_id = opts.asset_id; - } - - let pymParent = new pym.Parent(el.id, buildStreamIframeUrl(opts.talk, asset_url, comment), query); - - configurePymParent(pymParent, asset_url); + })); }; export default Coral; diff --git a/client/coral-framework/graphql/queries/streamQuery.graphql b/client/coral-framework/graphql/queries/streamQuery.graphql index c9f9d3d5b..102c656e5 100644 --- a/client/coral-framework/graphql/queries/streamQuery.graphql +++ b/client/coral-framework/graphql/queries/streamQuery.graphql @@ -1,6 +1,6 @@ #import "../fragments/commentView.graphql" -query AssetQuery($asset_id: ID, $asset_url: String!, $comment_id: ID!, $has_comment: Boolean!) { +query AssetQuery($asset_id: ID, $asset_url: String, $comment_id: ID!, $has_comment: Boolean!) { # the comment here is for loading one comment and it's children, probably after following a permalink # $has_comment is derived from the comment_id query param in the iframe url, # which is in turn pulled from the host page url diff --git a/graph/resolvers/root_query.js b/graph/resolvers/root_query.js index 6f4151ff6..32e2e4263 100644 --- a/graph/resolvers/root_query.js +++ b/graph/resolvers/root_query.js @@ -8,11 +8,6 @@ const RootQuery = { }, asset(_, query, {loaders: {Assets}}) { if (query.id) { - - // TODO: we may not always have a comment stream here, therefore, when we - // load it, we may also need to create with the url. This may also have to - // move the logic over to the mutators function as an upsert operation - // possibly. return Assets.getByID.load(query.id); } diff --git a/routes/assets/index.js b/routes/assets/index.js index e8c508b52..a5c0eca50 100644 --- a/routes/assets/index.js +++ b/routes/assets/index.js @@ -14,6 +14,7 @@ router.get('/id/:asset_id', (req, res, next) => { } res.render('article', { title: asset.title, + asset_id: asset.id, asset_url: asset.url, body: '', basePath: '/client/embed/stream' diff --git a/routes/index.js b/routes/index.js index 58c8f13d7..0ed2d9dcf 100644 --- a/routes/index.js +++ b/routes/index.js @@ -16,6 +16,7 @@ if (process.env.NODE_ENV !== 'production') { return res.render('article', { title: 'Coral Talk', asset_url: '', + asset_id: '', body: '', basePath: '/client/embed/stream' }); diff --git a/views/article.ejs b/views/article.ejs index 5f90cdf4d..e20f91117 100644 --- a/views/article.ejs +++ b/views/article.ejs @@ -27,7 +27,8 @@ diff --git a/views/embed-stream.ejs b/views/embed-stream.ejs deleted file mode 100644 index c69773bcb..000000000 --- a/views/embed-stream.ejs +++ /dev/null @@ -1,48 +0,0 @@ - - - - - - Talk - Coral Admin - - - - - - -
- - - - From 2546902bcf1f28cc423bb8a06be891141d4470f3 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Mon, 3 Apr 2017 16:59:31 -0600 Subject: [PATCH 2/3] Still grab the url when we have the id --- client/coral-embed/src/index.js | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/client/coral-embed/src/index.js b/client/coral-embed/src/index.js index 9b1b29879..21c176500 100644 --- a/client/coral-embed/src/index.js +++ b/client/coral-embed/src/index.js @@ -175,15 +175,13 @@ Talk.render = function (el, opts) { query.comment_id = window.location.hash.slice(1); query.asset_id = opts.asset_id; - if (!query.asset_id) { - query.asset_url = opts.asset_url; - if (!query.asset_url && !query.asset_id) { - try { - query.asset_url = document.querySelector('link[rel="canonical"]').href; - } catch (e) { - console.warn('This page does not include a canonical link tag. Talk has inferred this asset_url from the window object. Query params have been stripped, which may cause a single thread to be present across multiple pages.'); - query.asset_url = window.location.origin + window.location.pathname; - } + query.asset_url = opts.asset_url; + if (!query.asset_url) { + try { + query.asset_url = document.querySelector('link[rel="canonical"]').href; + } catch (e) { + console.warn('This page does not include a canonical link tag. Talk has inferred this asset_url from the window object. Query params have been stripped, which may cause a single thread to be present across multiple pages.'); + query.asset_url = window.location.origin + window.location.pathname; } } From 79d5c28d00555b327a9a5e5a8bd99b7d3500dddd Mon Sep 17 00:00:00 2001 From: David Erwin Date: Thu, 6 Apr 2017 10:47:56 -0400 Subject: [PATCH 3/3] Pass asset_id in title route --- routes/assets/index.js | 1 + 1 file changed, 1 insertion(+) diff --git a/routes/assets/index.js b/routes/assets/index.js index a5c0eca50..54d4e72ed 100644 --- a/routes/assets/index.js +++ b/routes/assets/index.js @@ -27,6 +27,7 @@ router.get('/title/:asset_title', (req, res) => { return res.render('article', { title: req.params.asset_title.split('-').join(' '), asset_url: '', + asset_id: null, body: body, basePath: '/client/embed/stream' });