Skip to content
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

Fixes #3681 Note Value Icons now display for Apple Devices #4127

Merged
merged 3 commits into from
Dec 14, 2024

Conversation

jivansh77
Copy link
Contributor

Summary

This PR introduces a noteBlockGenerator method in ProtoBlock to create blocks with custom SVG representations specifically for Apple devices.

Changes

  • Generates SVG for musical notes (whole, half, sixteenth, thirty-second, and sixty-fourth).
  • Utilizes a switch statement to select the appropriate SVG path based on the block name.
  • Modified WholeNoteBlock, HalfNoteBlock, and others to use the new generator method.
  • Ensured that only Apple devices utilize this generator.

image

@jivansh77
Copy link
Contributor Author

@walterbender @pikurasa Please review this PR.

@walterbender
Copy link
Member

I hadn't realized this was a problem on Apple devices. Is there an associated issue for this?
I wonder if we don't want to just use this (SVG) for all devices to simplify the code?

@pikurasa
Copy link
Collaborator

I hadn't realized this was a problem on Apple devices. Is there an associated issue for this? I wonder if we don't want to just use this (SVG) for all devices to simplify the code?

The associated issue is #3681

I agree that it's worth trying SVG for all devices, not just Apple.

@jivansh77
Copy link
Contributor Author

jivansh77 commented Dec 10, 2024

@pikurasa @walterbender The issue with some notes rendering was primarily observed on Apple devices due to differences in how certain Unicode characters are displayed.
While using SVGs universally could simplify the code, the current approach aims to minimize changes for other devices which support those Unicodes. However, I can remove the conditional and keep svg for all the notes, for all devices. Lemme know ur thoughts.

@walterbender walterbender merged commit 6bc1644 into sugarlabs:master Dec 14, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants