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

Change favicon when busy #1837

Merged
merged 2 commits into from Oct 18, 2016
Merged

Change favicon when busy #1837

merged 2 commits into from Oct 18, 2016

Conversation

@gnestor
Copy link
Contributor

@gnestor gnestor commented Oct 15, 2016

Closes #1820

I’m open to busy icon suggestions…

screenshot

@gnestor gnestor modified the milestones: 4.3, 5.0 Oct 15, 2016
@takluyver
Copy link
Member

@takluyver takluyver commented Oct 17, 2016

👍

@jasongrout
Copy link
Member

@jasongrout jasongrout commented Oct 17, 2016

I really like this change. Can we also not change the page title anymore, now that we have this much less distracting indicator in a tab? This may be for a different PR, but if you could do it in this PR, that would be awesome.

@takluyver
Copy link
Member

@takluyver takluyver commented Oct 17, 2016

I'd also favour not changing the title - my tabs are often quite short, and "(Busy)" will take up ~all the visible space.

@gnestor
Copy link
Contributor Author

@gnestor gnestor commented Oct 17, 2016

@jasongrout @takluyver Updated this PR with your suggestion...

});

this.events.on('kernel_ready.Kernel', function () {
that.save_widget.update_document_title();
$kernel_ind_icon.attr('class','kernel_idle_icon').attr('title','Kernel Idle');
knw.info("Kernel ready", 500);
change_favicon('/static/base/images/favicon.ico');
});

this.events.on('kernel_idle.Kernel', function () {
that.save_widget.update_document_title();

This comment has been minimized.

@takluyver

takluyver Oct 17, 2016
Member

We probably don't need this on each kernel idle event now, as we're no longer changing the title on kernel busy.

This comment has been minimized.

@gnestor

gnestor Oct 17, 2016
Author Contributor

I'm not sure I follow... It current changes the favicon to the busy icon on 'kernel_busy.Kernel' and back to normal on 'kernel_idle.Kernel' same for 'kernel_starting.Kernel kernel_created.Session' and 'kernel_ready.Kernel'. If we remove this from kernel idle, then the favicon will indicate that the kernel is busy even after it's idle/done.

This comment has been minimized.

@takluyver

takluyver Oct 17, 2016
Member

I meant specifically this line that calls update_document_title().

This comment has been minimized.

@gnestor

gnestor Oct 17, 2016
Author Contributor

👌

@Carreau
Copy link
Member

@Carreau Carreau commented Oct 17, 2016

I kicked Appveyor.

@gnestor gnestor force-pushed the gnestor:busy-favicon branch from e820ca2 to b5cff95 Oct 17, 2016
@takluyver
Copy link
Member

@takluyver takluyver commented Oct 18, 2016

Appveyor is having constant problems with Python 2 right now, failing with the error "The input line is too long." I don't know what to do about that - I'm ignoring it for now...

@takluyver takluyver merged commit cb88603 into jupyter:master Oct 18, 2016
1 of 2 checks passed
1 of 2 checks passed
continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@minrk
Copy link
Member

@minrk minrk commented Oct 18, 2016

Nice!

@gnestor gnestor deleted the gnestor:busy-favicon branch Oct 18, 2016
@gnestor gnestor added this to Merged PRs in 5.0 Feb 4, 2017
@oessaid
Copy link

@oessaid oessaid commented Apr 10, 2019

Is this supposed to work on Safari ? I'm using Version 12.1 (14607.1.40.1.4), enabled favicons in settings>tabs but the icon is stuck on its idle version regardless of kernel state.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
5.0
Merged PRs
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
You can’t perform that action at this time.