Skip to content
This repository was archived by the owner on Aug 10, 2026. It is now read-only.

Rewind draft tutorial - #767

Merged
Srushtika merged 8 commits into
masterfrom
rewind-tutorial
Nov 19, 2019
Merged

Rewind draft tutorial#767
Srushtika merged 8 commits into
masterfrom
rewind-tutorial

Conversation

@Srushtika

Copy link
Copy Markdown
Contributor

No description provided.

@mattheworiordan
mattheworiordan temporarily deployed to ably-docs-staging-pr-767 November 13, 2019 17:48 Inactive
@mattheworiordan
mattheworiordan temporarily deployed to ably-docs-staging-pr-767 November 14, 2019 14:23 Inactive
@mattheworiordan
mattheworiordan temporarily deployed to ably-docs-staging-pr-767 November 14, 2019 17:23 Inactive
@mattheworiordan
mattheworiordan temporarily deployed to ably-docs-staging-pr-767 November 18, 2019 14:28 Inactive
@mattheworiordan
mattheworiordan temporarily deployed to ably-docs-staging-pr-767 November 19, 2019 16:38 Inactive
@Srushtika
Srushtika marked this pull request as ready for review November 19, 2019 16:38

@tomczoinktomczoink left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few points, otherwise nicely done

Comment threadcontent/tutorials/channel-rewind.textile Outdated
Comment threadcontent/tutorials/channel-rewind.textile Outdated
Comment threadcontent/tutorials/channel-rewind.textile Outdated
Comment threadcontent/tutorials/channel-rewind.textile Outdated
Comment threadcontent/tutorials/channel-rewind.textile Outdated
Comment threadcontent/tutorials/channel-rewind.textile Outdated
Comment threadcontent/tutorials/channel-rewind.textile Outdated
Comment threadcontent/tutorials/channel-rewind.textile
Comment threadcontent/tutorials/channel-rewind.textile Outdated
@mattheworiordan
mattheworiordan temporarily deployed to ably-docs-staging-pr-767 November 19, 2019 17:07 Inactive
Comment threadcontent/tutorials/channel-rewind.textile
Comment threadcontent/tutorials/channel-rewind.textile
subscribeOpenWeather($('select#city-id').val());
function subscribeOpenWeather(id) {
var channelWeather = "[product:ably-openweathermap/weather?rewind=1]weather:" + id;
var URL = `https://realtime.ably.io/sse?v=1.1&key=${apiKey}&channels=${channelWeather}`;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want to include a channel name with params in the name, you need to uri-encode it, which you're not doing here. A much easier way is just to add rewind=1 as another url param. This is explained in the documentation pr:
https://github.com/ably/docs/pull/729/files#diff-7b572e68f8441588d6eacc180bc8f989R76-R89

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(to explain this a bit further: the reason this is especially necessary here is that question marks have special meaning in urls, they start the query string ("/sse?v=..."). Maybe in some browsers having subsequent question marks inside the query string will work, but it's not guaranteed to work in all of them. The correct/safe thing to do that will work in all browsers is uri-encode them. (Really this should be done for all api streamer sse examples -- the brackets and forwardslash in [product:foo/bar]baz are technically also disallowed, though again, will probably work in some browsers))

Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@Srushtika@paddybyers@SimonWoolf@tomczoink@mattheworiordan