From e040b3e85929cd7ebc7a567605b29ed80d2cc191 Mon Sep 17 00:00:00 2001 From: lisa-wolfgang <43426138+lisa-wolfgang@users.noreply.github.com> Date: Mon, 18 Jul 2022 18:17:30 -0500 Subject: [PATCH 01/10] Add quality guidelines, mention in addon tutorial --- .../getting-started/creating-an-addon.md | 5 +++-- .../getting-started/quality-guidelines.md | 18 ++++++++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) create mode 100644 content/docs/develop/getting-started/quality-guidelines.md diff --git a/content/docs/develop/getting-started/creating-an-addon.md b/content/docs/develop/getting-started/creating-an-addon.md index dd8d3157f..a8dc090d9 100644 --- a/content/docs/develop/getting-started/creating-an-addon.md +++ b/content/docs/develop/getting-started/creating-an-addon.md @@ -78,5 +78,6 @@ Make sure your addon doesn't break other addons. Make sure your code is understandable; having unnecessary comments is better than no comments. ## Step 12: Send a pull request! -Fork the repo if you haven't already, commit your new addon and send a PR! -Keep in mind we might request you to make some changes, however, we will probably accept your addon as long as it's minimally suitable. +Fork the repo if you haven't already, create a new branch, commit your new addon, and send a PR! +Keep in mind we might request that you make some changes. However, we will probably accept your addon as long as it's minimally suitable. +To keep the process smooth and improve the chances of your PR being merged, you should review the [quality guidelines](/docs/develop/getting-started/quality-guidelines) that we abide by. diff --git a/content/docs/develop/getting-started/quality-guidelines.md b/content/docs/develop/getting-started/quality-guidelines.md new file mode 100644 index 000000000..0aa3fa730 --- /dev/null +++ b/content/docs/develop/getting-started/quality-guidelines.md @@ -0,0 +1,18 @@ +--- +title: Quality Guidelines +aliases: + - /docs/developing/getting-started/quality-guidelines +--- +Keep these guidelines in mind when creating or reviewing a pull request (PR). If you're not sure about a certain guideline when creating a PR, you can submit the PR as a draft and mention that you need help with a specific aspect of your PR. Many developers and community members are willing and able to help your PR improve! + +All PRs: +- should have sound reasoning as to why they should be merged. This should be specified under the "Reason for changes" header when writing the PR description. Even if it seems obvious, others may need the additional context. PRs with little to no reason to be merged are likely to be delayed, ignored, or even closed. +- must support both the Chromium and Firefox browser engines. Avoid using new or experimental web technologies unless a fallback is implemented that preserves feature parity. View the [unsupported browser](https://scratchaddons.com/unsupported-browser/) page to determine the current minimum versions that must be supported. +- must be tested thoroughly. We have a very large userbase, so it's especially important that all possible use cases are considered. Every PR should be tested on a Chromium-based browser (Chrome, Edge, Brave, or another) **and** Firefox. Any tests done should be added to the "Tests" section of the PR description. +- must have **all** code reviewed line-by-line. **Org members, this is required in order to give approval.** +- should be reviewed by someone familiar with the aspects that the PR is changing. This isn't a hard requirement, but a person like this will help iron out edge cases and minor issues. + +New addons: +- must have clear, concise, and easy-to-understand titles, descriptions, info notices (if any), and setting names (if any). English is required: the base addon manifest is used as a reference for our translators. The developers and members of the community should scrutinize and help with this. An effective meter for this is to test it **blindly** -- that is, download and try out the addon before reading the PR description. +- must be easy to use. The labels and design language used in any UI that the addon adds or modifies should be understandable by whoever will be using that addon. Again, testing blindly is an effective meter for this. Addons without an effective UI communicating its changes will be ranked lower than other addons in the settings page. +- must be compatible with our existing addons. This doesn't just mean avoiding bugs: addons should complement each other by supporting each other's features. For example, an addon that has a user interface should support our respective dark mode addon. View the [compatibility checklist](/docs/reference/compatibility-checklist) for common issues to consider. \ No newline at end of file From 0be88bacf5c42681ce1d284f23fc8d185e3c642f Mon Sep 17 00:00:00 2001 From: lisa-wolfgang <43426138+lisa-wolfgang@users.noreply.github.com> Date: Mon, 18 Jul 2022 19:37:22 -0500 Subject: [PATCH 02/10] Change addon compat page name --- content/docs/reference/addon-compatibility-checklist.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 content/docs/reference/addon-compatibility-checklist.md diff --git a/content/docs/reference/addon-compatibility-checklist.md b/content/docs/reference/addon-compatibility-checklist.md new file mode 100644 index 000000000..6e5ea7e65 --- /dev/null +++ b/content/docs/reference/addon-compatibility-checklist.md @@ -0,0 +1,5 @@ +--- +title: Addon Compatibility Checklist +aliases: + - /docs/developing/addon-compatibility-checklist +--- From d63e18231754528e135d6fe152577f135ed4b955 Mon Sep 17 00:00:00 2001 From: lisa-wolfgang <43426138+lisa-wolfgang@users.noreply.github.com> Date: Mon, 18 Jul 2022 22:13:41 -0500 Subject: [PATCH 03/10] Compat list additions - Navbar - Editor (general) - Stage header --- .../addon-compatibility-checklist.md | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/content/docs/reference/addon-compatibility-checklist.md b/content/docs/reference/addon-compatibility-checklist.md index 6e5ea7e65..219e142d8 100644 --- a/content/docs/reference/addon-compatibility-checklist.md +++ b/content/docs/reference/addon-compatibility-checklist.md @@ -3,3 +3,40 @@ title: Addon Compatibility Checklist aliases: - /docs/developing/addon-compatibility-checklist --- + +When developing an addon, make sure to review the categories that you're editing for any factors that need to be considered. + +# Non-Editor Addons + +## Navbar +New addons that modify the navbar must be compatible with the following addons: +- Customizable navigation bar +- Expandable search bar + +# Editor Addons + +## General + +Any elements inside `
` that are modified with a userscript will be wiped and restored to vanilla Scratch when the user switches to the project page. This also happens with other parent `div`s: for example, when a new button is added to the color picker, then the color picker is closed. The convention is to reinstate custom UI elements by using a `while (true)` loop with `addon.tab.waitForElement` like so: + + ```js + while (true) { + let element = await addon.tab.waitForElement(/* CSS selector of desired parent element from regular Scratch */, { + markAsSeen: true, + } + /* Re-initialize anything related to the UI here */ + } + ``` + +## Stage Header +New addons that add elements to the stage header must be compatible with the following addons by using the `stageHeader` space with the `addon.tab.appendToSharedSpace` API: +- 60FPS project player mode +- Clone counter +- Gamepad support +- Mouse position +- Muted project player mode +- Pause button + +Other things that affect the stage header: +- Entering full screen (the `fullscreenStageHeader` space must be used) +- The "reverse order of project controls" addon From 271fda290a53fdee0e5848ba01f3ef0e17ef433d Mon Sep 17 00:00:00 2001 From: lisa-wolfgang <43426138+lisa-wolfgang@users.noreply.github.com> Date: Mon, 18 Jul 2022 22:24:44 -0500 Subject: [PATCH 04/10] Compat list: fix inaccuracies in Stage Header --- content/docs/reference/addon-compatibility-checklist.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/content/docs/reference/addon-compatibility-checklist.md b/content/docs/reference/addon-compatibility-checklist.md index 219e142d8..dfec85160 100644 --- a/content/docs/reference/addon-compatibility-checklist.md +++ b/content/docs/reference/addon-compatibility-checklist.md @@ -29,7 +29,7 @@ Any elements inside `
` that are modified with a userscript will be ``` ## Stage Header -New addons that add elements to the stage header must be compatible with the following addons by using the `stageHeader` space with the `addon.tab.appendToSharedSpace` API: +New addons that add elements to the stage header must be compatible with the following addons by using their respective spaces with the [`addon.tab.appendToSharedSpace`](https://scratchaddons.com/docs/reference/addon-api/addon.tab/addon.tab.appendtosharedspace/) API: - 60FPS project player mode - Clone counter - Gamepad support @@ -38,5 +38,6 @@ New addons that add elements to the stage header must be compatible with the fol - Pause button Other things that affect the stage header: -- Entering full screen (the `fullscreenStageHeader` space must be used) +- Entering and exiting full screen (the `fullscreenStageHeader` space must be used in full screen) +- Activating Turbo Mode - The "reverse order of project controls" addon From f6765d58b99dfcc7265a4ad42505a78ddfa4b6aa Mon Sep 17 00:00:00 2001 From: lisa-wolfgang <43426138+lisa-wolfgang@users.noreply.github.com> Date: Wed, 20 Jul 2022 17:42:50 -0500 Subject: [PATCH 05/10] Quality guidelines: make testing required --- content/docs/develop/getting-started/quality-guidelines.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/content/docs/develop/getting-started/quality-guidelines.md b/content/docs/develop/getting-started/quality-guidelines.md index 0aa3fa730..e29104c72 100644 --- a/content/docs/develop/getting-started/quality-guidelines.md +++ b/content/docs/develop/getting-started/quality-guidelines.md @@ -8,7 +8,7 @@ Keep these guidelines in mind when creating or reviewing a pull request (PR). If All PRs: - should have sound reasoning as to why they should be merged. This should be specified under the "Reason for changes" header when writing the PR description. Even if it seems obvious, others may need the additional context. PRs with little to no reason to be merged are likely to be delayed, ignored, or even closed. - must support both the Chromium and Firefox browser engines. Avoid using new or experimental web technologies unless a fallback is implemented that preserves feature parity. View the [unsupported browser](https://scratchaddons.com/unsupported-browser/) page to determine the current minimum versions that must be supported. -- must be tested thoroughly. We have a very large userbase, so it's especially important that all possible use cases are considered. Every PR should be tested on a Chromium-based browser (Chrome, Edge, Brave, or another) **and** Firefox. Any tests done should be added to the "Tests" section of the PR description. +- must be tested thoroughly. We have a very large userbase, so it's especially important that all possible use cases are considered. Every PR should be tested on a Chromium-based browser (Chrome, Edge, Brave, or another) **and** Firefox. Any tests done should be added to the "Tests" section of the PR description. **Org members, this is required in order to give approval.** - must have **all** code reviewed line-by-line. **Org members, this is required in order to give approval.** - should be reviewed by someone familiar with the aspects that the PR is changing. This isn't a hard requirement, but a person like this will help iron out edge cases and minor issues. From 9f5949bcefa9ac4eae87a8e4f7971712909340cb Mon Sep 17 00:00:00 2001 From: lisa-wolfgang <43426138+lisa-wolfgang@users.noreply.github.com> Date: Wed, 20 Jul 2022 20:58:45 -0500 Subject: [PATCH 06/10] Remove addon compat list It will get its own PR --- .../getting-started/quality-guidelines.md | 2 +- .../addon-compatibility-checklist.md | 43 ------------------- 2 files changed, 1 insertion(+), 44 deletions(-) delete mode 100644 content/docs/reference/addon-compatibility-checklist.md diff --git a/content/docs/develop/getting-started/quality-guidelines.md b/content/docs/develop/getting-started/quality-guidelines.md index e29104c72..ac4dffa0c 100644 --- a/content/docs/develop/getting-started/quality-guidelines.md +++ b/content/docs/develop/getting-started/quality-guidelines.md @@ -15,4 +15,4 @@ All PRs: New addons: - must have clear, concise, and easy-to-understand titles, descriptions, info notices (if any), and setting names (if any). English is required: the base addon manifest is used as a reference for our translators. The developers and members of the community should scrutinize and help with this. An effective meter for this is to test it **blindly** -- that is, download and try out the addon before reading the PR description. - must be easy to use. The labels and design language used in any UI that the addon adds or modifies should be understandable by whoever will be using that addon. Again, testing blindly is an effective meter for this. Addons without an effective UI communicating its changes will be ranked lower than other addons in the settings page. -- must be compatible with our existing addons. This doesn't just mean avoiding bugs: addons should complement each other by supporting each other's features. For example, an addon that has a user interface should support our respective dark mode addon. View the [compatibility checklist](/docs/reference/compatibility-checklist) for common issues to consider. \ No newline at end of file +- must be compatible with our existing addons. This doesn't just mean avoiding bugs: addons should complement each other by supporting each other's features. For example, an addon that has a user interface should support our respective dark mode addon. \ No newline at end of file diff --git a/content/docs/reference/addon-compatibility-checklist.md b/content/docs/reference/addon-compatibility-checklist.md deleted file mode 100644 index dfec85160..000000000 --- a/content/docs/reference/addon-compatibility-checklist.md +++ /dev/null @@ -1,43 +0,0 @@ ---- -title: Addon Compatibility Checklist -aliases: - - /docs/developing/addon-compatibility-checklist ---- - -When developing an addon, make sure to review the categories that you're editing for any factors that need to be considered. - -# Non-Editor Addons - -## Navbar -New addons that modify the navbar must be compatible with the following addons: -- Customizable navigation bar -- Expandable search bar - -# Editor Addons - -## General - -Any elements inside `
` that are modified with a userscript will be wiped and restored to vanilla Scratch when the user switches to the project page. This also happens with other parent `div`s: for example, when a new button is added to the color picker, then the color picker is closed. The convention is to reinstate custom UI elements by using a `while (true)` loop with `addon.tab.waitForElement` like so: - - ```js - while (true) { - let element = await addon.tab.waitForElement(/* CSS selector of desired parent element from regular Scratch */, { - markAsSeen: true, - } - /* Re-initialize anything related to the UI here */ - } - ``` - -## Stage Header -New addons that add elements to the stage header must be compatible with the following addons by using their respective spaces with the [`addon.tab.appendToSharedSpace`](https://scratchaddons.com/docs/reference/addon-api/addon.tab/addon.tab.appendtosharedspace/) API: -- 60FPS project player mode -- Clone counter -- Gamepad support -- Mouse position -- Muted project player mode -- Pause button - -Other things that affect the stage header: -- Entering and exiting full screen (the `fullscreenStageHeader` space must be used in full screen) -- Activating Turbo Mode -- The "reverse order of project controls" addon From 05dc38eb49862549a021275fc5ef7877fd413efc Mon Sep 17 00:00:00 2001 From: lisa-wolfgang <43426138+lisa-wolfgang@users.noreply.github.com> Date: Wed, 20 Jul 2022 21:02:14 -0500 Subject: [PATCH 07/10] Move to reference category --- .../getting-started => reference}/quality-guidelines.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename content/docs/{develop/getting-started => reference}/quality-guidelines.md (97%) diff --git a/content/docs/develop/getting-started/quality-guidelines.md b/content/docs/reference/quality-guidelines.md similarity index 97% rename from content/docs/develop/getting-started/quality-guidelines.md rename to content/docs/reference/quality-guidelines.md index ac4dffa0c..9776d5010 100644 --- a/content/docs/develop/getting-started/quality-guidelines.md +++ b/content/docs/reference/quality-guidelines.md @@ -1,7 +1,7 @@ --- title: Quality Guidelines aliases: - - /docs/developing/getting-started/quality-guidelines + - /docs/developing/quality-guidelines --- Keep these guidelines in mind when creating or reviewing a pull request (PR). If you're not sure about a certain guideline when creating a PR, you can submit the PR as a draft and mention that you need help with a specific aspect of your PR. Many developers and community members are willing and able to help your PR improve! From 5dbd3b7fdcf40af962dc0739be4bf2186bc131b0 Mon Sep 17 00:00:00 2001 From: lisa-wolfgang <43426138+lisa-wolfgang@users.noreply.github.com> Date: Thu, 4 Aug 2022 17:59:59 -0500 Subject: [PATCH 08/10] Add approval checklist --- content/docs/reference/quality-guidelines.md | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/content/docs/reference/quality-guidelines.md b/content/docs/reference/quality-guidelines.md index 9776d5010..07cf7acac 100644 --- a/content/docs/reference/quality-guidelines.md +++ b/content/docs/reference/quality-guidelines.md @@ -15,4 +15,21 @@ All PRs: New addons: - must have clear, concise, and easy-to-understand titles, descriptions, info notices (if any), and setting names (if any). English is required: the base addon manifest is used as a reference for our translators. The developers and members of the community should scrutinize and help with this. An effective meter for this is to test it **blindly** -- that is, download and try out the addon before reading the PR description. - must be easy to use. The labels and design language used in any UI that the addon adds or modifies should be understandable by whoever will be using that addon. Again, testing blindly is an effective meter for this. Addons without an effective UI communicating its changes will be ranked lower than other addons in the settings page. -- must be compatible with our existing addons. This doesn't just mean avoiding bugs: addons should complement each other by supporting each other's features. For example, an addon that has a user interface should support our respective dark mode addon. \ No newline at end of file +- must be compatible with our existing addons. This doesn't just mean avoiding bugs: addons should complement each other by supporting each other's features. For example, an addon that has a user interface should support our respective dark mode addon. + +In order for an approval to be valid, it must have the following checklist accompanying it as a comment: +```markdown + +[] Valid reason to merge +[] To the best of my knowledge, supports all browsers supported by the extension +[] I personally tested all features in both a Chromium-based browser and Firefox +[] I personally reviewed all code line-by-line + +[] All titles, descriptions, info notices, and setting names provided are clear and concise +[] I personally found the addon to be easy to use during my testing +[] I personally tested with all other addons and features enabled and did not find compatibility issues +[] The addon is well-integrated with other addons + +[] I blind-tested this PR +[] I'm a specialist in the area this PR is changing +``` \ No newline at end of file From 8de282c4e0b9dbee3833ffa77d9e01aa72ffe0d5 Mon Sep 17 00:00:00 2001 From: lisa-wolfgang <43426138+lisa-wolfgang@users.noreply.github.com> Date: Sun, 7 Aug 2022 19:04:34 -0500 Subject: [PATCH 09/10] Add standalone addon requirement --- content/docs/reference/quality-guidelines.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/content/docs/reference/quality-guidelines.md b/content/docs/reference/quality-guidelines.md index 07cf7acac..b7eaf8eb5 100644 --- a/content/docs/reference/quality-guidelines.md +++ b/content/docs/reference/quality-guidelines.md @@ -16,6 +16,7 @@ New addons: - must have clear, concise, and easy-to-understand titles, descriptions, info notices (if any), and setting names (if any). English is required: the base addon manifest is used as a reference for our translators. The developers and members of the community should scrutinize and help with this. An effective meter for this is to test it **blindly** -- that is, download and try out the addon before reading the PR description. - must be easy to use. The labels and design language used in any UI that the addon adds or modifies should be understandable by whoever will be using that addon. Again, testing blindly is an effective meter for this. Addons without an effective UI communicating its changes will be ranked lower than other addons in the settings page. - must be compatible with our existing addons. This doesn't just mean avoiding bugs: addons should complement each other by supporting each other's features. For example, an addon that has a user interface should support our respective dark mode addon. +- must work without any other addons enabled. In order for an approval to be valid, it must have the following checklist accompanying it as a comment: ```markdown @@ -28,6 +29,7 @@ In order for an approval to be valid, it must have the following checklist accom [] All titles, descriptions, info notices, and setting names provided are clear and concise [] I personally found the addon to be easy to use during my testing [] I personally tested with all other addons and features enabled and did not find compatibility issues +[] I personally tested with no other addons enabled and did not find issues [] The addon is well-integrated with other addons [] I blind-tested this PR From 3b3c6693a8ddfd9a848a52836b03f0dedb0c9ccb Mon Sep 17 00:00:00 2001 From: lisa-wolfgang <43426138+lisa-wolfgang@users.noreply.github.com> Date: Sat, 15 Oct 2022 16:38:52 -0500 Subject: [PATCH 10/10] RTL --- content/docs/reference/quality-guidelines.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/content/docs/reference/quality-guidelines.md b/content/docs/reference/quality-guidelines.md index b7eaf8eb5..2ffbed147 100644 --- a/content/docs/reference/quality-guidelines.md +++ b/content/docs/reference/quality-guidelines.md @@ -8,7 +8,7 @@ Keep these guidelines in mind when creating or reviewing a pull request (PR). If All PRs: - should have sound reasoning as to why they should be merged. This should be specified under the "Reason for changes" header when writing the PR description. Even if it seems obvious, others may need the additional context. PRs with little to no reason to be merged are likely to be delayed, ignored, or even closed. - must support both the Chromium and Firefox browser engines. Avoid using new or experimental web technologies unless a fallback is implemented that preserves feature parity. View the [unsupported browser](https://scratchaddons.com/unsupported-browser/) page to determine the current minimum versions that must be supported. -- must be tested thoroughly. We have a very large userbase, so it's especially important that all possible use cases are considered. Every PR should be tested on a Chromium-based browser (Chrome, Edge, Brave, or another) **and** Firefox. Any tests done should be added to the "Tests" section of the PR description. **Org members, this is required in order to give approval.** +- must be tested thoroughly. We have a very large userbase, so it's especially important that all possible use cases are considered. Every PR should be tested on a Chromium-based browser (Chrome, Edge, Brave, or another) **and** Firefox. In addition, both LTR and RTL language modes should be supported. Any tests done should be added to the "Tests" section of the PR description. **Org members, this is required in order to give approval.** - must have **all** code reviewed line-by-line. **Org members, this is required in order to give approval.** - should be reviewed by someone familiar with the aspects that the PR is changing. This isn't a hard requirement, but a person like this will help iron out edge cases and minor issues. @@ -24,6 +24,7 @@ In order for an approval to be valid, it must have the following checklist accom [] Valid reason to merge [] To the best of my knowledge, supports all browsers supported by the extension [] I personally tested all features in both a Chromium-based browser and Firefox +[] All added UI elements support both LTR and RTL language modes [] I personally reviewed all code line-by-line [] All titles, descriptions, info notices, and setting names provided are clear and concise