Skip to content

Commit 5e0c814

Browse files
DhyaniKavyaDhyaniKavya
authored
Merge branch 'master' into fix/restore-focus-outline-accessibility
2 parents 528604c + c270201 commit 5e0c814

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

76 files changed

+13531
-2174
lines changed

‎.github/PULL_REQUEST_TEMPLATE/generic.md‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,17 @@ about: Submit changes to the project for review and inclusion
1515

1616
This PR fixes #
1717

18+
## PR Category
19+
20+
<!--- CI ENFORCED: You MUST check at least ONE category below or the CI will fail. -->
21+
<!--- Check all categories that apply to this pull request. -->
22+
23+
- [ ] Bug Fix — Fixes a bug or incorrect behavior
24+
- [ ] Feature — Adds new functionality
25+
- [ ] Performance — Improves performance (load time, memory, rendering, etc.)
26+
- [ ] Tests — Adds or updates test coverage
27+
- [ ] Documentation — Updates to docs, comments, or README
28+
1829
## Changes Made
1930

2031
<!--- Provide a summary of the changes made in this pull request. -->

‎.github/workflows/pr-category-check.yml‎

Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,169 @@
1+
name: PR Category Check
2+
3+
# Ensures every PR has at least one category checkbox checked.
4+
# Automatically applies matching GitHub labels to the PR.
5+
# Creates labels with custom colors if they don't exist yet.
6+
# Uses pull_request_target so it works for fork PRs too (runs in base repo context).
7+
# This is safe because we only read the PR body from the event payload — no fork code is checked out or executed.
8+
9+
on:
10+
pull_request_target:
11+
types: [opened, edited, synchronize, reopened]
12+
13+
permissions:
14+
pull-requests: write
15+
contents: read
16+
17+
jobs:
18+
check-pr-category:
19+
name: Validate PR Category & Auto-Label
20+
runs-on: ubuntu-latest
21+
steps:
22+
- name: Check categories and apply labels
23+
uses: actions/github-script@v7
24+
with:
25+
script: |
26+
const prBody = context.payload.pull_request.body || '';
27+
const prNumber = context.payload.pull_request.number;
28+
29+
// Category checkboxes mapped to their GitHub label names, colors, and descriptions
30+
const categories = [
31+
{
32+
label: 'bug fix',
33+
color: 'D73A4A',
34+
description: 'Fixes a bug or incorrect behavior',
35+
displayName: 'Bug Fix',
36+
pattern: /-\s*\[x\]\s*Bug Fix/i,
37+
},
38+
{
39+
label: 'feature',
40+
color: '9333EA',
41+
description: 'Adds new functionality',
42+
displayName: 'Feature',
43+
pattern: /-\s*\[x\]\s*Feature/i,
44+
},
45+
{
46+
label: 'performance',
47+
color: 'F97316',
48+
description: 'Improves performance (load time, memory, rendering)',
49+
displayName: 'Performance',
50+
pattern: /-\s*\[x\]\s*Performance/i,
51+
},
52+
{
53+
label: 'tests',
54+
color: '3B82F6',
55+
description: 'Adds or updates test coverage',
56+
displayName: 'Tests',
57+
pattern: /-\s*\[x\]\s*Tests/i,
58+
},
59+
{
60+
label: 'documentation',
61+
color: '10B981',
62+
description: 'Updates to docs, comments, or README',
63+
displayName: 'Documentation',
64+
pattern: /-\s*\[x\]\s*Documentation/i,
65+
},
66+
];
67+
68+
const checkedCategories = categories.filter(cat => cat.pattern.test(prBody));
69+
const uncheckedCategories = categories.filter(cat => !cat.pattern.test(prBody));
70+
71+
// --- Step 1: Fail CI if no category is selected ---
72+
if (checkedCategories.length === 0) {
73+
const message = [
74+
'## PR Category Required',
75+
'',
76+
'This pull request does not have any **PR Category** selected.',
77+
'Please edit your PR description and check **at least one** category checkbox:',
78+
'',
79+
'| Category | Description |',
80+
'|----------|-------------|',
81+
'| Bug Fix | Fixes a bug or incorrect behavior |',
82+
'| Feature | Adds new functionality |',
83+
'| Performance | Improves performance |',
84+
'| Tests | Adds or updates test coverage |',
85+
'| Documentation | Updates to docs, comments, or README |',
86+
'',
87+
'Example: Change `- [ ] Bug Fix` to `- [x] Bug Fix`',
88+
'',
89+
'> **Tip:** You can select multiple categories if your PR spans several areas.',
90+
].join('\n');
91+
92+
core.setFailed(message);
93+
return;
94+
}
95+
96+
// --- Step 2: Ensure labels exist with proper colors ---
97+
const { data: existingLabels } = await github.rest.issues.listLabelsForRepo({
98+
owner: context.repo.owner,
99+
repo: context.repo.repo,
100+
per_page: 100,
101+
});
102+
const existingLabelNames = existingLabels.map(l => l.name);
103+
104+
for (const cat of categories) {
105+
if (!existingLabelNames.includes(cat.label)) {
106+
try {
107+
await github.rest.issues.createLabel({
108+
owner: context.repo.owner,
109+
repo: context.repo.repo,
110+
name: cat.label,
111+
color: cat.color,
112+
description: cat.description,
113+
});
114+
core.info(`Created label: "${cat.label}" with color #${cat.color}`);
115+
} catch (error) {
116+
core.warning(`Could not create label "${cat.label}". Error: ${error.message}`);
117+
}
118+
}
119+
}
120+
121+
// --- Step 3: Auto-apply labels for checked categories ---
122+
const labelsToAdd = checkedCategories.map(cat => cat.label);
123+
const labelsToRemove = uncheckedCategories.map(cat => cat.label);
124+
125+
// Get current labels on the PR
126+
const { data: currentLabels } = await github.rest.issues.listLabelsOnIssue({
127+
owner: context.repo.owner,
128+
repo: context.repo.repo,
129+
issue_number: prNumber,
130+
});
131+
const currentLabelNames = currentLabels.map(l => l.name);
132+
133+
// Add labels that are checked but not yet on the PR
134+
for (const label of labelsToAdd) {
135+
if (!currentLabelNames.includes(label)) {
136+
try {
137+
await github.rest.issues.addLabels({
138+
owner: context.repo.owner,
139+
repo: context.repo.repo,
140+
issue_number: prNumber,
141+
labels: [label],
142+
});
143+
core.info(`Added label: "${label}"`);
144+
} catch (error) {
145+
core.warning(`Could not add label "${label}". Error: ${error.message}`);
146+
}
147+
}
148+
}
149+
150+
// Remove labels that are unchecked but still on the PR
151+
for (const label of labelsToRemove) {
152+
if (currentLabelNames.includes(label)) {
153+
try {
154+
await github.rest.issues.removeLabel({
155+
owner: context.repo.owner,
156+
repo: context.repo.repo,
157+
issue_number: prNumber,
158+
name: label,
159+
});
160+
core.info(`Removed label: "${label}"`);
161+
} catch (error) {
162+
core.warning(`Could not remove label "${label}". Error: ${error.message}`);
163+
}
164+
}
165+
}
166+
167+
const selected = checkedCategories.map(c => c.displayName).join(', ');
168+
core.info(`PR categories selected: ${selected}`);
169+
core.info(`Labels synced: ${labelsToAdd.join(', ')}`);

‎CONTRIBUTING.md‎

Lines changed: 97 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ is a community-driven project, and every meaningful contribution helps
66
improve the platform for learners and educators around the world.
77

88
If you’re new to the project, start by setting up the local
9-
development environment using the guide linked above, then explore
9+
development environment using the guide linked below, then explore
1010
open issues or discussions to find a place to contribute.
1111

1212
- [How to set up a local server](README.md#how-to-set-up-a-local-server)
@@ -49,9 +49,63 @@ following resources:
4949
- [JavaScript tutorial - w3schools.com](https://www.w3schools.com/js/default.asp)
5050
- [JavaScript reference - MDN Web Docs](https://developer.mozilla.org/en-US/docs/Web/JavaScript)
5151

52-
Programmers, please follow these general [guidelines for
52+
For code contributions, please follow these general [guidelines for
5353
contributions](https://github.com/sugarlabs/sugar-docs/blob/master/src/contributing.md).
5454

55+
### AI guidelines
56+
57+
Follow [AI guidelines for Sugar Labs](https://github.com/sugarlabs/sugar-docs/blob/master/src/contributing.md#ai-guidelines-for-sugar-labs)
58+
59+
AI-assisted development tools (such as GitHub Copilot, ChatGPT, Cursor, Claude,
60+
or similar systems) may be used to support contributions. However, contributors
61+
remain fully responsible for any code they submit.
62+
63+
When using AI tools, please follow these guidelines:
64+
65+
- **Understand the code** - Do not submit code that you do not fully understand.
66+
Contributors must be able to explain and maintain their changes.
67+
- **Review carefully** - AI-generated code can contain errors, security issues,
68+
or incorrect assumptions. Always review outputs critically.
69+
- **Follow project conventions** - Ensure that generated code matches the existing
70+
coding style, architecture, and design patterns used in the repository.
71+
- **Test thoroughly** - AI-assisted changes must pass all project checks. Run
72+
linting, formatting, and test commands before submitting.
73+
- **Avoid large blind changes** - Large-scale automated modifications should be
74+
reviewed incrementally and preferably split into smaller, focused pull requests.
75+
- **Licensing awareness** - Ensure that generated content does not introduce
76+
incompatible licensed material or copied external code without attribution.
77+
- **Architecture awareness** - Prefer small, incremental AI-assisted changes that
78+
align with existing architecture rather than large structural rewrites.
79+
80+
Mentioning AI assistance in your pull request description is optional but encouraged
81+
for transparency.
82+
83+
#### Using AI/LLM tools for code changes
84+
85+
AI tools such as ChatGPT, Copilot, or other LLMs may assist contributors
86+
in understanding the codebase or drafting code changes. However,
87+
contributors remain fully responsible for the code they submit.
88+
89+
When using AI tools:
90+
91+
- Ensure you understand the generated code before including it in a pull request.
92+
- Verify that the code follows project style and architecture.
93+
- Avoid submitting large AI-generated patches without manual review.
94+
- Run linting, formatting, and tests before submitting changes.
95+
- Ensure the generated code does not introduce licensing issues.
96+
97+
#### AI-assisted pull requests
98+
99+
If AI tools were used while preparing a pull request:
100+
101+
- Clearly review and test all generated changes.
102+
- Keep pull requests small and focused.
103+
- Avoid submitting unrelated modifications suggested by AI.
104+
- Be prepared to explain the reasoning behind the changes during review.
105+
106+
AI tools should assist development, but they should not replace
107+
understanding of the codebase.
108+
55109
### Before You Push
56110

57111
Run these commands locally before submitting a PR:
@@ -62,10 +116,50 @@ npx prettier --check . # Formatting
62116
npm test # Jest
63117
```
64118

65-
NOTE: Only run ```prettier``` on the files you have modified.
119+
NOTE: Only run `prettier` on the files you have modified.
66120

67121
If formatting fails, run `npx prettier --write .` to fix it.
68122

123+
### Creating Pull Requests
124+
125+
Follow these steps when contributing:
126+
127+
1. **Create a new branch**
128+
129+
```
130+
git checkout -b docs/issue-number-short-description
131+
```
132+
133+
2. Make your changes following project guidelines.
134+
135+
3. Run required checks before pushing:
136+
137+
```
138+
npm run lint
139+
npx prettier --check .
140+
npm test
141+
```
142+
143+
4. Commit with clear, descriptive messages:
144+
145+
```
146+
git commit -m "docs: add AI contribution guidelines (Related to #XXXX)"
147+
```
148+
149+
5. Push your branch:
150+
151+
```
152+
git push origin branch-name
153+
```
154+
155+
6. **Open a Pull Request:**
156+
- Use a clear and descriptive title.
157+
- Link the related issue using `Related to #XXXX` or `Partially addresses #XXXX`.
158+
- Explain what changed and why.
159+
- Keep pull requests focused on a single topic or feature.
160+
161+
7. Respond to review feedback and update your branch as needed.
162+
69163
### After your PR is merged
70164
71165
Please note that production deployments of Music Blocks are **manual**.

‎Docs/PRODUCTION_BUILD_STRATEGY.md‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
# Production Build Optimization Strategy: Architecture & Performance
2+
3+
## Why
4+
Music Blocks currently serves mostly unbundled, raw JavaScript and asset files.
5+
While this works in development, it introduces limitations for:
6+
- Production performance (high HTTP request count)
7+
- Predictable offline caching
8+
- Service worker reliability
9+
- Long-term maintainability alongside AMD-based loading
10+
11+
This document explores a **lightweight investigation** of production build optimizations without introducing a full migration or breaking existing architecture.
12+
13+
## Current Behavior
14+
- JavaScript and assets are largely served as individual files.
15+
- No formal production bundling or minification strategy exists.
16+
- Service worker caching must account for many independent assets (hundreds of individual JS files).
17+
- RequireJS / AMD loading is the primary module system, which constrains conventional bundling approaches that expect ES modules or CommonJS.
18+
19+
## Analysis: Current State & Offline Impact
20+
21+
### Baseline Metrics (as of Feb 2026)
22+
To provide a comparison reference for future optimizations:
23+
- **Total JavaScript Request Count:** ~248 files (209 Application, 39 Libraries).
24+
- **Total JavaScript Size (Uncompressed):** ~12.94 MB.
25+
- **Application Logic (`js/`):** 209 files, 6.42 MB.
26+
- **Libraries (`lib/`):** 39 files, 6.52 MB.
27+
- **Loading Model:** Sequential AMD loading, resulting in a deep waterfall of requests.
28+
29+
### Service Worker & Offline Caching
30+
The current `sw.js` implementation follows a **stale-while-revalidate** pattern with significant constraints for offline reliability:
31+
1. **Limited Pre-caching:** Only `index.html` is explicitly pre-cached. All other assets are cached dynamically upon first request.
32+
2. **Fragmentation:** Caching 200+ individual JS files increases the risk of partial cache states (where some files are cached and others are not), leading to runtime errors in offline mode.
33+
3. **Implicit Dependencies:** If the service worker fails to intercept a single AMD dependency (e.g., due to a network blip), the entire module fails to load.
34+
4. **Cache Invalidation:** Without content hashing, ensuring users receive the latest version of a file depends on the browser's fetch behavior and the SW's revalidation logic, which can be inconsistent.
35+
36+
### Proposed Strategy (Groundwork)
37+
This strategy focuses on low-risk, future-oriented thinking:
38+
39+
### 1. Selective Minification
40+
Before full bundling, we can investigate minifying individual files during a "build" step.
41+
- Reduces payload size without changing the loading model.
42+
- Keep source maps for easier production debugging.
43+
44+
### 2. Asset Grouping (Low-Risk Experiment)
45+
Instead of bundling everything into one file (which breaks RequireJS's dynamic loading), we can group "core" modules that are always loaded together.
46+
- Example: `js/utils/utils.js`, `js/turtles.js`, and basic library wrappers.
47+
48+
### 3. Hashing/Versioning
49+
Introduce a simple hashing mechanism for production assets to ensure service workers can reliably identify when an asset has changed.
50+
51+
### Running the Asset Analysis Script
52+
53+
From the repository root:
54+
55+
```bash
56+
node scripts/analyze-production-assets.js
57+
```
58+
59+
This script recursively scans the `js/` and `lib/` directories to report:
60+
- Total JavaScript file count
61+
- Aggregate size
62+
- Estimated minification savings
63+
64+
No build step or configuration is required.
65+
66+
## Scope & Constraints
67+
- **Documentation first:** This document serves as the primary outcome of this phase.
68+
- **No replacement of RequireJS / AMD:** The current module system is deeply integrated and stable.
69+
- **No build system overhaul:** Use existing Node.js scripts or lightweight tools if any implementation is attempted.
70+
- **No runtime behavior changes:** The priority is stability.
71+
72+
## Next Steps / Roadmap
73+
- [x] Audit total request count and asset sizes.
74+
- [ ] Implement a lightweight minification pass for `js/` files in the `dist/` task.
75+
- [ ] Research RequireJS `r.js` optimizer or modern alternatives (like Vite or esbuild) that can target AMD.
76+
- [ ] Create a manifest for the Service Worker to enable reliable pre-caching of core assets.

0 commit comments

Comments
 (0)