Skip to content

fix: toggle play button icon during playback in LEGO Bricks#7456

Open
santhosh-7777 wants to merge 2 commits into
sugarlabs:masterfrom
santhosh-7777:fix-legobricks-play-icon
Open

fix: toggle play button icon during playback in LEGO Bricks#7456
santhosh-7777 wants to merge 2 commits into
sugarlabs:masterfrom
santhosh-7777:fix-legobricks-play-icon

Conversation

@santhosh-7777

Copy link
Copy Markdown
Contributor

PR Category

  • Bug Fix - Fixes a bug or incorrect behavior

Summary

Resolves #7455 — the play button icon in the LEGO Bricks
widget did not toggle during playback.

Changes

js/widgets/legobricks.js

  • Added icon toggle to stop-button.svg in playButton.onclick
    handler when playback starts
  • Added icon revert to play-button.svg at the start of
    _stopPlayback() when playback ends

Test Results

Before: Play button icon remains ▶️ throughout playback
After: Icon toggles to ⏹️ during playback and reverts to ▶️ when done

Checklist

  • All tests pass locally (npm test)
  • Lint clean (npm run lint)
  • Prettier clean (npx prettier --check .)
  • No new dependencies added

@github-actions github-actions Bot added bug fix Fixes a bug or incorrect behavior size/XS Extra small: < 10 lines changed area/javascript Changes to JS source files labels Jun 1, 2026
@github-actions

github-actions Bot commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

🧪 Jest Test Results

✅ All Jest tests passed! This PR is ready to merge.

Coverage: Statements: 48.09% | Branches: 39.61% | Functions: 52.88% | Lines: 48.49%
Master Coverage: Statements: 48.23% | Branches: 39.77% | Functions: 52.95% | Lines: 48.62%

@kartikktripathi kartikktripathi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a good change, I would only suggest you to handle the operation of stopping the flow when we click on the pause button once it has started.

@kartikktripathi

Copy link
Copy Markdown
Contributor

Also, I just checked, this branch needs to be rebased as three test suites are missing. (162 of 165 present, 3 test suites from the master branch are not visible)

@github-actions

Copy link
Copy Markdown
Contributor

🧪 Jest Test Results

✅ All Jest tests passed! This PR is ready to merge.

Coverage: Statements: 48.29% | Branches: 39.9% | Functions: 52.99% | Lines: 48.69%
Master Coverage: Statements: 48.29% | Branches: 39.91% | Functions: 52.99% | Lines: 48.69%

@github-actions github-actions Bot added size/S Small: 10-49 lines changed and removed size/XS Extra small: < 10 lines changed labels Jun 11, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🧪 Jest Test Results

✅ All Jest tests passed! This PR is ready to merge.

Coverage: Statements: 48.29% | Branches: 39.9% | Functions: 52.99% | Lines: 48.68%
Master Coverage: Statements: 48.29% | Branches: 39.91% | Functions: 52.99% | Lines: 48.69%

@github-actions

Copy link
Copy Markdown
Contributor

This PR has merge conflicts with master.

Please rebase your branch:

# Add upstream remote (one-time setup)
git remote add upstream https://github.com/sugarlabs/musicblocks.git

# Fetch latest master and rebase
git fetch upstream
git rebase upstream/master

# Resolve any conflicts, then:
git push --force-with-lease origin YOUR_BRANCH

Tip: Enable "Allow edits from maintainers" on this PR so we can auto-rebase for you next time. This only grants access to your PR branch. Your fork's other branches are not affected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/javascript Changes to JS source files bug fix Fixes a bug or incorrect behavior needs-rebase size/S Small: 10-49 lines changed

Projects

Development

Successfully merging this pull request may close these issues.

[Bug] LEGO Bricks play button icon does not toggle during playback

2 participants