Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Abstract content fetching mechanism #2

Merged
merged 3 commits into from
Oct 9, 2014
Merged

Abstract content fetching mechanism #2

merged 3 commits into from
Oct 9, 2014

Conversation

josh
Copy link
Contributor

@josh josh commented Oct 6, 2014

Defines a DeferredContentElement.prototype.fetch can be overridden in subclasses to define a custom fetch method. The method takes a String url => Promise(String html).

I think the current poll fetch method is too specific to github's legacy polling design and shouldn't be promoted as the core api.

PollDeferredContentPrototype.fetch = pollFetch;

document.registerElement('poll-deferred-content', {
  prototype: PollDeferredContentPrototype,
  extends: 'deferred-content'
});
<deferred-content is="polled-deferred-content" src="branches.html" wait=1000></deferred-content>

@dgraham
Copy link
Contributor

dgraham commented Oct 6, 2014

If there's one dependency worth adding, it's Promises. I was going for zero, but it might be worth it.

This feels a little premature, though. I was hoping to replace more of our .js-deferred-content usage with this element to find out if polling was really needed or not.

@josh
Copy link
Contributor Author

josh commented Oct 6, 2014

This feels a little premature, though

Its still useful for overriding for other things. If you wanted timeouts, I think this would be a good place to override.

@josh
Copy link
Contributor Author

josh commented Oct 6, 2014

We could also consider specing this as an "abortable" promise extension. If the promise defines .abort(), we'll call it when .src changes and the promise is still pending.

@josh
Copy link
Contributor Author

josh commented Oct 6, 2014

If there's one dependency worth adding, it's Promises. I was going for zero, but it might be worth it.

We definitely want promises to help implement #4.

Conflicts:
	deferred-content.js
@josh
Copy link
Contributor Author

josh commented Oct 7, 2014

Tests pass after merging #6.

josh added a commit that referenced this pull request Oct 9, 2014
Abstract content fetching mechanism
@josh josh merged commit 8b8a0c2 into master Oct 9, 2014
@josh josh deleted the fetch branch October 9, 2014 22:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants