Skip to content

Conversation

@sa-fw-an
Copy link
Member

Description

  • Updated the Instruments list by adding the Harmonium. Users can now enjoy this new functionality :)

Users can access the instrument as shown in the video below:


Screen.Recording.2025-02-06.at.3.11.54.PM.mov

@sa-fw-an
Copy link
Member Author

@pikurasa Could u please review the updated PR for adding harmonium as an instrument with fixed sample

*/
const stringInstruments = ["piano","sitar", "guitar", "acoustic guitar", "electric guitar"];

const stringInstruments = ["piano","harmonium","sitar", "guitar", "acoustic guitar", "electric guitar"];
Copy link
Collaborator

Choose a reason for hiding this comment

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

perhaps we add spaces after all commas for consistency?

Copy link
Member Author

@sa-fw-an sa-fw-an Feb 21, 2025

Choose a reason for hiding this comment

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

Sure, Will edit and raise a PR.

@pikurasa
Copy link
Collaborator

I tested it. It sounds pretty good.

Its pitch center may be off a few cents, but it's an acceptable amount. That said, I don't want that to block it being merged since it's very, very close. We should definitely continue exploring debugging options -- perhaps even creating some more tools available to the user from within the Sampler widget. (pinging @haroon10725 on this comment.)

@walterbender I think we may merge it.

@github-actions
Copy link
Contributor

❌ Some Jest tests failed. Please check the logs and fix the issues before merging.

Failed Tests:

synthutils.test.js
turtle-singer.test.js

@walterbender walterbender merged commit e21cab0 into sugarlabs:master Feb 21, 2025
5 checks passed
@sa-fw-an sa-fw-an deleted the harmonium branch March 5, 2025 13:52
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