Skip to content

Update p5.sound toggle behaviour #3522

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.

Sign up for GitHub

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

Merged
raclim merged 5 commits into from
Jun 24, 2025
Merged

Update p5.sound toggle behaviour #3522

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
51642d5
Update p5.sound toggle behaviour
davepagurek Jun 10, 2025
0b019cd
Factor out helper function to get major version
davepagurek Jun 11, 2025
39a31d0
Update default p5 version, use it in p5.sound URL
davepagurek Jun 11, 2025
b9669a1
Merge branch 'develop' into p5-sound-toggle
davepagurek Jun 13, 2025
1478bc1
Add common folder for p5 versions
davepagurek Jun 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 15 additions & 22 deletions client/modules/IDE/components/Preferences/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -578,17 +578,7 @@ export default function Preferences() {
<input
type="radio"
onChange={() => {
if (versionInfo.lastP5SoundURL) {
// If the sketch previously used a nonstandard p5.sound
// URL, restore that URL
updateHTML(
versionInfo.setP5SoundURL(versionInfo.lastP5SoundURL)
);
versionInfo.setLastP5SoundURL(undefined);
} else {
// Otherwise, turn on the default p5.sound URL
updateHTML(versionInfo.setP5Sound(true));
}
updateHTML(versionInfo.setP5Sound(true));
}}
aria-label={`${t('Preferences.SoundAddon')} ${t(
'Preferences.AddonOn'
Expand All @@ -605,12 +595,6 @@ export default function Preferences() {
<input
type="radio"
onChange={() => {
// If the previous p5.sound.js script tag is not the
// default version that one will get via this toggle,
// record it so we can give the option to put it back
if (versionInfo.p5SoundURL !== p5SoundURL) {
versionInfo.setLastP5SoundURL(versionInfo.p5SoundURL);
}
updateHTML(versionInfo.setP5Sound(false));
}}
aria-label={`${t('Preferences.SoundAddon')} ${t(
Expand All @@ -628,11 +612,20 @@ export default function Preferences() {
>
{t('Preferences.Off')}
</label>
{versionInfo.lastP5SoundURL && (
<legend className="preference__warning">
{t('Preferences.UndoSoundVersion')}
</legend>
)}
<legend className="preference__warning">
<a
target="_blank"
rel="noreferrer"
href={`https://${
versionInfo.isVersion2 ? 'beta.' : ''
}p5js.org/reference/p5.sound`}
>
{t('Preferences.SoundReference').replace(
'$VERSION',
versionInfo.version
)}
</a>
</legend>
</fieldset>
</div>
<div className="preference">
Expand Down
49 changes: 35 additions & 14 deletions client/modules/IDE/hooks/useP5Version.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* eslint-disable func-names */
import React, { useContext, useMemo, useState } from 'react';

Check warning on line 2 in client/modules/IDE/hooks/useP5Version.jsx

View workflow job for this annotation

GitHub Actions / Test and lint code base 

'useState' is defined but never used
import { useSelector } from 'react-redux';
import PropTypes from 'prop-types';

Expand Down Expand Up @@ -144,7 +144,12 @@

export const currentP5Version = '1.11.5'; // Don't update to 2.x until 2026

export const p5SoundURLOld = `https://cdnjs.cloudflare.com/ajax/libs/p5.js/1.11.3/addons/p5.sound.min.js`;
export const p5SoundURLOldTemplate =
'https://cdnjs.cloudflare.com/ajax/libs/p5.js/$VERSION/addons/p5.sound.min.js';
export const p5SoundURLOld = p5SoundURLOldTemplate.replace(
'$VERSION',
'1.11.3'
Copy link
Copy Markdown
Member

@ksen0 ksen0 Jun 11, 2025

Choose a reason for hiding this comment

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

@davepagurek Would it be possible to use latest within 1.x? Since we are still bugfixing for a while.

Copy link
Copy Markdown
Contributor Author

@davepagurek davepagurek Jun 11, 2025

Choose a reason for hiding this comment

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

Updated this to use currentP5Version. I also updated the default web editor version to 1.11.8, since it was still 1.11.7 elsewhere. @raclim ideally we use the same string and splice it into createDefaultFiles.js so we have less places to update when there's a new version, as currently the two were out of date. Looks like that file is run on the server? Is there a spot for anything currently that gets imported in both client and server that I could move that constant to?

Copy link
Copy Markdown
Collaborator

@raclim raclim Jun 13, 2025

Choose a reason for hiding this comment

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

I...don't believe that it exists yet. I'm sorry I think it might have to be created!

Copy link
Copy Markdown
Contributor Author

@davepagurek davepagurek Jun 13, 2025

Choose a reason for hiding this comment

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

No problem! I'm happy to create that. Would something like a top-level common folder be ok?

Copy link
Copy Markdown
Collaborator

@raclim raclim Jun 13, 2025

Choose a reason for hiding this comment

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

Yes, that sounds good to me! Thank you so much!

Copy link
Copy Markdown
Contributor Author

@davepagurek davepagurek Jun 13, 2025

Choose a reason for hiding this comment

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

ok I think I've added that! i added a thing to the dockerfile about the common folder, which seems to work running the project via docker, but I'm less confident that that's correct or necessary, so let me know if that seems ok to you!

Copy link
Copy Markdown
Collaborator

@raclim raclim Jun 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Yes I think its correct, thanks so much for catching that I completely forgot about it! The folder setup/Dockerfile looks good to me! :)

);
export const p5SoundURL =
'https://cdn.jsdelivr.net/npm/p5.sound@0.2.0/dist/p5.sound.min.js';
export const p5PreloadAddonURL =
Expand All @@ -168,8 +173,6 @@
const indexSrc = indexFile?.content;
const indexID = indexFile?.id;

const [lastP5SoundURL, setLastP5SoundURL] = useState(undefined);

// { version: string, minified: boolean, replaceVersion: (version: string) => string } | null
const versionInfo = useMemo(() => {
if (!indexSrc) return null;
Expand Down Expand Up @@ -207,14 +210,6 @@
// We only know for certain which one we've got if
if (usedP5Versions.length === 1) {
const { version, minified, scriptNode } = usedP5Versions[0];
const replaceVersion = function (newVersion) {
const file = minified ? 'p5.min.js' : 'p5.js';
scriptNode.setAttribute(
'src',
`https://cdn.jsdelivr.net/npm/p5@${newVersion}/lib/${file}`
);
return serializeResult();
};

const p5SoundNode = [
...dom.documentElement.querySelectorAll('script')
Expand All @@ -232,7 +227,9 @@
const newNode = document.createElement('script');
newNode.setAttribute(
'src',
version.startsWith('2') ? p5SoundURL : p5SoundURLOld
version.startsWith('2')
Comment thread
davepagurek marked this conversation as resolved.
Outdated
? p5SoundURL
: p5SoundURLOldTemplate.replace('$VERSION', version)
);
scriptNode.parentNode.insertBefore(newNode, scriptNode.nextSibling);
}
Expand All @@ -250,6 +247,31 @@
return serializeResult();
};

const replaceVersion = function (newVersion) {
const file = minified ? 'p5.min.js' : 'p5.js';
scriptNode.setAttribute(
'src',
`https://cdn.jsdelivr.net/npm/p5@${newVersion}/lib/${file}`
);

if (p5SoundNode) {
if (version.startsWith('2.') !== newVersion.startsWith('2.')) {
// Turn off p5.sound if the user switched from 1.x to 2.x
setP5Sound(false);
} else {
// Replace the existing p5.sound with the one compatible with
// the new version
setP5SoundURL(
version.startsWith('2.')
? p5SoundURL
: p5SoundURLOldTemplate.replace('$VERSION', newVersion)
);
}
}

return serializeResult();
};

const p5PreloadAddonNode = [
...dom.documentElement.querySelectorAll('script')
].find((s) => s.getAttribute('src') === p5PreloadAddonURL);
Expand Down Expand Up @@ -294,14 +316,13 @@

return {
version,
isVersion2: version.startsWith('2.'),
minified,
replaceVersion,
p5Sound: !!p5SoundNode,
setP5Sound,
setP5SoundURL,
p5SoundURL: p5SoundNode?.getAttribute('src'),
lastP5SoundURL,
setLastP5SoundURL,
p5PreloadAddon: !!p5PreloadAddonNode,
setP5PreloadAddon,
p5ShapesAddon: !!p5ShapesAddonNode,
Expand Down
8 changes: 6 additions & 2 deletions client/styles/components/_preferences.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,8 +150,12 @@
.preference__warning {
@include themify() {
display: contents;
font-weight: bold;
color: getThemifyVariable("preferences-warning-color");
& a {
color: getThemifyVariable('button-background-hover-color');
}
& a:hover {
text-decoration: underline;
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion translations/locales/en-US/translations.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@
"DataAddon": "p5.js 1.x Compatibility Add-on Library — Data Structures",
"AddonOnARIA": "on",
"AddonOffARIA": "off",
"UndoSoundVersion": "Want to use p5.sound.js again? Turning it back on will restore the version you were using before.",
"SoundReference": "View the reference for p5.sound compatible with p5.js $VERSION",
"CopyToClipboardSuccess": "Copied to clipboard!",
"CopyToClipboardFailure": "We weren't able to copy the text, try selecting it and copying it manually."
},
Expand Down