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

Store state of lighthouse in extension while running. #1185

Merged
merged 2 commits into from
Dec 30, 2016

Conversation

wardpeet
Copy link
Collaborator

When extension popup is closed & lighthouse is still running we show the state lighthouse is running in instead of showing the startup screen.
schermopname

When extension popup is closed & lighthouse is still running we show the state lighthouse is running in instead of showing the startup screen.
Copy link
Collaborator

@patrickhulce patrickhulce left a comment

Choose a reason for hiding this comment

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

nice work!

@@ -128,6 +128,10 @@ document.addEventListener('DOMContentLoaded', _ => {
return frag;
}

if (background.isRunning()) {
startSpinner();
Copy link
Collaborator

Choose a reason for hiding this comment

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

not super familiar with the code here, but i'm assuming we're already stopping the spinner everywhere we need to? :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes whenever runner fails promise then or catch than we do a stopSpinner. Not sure if Start and Stop spinner is still a good name for it. Maybe something a long the lines of showProcessPage or just showProcess?

@@ -213,7 +227,20 @@ window.loadSelectedAggregations = function() {
};

window.listenForStatus = function(callback) {
log.events.addListener('status', callback);
log.events.addListener('status', function(args) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: maybe just log or something other than args? staring at it a bit assuming it was a list ala arguments and not just the one input

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

thanks :) sounds better indeed 👍

Copy link
Contributor

@ebidel ebidel left a comment

Choose a reason for hiding this comment

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

Tested locally. Really great addition!

@ebidel ebidel merged commit e98cefd into GoogleChrome:master Dec 30, 2016
@paulirish
Copy link
Member

This is killer. nice work @wardpeet

andrewrota pushed a commit to andrewrota/lighthouse that referenced this pull request Jan 13, 2017
)

* Store state of lighthouse in extension while running.

When extension popup is closed & lighthouse is still running we show the state lighthouse is running in instead of showing the startup screen.

* Changed variable name args to log (review)
@wardpeet wardpeet deleted the extension-save-state branch July 24, 2017 05:07
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.

None yet

4 participants