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

Use pageAction instead of browserAction? #1

Closed
timnew opened this issue Oct 27, 2015 · 13 comments
Closed

Use pageAction instead of browserAction? #1

timnew opened this issue Oct 27, 2015 · 13 comments

Comments

@timnew
Copy link

timnew commented Oct 27, 2015

@zalmoxisus Do you think it would be nicer it the extension is registered as pageAction instead of browserAction. Since we only use it in particular pages instead of every one.

Before the extension can recognize the redux store, we can use it as a permanent page action(displayed on every page)?

@zalmoxisus
Copy link
Owner

Yes, good suggestion. Will implement this when we'll have an option page first, where we may indicate for which pages to use it.

@timnew
Copy link
Author

timnew commented Oct 27, 2015

I have a suggestion, how about this:

  1. We disable the page action by default.
  2. We can inject the devToolsExtension to window object with content script on every page.
  3. Then we can when the devToolExtension is invoked, then we can send a message to backend script via sendMessage
  4. When the backend script received the message, then it enable the pageAction for the tab that sent the message(we can get the tab id in the message sender).

Then the extension have the capability to know when it should be enabled.

@timnew
Copy link
Author

timnew commented Oct 27, 2015

I check the code, I think the only block is that the store is a singleton in the extension.
So it might be a problem if we are debugging multiple tabs with the extension simultaneously.
But I think it can be fixed, if we monitor the tab lifecycle events?!

@zalmoxisus
Copy link
Owner

Yes, it is a good solution, but we also need to enable/disable specific urls explicitly, for example in production (instead of disabling/enabling the extension).

@timnew
Copy link
Author

timnew commented Oct 27, 2015

Hmm, sorry I missed this part.

I imagine that we could use Dan's multiple file solution.

But anyway, we restrict the behavior against specific url, then we can avoid to inject content script to every tab. Instead, we use declarative web request to inject content script on demand.

Only when the url matches the criteria, the devToolsExtension is injected. It also avoid the potential security issue.

@zalmoxisus
Copy link
Owner

Thanks, @timnew! It is much better with a pageAction.

If you want to test it before the next release, just run npm run build:extension.

About debugging multiple tabs, it's the same as for the browserAction. The popup/window's store is updated if you open/refresh the page or dispatch an action, otherwise you have their the devtools for the last active tab (as in demo). I'm not sure whether it is a bug or a feature, but I found it useful. If someone wants to have there only the devtools for the current tab, just file and issue, it is easy to fix.

@zalmoxisus
Copy link
Owner

Fixed the issue with multiple tabs in 49b5298.

@timnew
Copy link
Author

timnew commented Oct 28, 2015

awesome, I was thinking maybe I can provide a pull request later today.

I'll try latest release today!

Cheers,
Tim

Sent from my Galaxy Note 3
On Oct 28, 2015 4:15 AM, "Mihail Diordiev" notifications@github.com wrote:

Fixed the issue with multiple tabs in 49b5298
49b5298
.


Reply to this email directly or view it on GitHub
#1 (comment)
.

@zalmoxisus
Copy link
Owner

Feel free to reopen the issue if something goes wrong.
Opened another issue for the option page: #3.

@timnew
Copy link
Author

timnew commented Oct 28, 2015

Got it. Do you have any idea about how to build options page? Redux + React?

Actually, the stuff I'm building with redux and react is the option page for a chrome extension. 😂

@zalmoxisus
Copy link
Owner

It's as easy as an browser/page action page. I do not see nothing special. Just would be great to choose a React lib for the forms' UI. And of course, we need to save the configuration to chrome.storage (.sync/.local) and read it from the content script or receive it from the background script. The latter would be preferable (though more complicated) as to be compatible with Firefox (because firefox extensions do not have access to chrome.storage from the content script).

UPD: the first realisation will work in firefox when the issue is fixed.

@zalmoxisus
Copy link
Owner

If you found something different in the option page, we may add it in our boilerplate.

@timnew
Copy link
Author

timnew commented Oct 28, 2015

Sure!

I might need to get familiar with FF web extension first. I have no experience about it before. As you mentioned, there is some minor differences might impact on the implementation.

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

No branches or pull requests

2 participants