Add ability to disable all the features! #877
Conversation
Each feature has its own file; start/stop logic should not be outside. This keeps function names as intended and not `setup`
|
Good idea. I think this is a good balance and now we have a place to refer most "option request" issues. |
sindresorhus
changed the title
|
Cool! Give it a spin and let’s merge after we update the readme and extend the field description. Commits or suggestions welcome. |
source/libs/utils.js
Outdated
| */ | ||
| export const safely = async fn => fn(); | ||
| export const enableFeature = async (fn, fileName) => { |
There was a problem hiding this comment.
Filename is most commonly used as one word, not file name.
source/options.html
Outdated
| <input type="checkbox" name="hideStarsOwnRepos"> | ||
| Hide other users starring/forking your repos | ||
| </label> | ||
| List the features to disable, by <a href="https://github.com/sindresorhus/refined-github/tree/master/source/features" target="_blank">file name</a>: <br> |
source/options.html
Outdated
| List the features to disable, by <a href="https://github.com/sindresorhus/refined-github/tree/master/source/features" target="_blank">file name</a>: <br> | ||
| <textarea name="disabledFeatures" rows="5" placeholder="For example: | ||
| log-active-features | ||
| hide-own-stars |
There was a problem hiding this comment.
It's weird to use literal newlines here. I would use \n escape instead.
There was a problem hiding this comment.
\n doesn’t work but this might https://stackoverflow.com/a/983988
source/libs/utils.js
Outdated
| export const safely = async fn => fn(); | ||
| export const enableFeature = async (fn, fileName) => { | ||
| const {disabledFeatures} = await options; | ||
| const log = disabledFeatures.includes('log-active-features') ? () => {} : console.log; |
There was a problem hiding this comment.
Why have this as an option instead of just enabled by default when in dev mode?
There was a problem hiding this comment.
Because it’s a user-facing feature also. One might want to see what’s enabled/disabled on a page. Maybe?
There was a problem hiding this comment.
Because it’s a user-facing feature also. One might want to see what’s enabled/disabled on a page. Maybe?
There was a problem hiding this comment.
Could we have a checkbox to enable dev/debug mode instead? It doesn’t really feel like a feature in that sense.
There was a problem hiding this comment.
What about enabling logging by default with console.group(), or even better console.groupCollapsed(), so that way console is not cluttered and the user still gets to see what features are enabled.
It doesn’t really feel like a feature in that sense
👍
There was a problem hiding this comment.
I don't think extensions should clutter the console by default, even if it's just one item.
There was a problem hiding this comment.
Let's go with a checkbox that is unchecked by default. It can be useful for other production debugging we might want later on too.
source/background.js
Outdated
| }, | ||
| migrations: [ | ||
| options => { | ||
| if (options.hideStarsOwnRepos === true) { |
|
|
source/background.js
Outdated
| @@ -5,9 +5,16 @@ import dynamicContentScripts from 'webext-dynamic-content-scripts'; | |||
| // Define defaults | |||
| new OptionsSync().define({ | |||
| defaults: { | |||
| hideStarsOwnRepos: true | |||
| disabledFeatures: 'log-active-features', | |||
There was a problem hiding this comment.
Shouldn't this be empty now?
source/background.js
Outdated
| @@ -5,9 +5,16 @@ import dynamicContentScripts from 'webext-dynamic-content-scripts'; | |||
| // Define defaults | |||
| new OptionsSync().define({ | |||
| defaults: { | |||
| hideStarsOwnRepos: true | |||
| disabledFeatures: 'log-active-features', | |||
| logFeatures: false | |||
There was a problem hiding this comment.
I was thinking we make this more generic so we could use it for other production logging in the future. This locks it into just being logging of features.
There was a problem hiding this comment.
Done. When that happens the feature can be developed further.
|
Awesome! Some people are gonna love this. |
|
Yep! Can you update the readme? |
|
Sure |
Note:
npm installneeded.So here we are... 😅
Next:
The rawness of these options should discourage people from disabling stuff but they can if they try hard enough.
Fixes #779 #585 #515 #285 #193 #174 #131 #57 #10
Sadly doesn't fix any of the open change request since they're CSS-related.