-
Notifications
You must be signed in to change notification settings - Fork 367
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
Bug-fix (minor?) addons group #7242
base: master
Are you sure you want to change the base?
Conversation
For the addon setting migration process
webpages/settings/index.js
Outdated
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 also have written code for enabling fix-uploaded-svgs
for 10% of users, but I'm not sure if I want to include that in this PR without some discussion first.
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.
@WorldLanguages, how do you feel about this?
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.
For ten percent of users? What does that mean?
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.
For ten percent of users? What does that mean?
Just using Math.random() and if it's less than 0.1, enable it if it's currently not, otherwise don't. (As part of the migration logic for transitioning to v1.37 or whenever)
The point being, it would reduce the impact of any undetected bugs in the addon. (After what happened with Faster project loading)
Anyway, do you have any opinions on enabling an addon by default for a subset of users? Should we do that at all or just enable it for everyone?
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.
how do you feel about this?
I know it's been brought up before but I feel like this is the sort of thing that a beta version would help us solve. Google does staggered rollouts for basically everything and it's super annoying because even if I want a new feature I have no choice but to wait.
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.
Just using Math.random() and if it's less than 0.1
Hmm... Sounds weird
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.
Just using Math.random() and if it's less than 0.1
Hmm... Sounds weird
Probably should open an issue first for that, then.
Related open PR: #4515 |
@@ -48,8 +48,8 @@ | |||
"default": "none" | |||
} | |||
], | |||
"tags": ["editor", "recommended"], | |||
"enabledByDefault": false, | |||
"tags": ["editor", "recommended", "minor"], |
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.
This is the only addon with a setting and the new tag.
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.
TODO:
- If the
disable-sprite-wobble
addon gets enabled automatically, the"mode"
setting should be reset.
@@ -686,10 +695,16 @@ chrome.storage.sync.get([...ADDON_SETTINGS_KEYS, "addonsEnabled"], (storageItems | |||
|
|||
// Finally, minify the settings and store them in the scratchAddons object | |||
const prerelease = chrome.runtime.getManifest().version_name.endsWith("-prerelease"); | |||
const currentVersion = chrome.runtime.getManifest().version; | |||
console.log(`Transitioned from ${lastVersionStr} \u2192 ${currentVersion}`); |
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.
Maybe should only log this if changes were made.
What should we do with Better editor comments? There are also a couple forum bugfix addons:
|
I know, right?
I'll include that once #6915 is merged. It changes the addon ID.
That one I think might be best left for the user to decide on, but maybe. |
Does this actually resolve #7152? I thought that issue was about using setting descriptions and a single addon for most bug fixes. |
It doesn't resolve the issue but will close the issue nonetheless as it won't be implemented because other alternatives has been chosen. |
This would also be the right time to change forums to a website subcategory and not a group. |
I think the main reason the forum addons are in a separate group is because not everyone uses the forums and the forums are a smaller area of the entire website. |
Almost no one uses the forums. |
Yeah, that's a better way of putting it. 😄 |
We could just make it list them at the bottom. |
The way addons are sorted is already confusing: #3012 |
Then let's just put forum bugfix addons as a bugfix. |
I don't think addons in the bug fixes group need the "recommended" tag.
There could be a separate "bug fixes on this tab" group below "addons on this tab". |
Just a comment: I haven't seen this PR in detail yet, but if there's consensus in favor of this I will just merge it :) |
Personally, I think this is a good idea. As these bug-fix addon usually don't have settings and there's no good reason to disable them, I don't see the point of listing them in the "enabled" group. These should, however, still be listed within the "addons (running) on this tab" group when using the settings page from the popup. |
Co-authored-by: Maximouse <51849865+mxmou@users.noreply.github.com>
Resolves #2617, resolves #7152
Changes
Adds a group to the addon list in the settings page called "Bug fixes" (we can change this as we see fit). These addons...
Reason for changes
Some addons fix bugs and those addons deserve to be enabled by default, but that would also clutter the "Enabled" group in the list of addons. The "bug fixes" group allows us to set them aside, making more room for addons that users would actually want to take a look at.
Tests
Tested on Edge 122.
To-Dos
fix-uploaded-svgs
?We might also want to address any confusion around why these addons aren't showing up under "Enabled".