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
Pass ATB param better #286
Conversation
@@ -9,9 +9,7 @@ const ATB = require('./atb.es6') | |||
chrome.runtime.onInstalled.addListener(function (details) { | |||
if (details.reason.match(/install/)) { | |||
ATB.updateATBValues() | |||
|
|||
// need to wait a bit for ATB to be set | |||
setTimeout(() => ATB.openPostInstallPage(), 2000) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice that this timeout could be removed now that we definitely know when ATB param is retrieved.
}) | ||
} | ||
|
||
resolve(tabs.map(tab => tab.url)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm just passing the URLs from here, then reading them back from within the ATB module. That way it's easier to add tests.
}) | ||
let getDDGTabUrls = () => { | ||
// we don't currently support getting ATB from install page on Safari | ||
return Promise.resolve([]) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It shouldn't be too difficult to actually return URLs from here, but I guess it's not a codepath we actually care about if Safari only does gallery installs now.
👍 |
Reviewer: @jdorweiler
CC: @bsstoner
Description:
This gets the extension to pick up ATB param on install from the query string.
Steps to test this PR:
On FF/Chrome:
v123-4ab
.Automated tests:
Reviewer Checklist:
PR Author Checklist: