-
Notifications
You must be signed in to change notification settings - Fork 75
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
Add Stackoverflow Search Navigation #433
Conversation
This pull request introduces 1 alert when merging af30687 into 222feb3 - view on LGTM.com new alerts:
|
c879ad5
to
b702d53
Compare
Looks good overall, left minor style comments. Please also update the CHANGELOG and README. |
@@ -413,7 +413,7 @@ class GoogleSearch { | |||
observer.observe(container, { | |||
attributes: false, | |||
childList: true, | |||
subtree: false, | |||
subtree: true, |
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.
Is this change related to the PR?
@@ -69,6 +69,7 @@ const OPTIONAL_PERMISSIONS_URLS = { | |||
'/*', | |||
), | |||
'github': ['https://github.com/*'], | |||
'stackoverflow': ['https://stackoverflow.com/*', 'https://www.stackoverflow.com/*'], |
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.
Please change to stack-overflow
, and for the JS variables below use stackOverflow
@@ -141,6 +142,10 @@ class OptionsPageManager { | |||
github.addEventListener('change', () => { | |||
setSearchEnginePermission_(github); | |||
}); | |||
const stackoverflow = document.getElementById('stackoverflow'); |
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.
Please use stackOverflow
@@ -241,6 +246,11 @@ class OptionsPageManager { | |||
github.checked = OPTIONAL_PERMISSIONS_URLS['github'].every((url) => { | |||
return permissions.origins.includes(url); | |||
}); | |||
const stackoverflow = document.getElementById('stackoverflow'); |
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.
Please use stackOverflow
@@ -228,6 +228,11 @@ <h2>EXPERIMENTAL: Alternative search engines</h2> | |||
<input type="checkbox" id="github"> Enable on Github | |||
</label> | |||
</div> | |||
<div class="option"> | |||
<label for="stackoverflow"> |
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.
stack-overflow here as well
// Question search result | ||
{ | ||
nodes: document.querySelectorAll(resultSelector), | ||
highlightClass: 'div.s-post-summary', |
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.
To enable customization with custom CSS, it would be best to use a custom highlight class like wsn-stack-overflow-focused-post
Hi everyone, I would be really interested in this feature as many others. Would it be possible to merge it? Or how can I help here? Thanks! |
Hi @HerrAugust I may look into it later |
Closes #429
Checkpoints: