-
Notifications
You must be signed in to change notification settings - Fork 96
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 separate whitelist and blocklist #14
base: master
Are you sure you want to change the base?
Conversation
This avoids the problem of the whitelist clobbering the blacklist or vice versa.
Interesting idea! Good call for usability :) I feel like, before deploying, we'd need to think a bit more about a default whitelist that would work for many users — clearly it's harder than a default blacklist, and a truly perfect solution is impossible, but we can definitely arrive at a good starting point. The default blacklist was based on a list of the 50 (I think) most popular sites on the web and deciding which are almost never work-related. The default whitelist oughta be based on that same pattern rather than what we as developers just so happen to consider our work-related sites. Also, I'd like to keep the new naming convention of "site" instead of "domain" for pref keys, since we now support path-based entries like "google.com/calendar", which is a site but isn't a domain. After those changes, I'll give the merge a final look for style consistency and run a bunch of paranoid checks on the new prefs-update function (unless maybe you've already done 'em?), and we'll move from there :) Thanks! |
I agree that we should change the default whitelist, as currently it's just my whitelist unchanged. I changed the names according to your convention, as well as the name of "whitelistEl" to "whitelistSelectEl", since I think that's clearer. I haven't tested the prefs-update, so I'd be glad if you did that. Thanks! |
Users should probably explicitly add sites that they want to be whitelisted. Since the default is to use a blacklist, users who "go with the defaults" will not be affacted.
On more thought, I think it probably makes most sense for the default whitelist to be empty. Since users must go into the options to switch away from the blacklist, I think it's reasonable to expect that they have some idea of which sites to whitelist. Users who don't change the default options will never interact with the default whitelist anyway. |
That makes good sense. Sorry this pull request has been so thoroughly back-burnered. I'm hoping to be able to get a new release out this month once exams are done, so we'll see how that goes :) Thanks for your patience, bro. |
Currently, there is only one "site list" that is used for both the blacklist and whitelist. This means that users who want to switch between these two types of blocking must clobber one list every time.
This patch fixes this problem by adding two separate lists for the whitelist and blacklist.