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

Add a spinner, in Service Worker mode, when browsing to another article. #155

Closed
wants to merge 1 commit into from

Conversation

mossroy
Copy link
Contributor

@mossroy mossroy commented Jan 8, 2016

So that the user knows his click is being processed.
Fixes #135

So that the user knows his click is being processed.
Fixes #135
if (contentInjectionMode === 'serviceworker') {
// In Service Worker mode, we want to display a spinner
// when the user clicks on a link
// We only do that when the click is somewhere inside a <a> tag
Copy link
Contributor

Choose a reason for hiding this comment

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

Is that really a safe way? We could wait for a message from the service worker (that is sent as soon as the service worker starts loading the article).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right : it's not a safe way at all. It was just an easy way to do a first version.
In fact, I would have expected the browser to inform the user by itself that a ServiceWorker is working. Just like it informs the user when some content is downloading.
It seems to be the case on Chromium, Firefox dev edition, but not on Firefox OS.

I should investigate if there is a way to have this built-in feedback on Firefox OS too, instead of trying to do it by hand.

@peter-x
Copy link
Contributor

peter-x commented Jan 9, 2016

I think this is good to merge for now, perhaps turn the comment above into an issue.

@mossroy
Copy link
Contributor Author

mossroy commented Apr 26, 2017

I close this old PR. It was not well tested, and I'm not sure if it was FirefosOS-specific.
The code is still in the branch and might be reused if necessary

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