-
Notifications
You must be signed in to change notification settings - Fork 42
update: Add 'call to action' #37
base: master
Are you sure you want to change the base?
Conversation
@bsstoner to test this out I suggest you do the following:
Thanks! |
if (os !== 'w') { | ||
chrome.tabs.create({ url: 'chrome://settings/searchEngines' }); | ||
chrome.notifications.create({ | ||
type: "basic", |
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.
@bsstoner This is indeed very basic and might not work as well as we would like it to.
It might be interesting to add some Rich notifications, especially with some nice pictures which will make the whole process easier for everyone.
@@ -51,6 +51,7 @@ | |||
"contextMenus", | |||
"webRequest", | |||
"webRequestBlocking", | |||
"*://*.duckduckgo.com/" | |||
"*://*.duckduckgo.com/", | |||
"notifications" |
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 do not like it very much, but this permission
is required -- otherwise we won't be able to trigger the popup/notification.
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.
Does it introduce an extra prompt on install?
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.
818d68f
to
347fbb7
Compare
Is there a way we can test this as part of the homepage flow before merging/pushing out the update? |
@bsstoner hmm, good question. The only way that comes to mind is creating a new, dummy, unlisted extension which will be used just for testing. |
347fbb7
to
35a2e4a
Compare
* Add Call to Action for non-windows platforms. Signed-off-by: mr.Shu <mr@shu.io>
35a2e4a
to
82c9867
Compare
Signed-off-by: mr.Shu mr@shu.io