-
Notifications
You must be signed in to change notification settings - Fork 324
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
feat: remember manual opt-ins and opt-outs per site #929
Conversation
This improves the way we handle manual opt-out performend per site by the user by storing both manual opt-outs and opt-ins. This way we are able to tweak implicit default per website while respecting preexisting user choices. Also, in the future we may run opt-in metrics that compare lengths of both lists to gain better insight into user's behavior. Closes #921
add-on/src/lib/options.js
Outdated
const { enabledOn, disabledOn } = await storage.get(['enabledOn', 'disabledOn']) | ||
for (const fqdn of [ | ||
'proto.school', // https://github.com/ipfs-shipyard/ipfs-companion/issues/921 | ||
'app.fleek.co' // TODO: confirm if ok |
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.
Janison from Fleek here - this looks good. Thanks for this. cc @studna
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.
Suggested some prefs text tweaks for clarity ...
Co-authored-by: Jessica Schilling <jessica@protocol.ai>
Co-authored-by: Jessica Schilling <jessica@protocol.ai>
Co-authored-by: Jessica Schilling <jessica@protocol.ai>
Co-authored-by: Jessica Schilling <jessica@protocol.ai>
We only support hostnames
@jessicaschilling merged your suggestions, but removed "URI" from preference label |
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.
LGTM, thank you!
This ensures that toggling a subdomain creates a rule for subdomain and does not impact existing rules for parents. If there is no direct opt-in or opt-out, parents are matched. If there is opt-out for a parent, but also a direct opt-in for subdomain, direct opt-in takes precedence.
I'm going to ship this to Beta soon, so we can dogfood it more, |
This PR improves the way we handle manual opt-out performed per site by the user by storing both opt-out and opt-in decisions made by the user.
Changes
Explicit opt-in and opt-out lists in Preferences
noIntegrationsHostnames
.disabledOn
.Menu toggle adds/moves hostname between lists
however the expectation is that most of the time users will not touch them.
Overriding implicit default per website
Short term (this PR)
proto.school
to opt-out list if it was not already present on either of lists.app.fleek.co
– feat: remember manual opt-ins and opt-outs per site #929 (review) (does not work from local gateway atm due to stripe.com limitation)Long term (separate PR)
We will be looking into supporting opt-out based on HTTP header or DNS, so any IPFS-powered website can change the implicit behaviour to opt-out. Tracked in #930
Closes #921 cc @terichadbourne @jessicaschilling