Add Whitelist #372
Add Whitelist #372
Conversation
base functionality
just show data in a text input no fancy formatting needed!
|
|
... via settings
still needs to update after press
|
|
- Button now reflects whitelist status instantly - Fixed bug where only last site whitelisted displayed properly - No longer breaks when spammed - Removed test code
|
I'm marking this as ready for review, before merging a few things need to happen
Not going to do the last task, a bit above my skill level |
|
@KevinRoebert do you have any plans to merge this? |
|
Would be great to have it, as currently clearurls breaks amazon sorting, resulting in user not being able to select any other sorting. |
|
@Banaanae First of all, thank you for your contribution. I am currently reviewing the code. It still needs some changes and also has a couple of bugs. I will make the changes directly and then merge. |
| * Skip whitelisted sites | ||
| */ | ||
| for (const site of storage.whitelist) { | ||
| if (url.indexOf(site) != -1) { |
| siteFound = true | ||
| } | ||
| }); | ||
| if (!siteFound) { |
| } | ||
| }); | ||
| if (!siteFound) { | ||
| if (data.response.length != 0) { |
There was a problem hiding this comment.
Remove this If. It causes the button to retain the "btn-danger" class when a page is added and immediately removed.
| function: "getData", | ||
| params: ['whitelist'] | ||
| }).then((data) => { | ||
| data.response.forEach(site => { |
There was a problem hiding this comment.
Should be replaced by the following: let siteFound = data.response.some(site => currentSite.indexOf(site) !== -1);
| * @param {boolean} remove If true remove current site instead of adding | ||
| */ | ||
| function changeWhitelist(removeWl = false) { | ||
| if (removeWl != true) { // Handle click obj |
There was a problem hiding this comment.
Assignment is useless. It is better to pass the correct state in the onclick event
| element.classList.replace('btn-danger', 'btn-primary') | ||
| } | ||
| element.textContent = translate('popup_html_configs_whitelist_button_add') | ||
| document.getElementById('whitelist_btn').onclick = changeWhitelist; |
There was a problem hiding this comment.
Pass the desired state here: () => {changeWhitelist(true)}
| }).then((data) => { | ||
| let siteUrl = new URL(site) | ||
| let domain = siteUrl.hostname | ||
| if (removeWl == false) { |
| if (removeWl == false) { | ||
| data.response.push(domain) | ||
| } else { | ||
| data.response = data.response.filter(wlSite => wlSite != domain) |
There was a problem hiding this comment.
Unsafe comparison, better !==
|
Thanks for your work, just noticed as I was posting in the other thread about a new issue I found, unrelated to your fix. I'm gonna quote my post as to not retype it, just thought if your actively working on it you might be interested. :)
Personally, I still use this mod BUT it's really good to get a list of places and situations, so we can be aware to disable manually Looking forward to trying your fixes! :D |
This PR adds a whitelist button, which then allows users to prevent sites from being filtered
Whitelisting is now pretty much finished, only the issue with subdomains being treated like separate sites remain, just waiting on maintainer's opinions on adding another library. (If current behaviour is also like there could be an option in settings)
This PR has only been tested on firefox, if chrome users could test it be greatly appreciated
www.example.comis whitelisted,example.comisn't. This could be considered intentional, but it would still break cases likewww.example.comcallingassets.example.com)Whitelist SitewithRemove From Whitelistwhen viewing a whitelisted page)Once done this will close #93 and its many duplicates