Skip to content

fix: replace SVG RECORDBUTTON with Material Icons ligature in toolbar#7785

Closed
ssz2605 wants to merge 2 commits into
sugarlabs:masterfrom
ssz2605:fix/record-button-material-icon
Closed

fix: replace SVG RECORDBUTTON with Material Icons ligature in toolbar#7785
ssz2605 wants to merge 2 commits into
sugarlabs:masterfrom
ssz2605:fix/record-button-material-icon

Conversation

@ssz2605

@ssz2605 ssz2605 commented Jul 11, 2026

Copy link
Copy Markdown
Collaborator

Description

PR #7757 replaced innerHTML with textContent for security, which broke
the Record button , RECORDBUTTON is an SVG string, not a Material Icons
ligature, so textContent cannot render it. PR #7782 reverted to innerHTML
as a temporary fix, but left the original problems unsolved:

This PR fixes all of them:

  • Add fiber_manual_record to #record in index.html, matching Play/Stop
  • Remove dynamic icon creation from toolbar-ui.js
  • Refactor recorder.js to use CSS class on #record instead of rec_inside
  • Add #record.recording CSS for blink animation and red color
  • Remove dead RECORDBUTTON SVG constant from artwork.js
  • Remove unused RECORDBUTTON imports from toolbar-ui.js and turtledefs.js
  • Update tests to reflect static icon and class-based recording state

PR Category

  • Bug Fix

@codecov

codecov Bot commented Jul 11, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 25.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 59.00%. Comparing base (3aa560a) to head (0c205d8).

Files with missing lines Patch % Lines
js/activity/recorder.js 25.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7785      +/-   ##
==========================================
- Coverage   59.00%   59.00%   -0.01%     
==========================================
  Files         175      175              
  Lines       57916    57911       -5     
==========================================
- Hits        34172    34169       -3     
+ Misses      23744    23742       -2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Record button used an SVG string set via innerHTML, which broke when
PR 7757 converted innerHTML to textContent for security. Instead of
reverting to innerHTML, move the icon to index.html as a static
Material Icons ligature element, matching how Play and Stop work.

- Add fiber_manual_record to #record in HTML
- Remove dynamic icon creation from toolbar-ui.js
- Refactor recorder.js to use CSS class on #record instead of
  targeting the now-removed rec_inside SVG element
- Add #record.recording CSS for blink animation and red color
- Remove dead RECORDBUTTON SVG constant from artwork.js
- Remove unused RECORDBUTTON imports from toolbar-ui.js and turtledefs.js
- Update tests to reflect static icon and class-based recording state
@github-actions github-actions Bot added bug fix Fixes a bug or incorrect behavior size/S Small: 10-49 lines changed area/javascript Changes to JS source files area/css Changes to CSS/SASS style files area/tests Changes to test files area/core Changes to core app entry files labels Jul 11, 2026
@ssz2605 ssz2605 force-pushed the fix/record-button-material-icon branch from 8993152 to c393edb Compare July 11, 2026 05:51
@ssz2605 ssz2605 force-pushed the fix/record-button-material-icon branch from c393edb to 0c205d8 Compare July 11, 2026 06:20
@github-actions github-actions Bot added size/M Medium: 50-249 lines changed and removed size/S Small: 10-49 lines changed labels Jul 11, 2026
@ssz2605 ssz2605 closed this Jul 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/core Changes to core app entry files area/css Changes to CSS/SASS style files area/javascript Changes to JS source files area/tests Changes to test files bug fix Fixes a bug or incorrect behavior size/M Medium: 50-249 lines changed

Projects

Development

Successfully merging this pull request may close these issues.

1 participant