-
Notifications
You must be signed in to change notification settings - Fork 343
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
Allow disabling the onboarding panel from enterprise policies #2631
base: main
Are you sure you want to change the base?
Allow disabling the onboarding panel from enterprise policies #2631
Conversation
src/js/popup.js
Outdated
try { | ||
disableOnboarding = await browser.storage.managed.get("disableOnboarding"); | ||
} catch (e) { | ||
console.error( |
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 don't think we need an error here. This is the normal case.
Does it actually throw if it isn't there?
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.
It does. We have bug 1868153 to track it upstream.
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.
Let's not show the error though since nothing can be done.
src/js/popup.js
Outdated
try { | ||
disableOnboarding = await browser.storage.managed.get("disableOnboarding"); | ||
} catch (e) { | ||
console.error( |
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.
Let's not show the error though since nothing can be done.
0c4e802
to
299057e
Compare
Before submitting your pull request
npm test
and all tests passed.Description
Allow disabling the onboarding panel from enterprise policies.
policies.json example:
Note: I didn't implement a policy to disable only the feature to connect to Mozilla VPN as requested because I prefer to do it in a separated patch.
Type of change
Select all that apply.
Tag issues related to this pull request:
Additional information
cc'ing @mkaply for a review on this too.