From 90abb6b2ef3f64389b97960ee2ec395c1c432843 Mon Sep 17 00:00:00 2001 From: skyash-dev Date: Fri, 1 May 2026 19:39:02 +0530 Subject: [PATCH 1/3] add UI toggle for loop protection and tests --- .../Preferences/Preferences.unit.test.jsx | 123 +++++++++++++++++- .../IDE/components/Preferences/index.jsx | 57 ++++++++ client/modules/IDE/utils/loopProtection.js | 18 +++ translations/locales/en-US/translations.json | 3 + 4 files changed, 199 insertions(+), 2 deletions(-) create mode 100644 client/modules/IDE/utils/loopProtection.js diff --git a/client/modules/IDE/components/Preferences/Preferences.unit.test.jsx b/client/modules/IDE/components/Preferences/Preferences.unit.test.jsx index 13e4c88881..de5ea083a3 100644 --- a/client/modules/IDE/components/Preferences/Preferences.unit.test.jsx +++ b/client/modules/IDE/components/Preferences/Preferences.unit.test.jsx @@ -3,6 +3,8 @@ import { act, fireEvent, reduxRender, screen } from '../../../../test-utils'; import { initialState } from '../../reducers/preferences'; import Preferences from './index'; import * as PreferencesActions from '../../actions/preferences'; +import * as FileActions from '../../actions/files'; +import { initialState as filesInitialState } from '../../reducers/files'; describe('', () => { // For backwards compatibility, spy on each action creator to see when it was dispatched. @@ -13,13 +15,32 @@ describe('', () => { }) ); - const subject = (initialPreferences = {}) => + const updateFileContentSpy = jest.spyOn(FileActions, 'updateFileContent'); + + const subject = ( + initialPreferences = {}, + sketchContent = ` + function setup() { + createCanvas(400, 400); + } + + function draw() { + background(220); + } + ` + ) => reduxRender(, { initialState: { preferences: { ...initialState, ...initialPreferences - } + }, + files: filesInitialState().map((file) => ({ + ...file, + ...(file.fileType === 'file' && + file.name === 'sketch.js' && + file.filePath === '' && { content: sketchContent }) + })) } }); @@ -485,6 +506,104 @@ describe('', () => { ); }); }); + describe('loop protection toggle', () => { + it('is ON by default when no noprotect comment exists', () => { + subject({}, 'function setup() {}'); + + const onRadio = screen.getByRole('radio', { + name: /loop protection on/i + }); + + expect(onRadio.checked).toBe(true); + }); + + it('is OFF when noprotect comment exists', () => { + subject({}, '// noprotect\nfunction setup() {}'); + + const offRadio = screen.getByRole('radio', { + name: /loop protection off/i + }); + + expect(offRadio.checked).toBe(true); + }); + + it('adds noprotect comment when turning OFF', () => { + subject({}, 'function setup() {}'); + + const offRadio = screen.getByRole('radio', { + name: /loop protection off/i + }); + + act(() => { + fireEvent.click(offRadio); + }); + + expect(updateFileContentSpy).toHaveBeenCalledTimes(1); + + const updatedSrc = updateFileContentSpy.mock.calls[0][1]; + expect(updatedSrc).toMatch(/^\/\/ noprotect\b/); + expect(updatedSrc.match(/\/\/ noprotect/g)?.length).toBe(1); + }); + + it('removes noprotect comment when turning ON', () => { + subject({}, '// noprotect\nfunction setup() {}'); + + const onRadio = screen.getByRole('radio', { + name: /loop protection on/i + }); + + act(() => { + fireEvent.click(onRadio); + }); + + expect(updateFileContentSpy).toHaveBeenCalledTimes(1); + + const updatedSrc = updateFileContentSpy.mock.calls[0][1]; + expect(updatedSrc).not.toMatch(/\/\/\s*noprotect/); + }); + + it('does not dispatch when clicking already selected state (ON)', () => { + subject({}, 'function setup() {}'); + + const onRadio = screen.getByRole('radio', { + name: /loop protection on/i + }); + + act(() => { + fireEvent.click(onRadio); + }); + + expect(updateFileContentSpy).toHaveBeenCalledTimes(0); + }); + + it('does not duplicate noprotect comment when already OFF', () => { + subject({}, '// noprotect\nfunction setup() {}'); + + const offRadio = screen.getByRole('radio', { + name: /loop protection off/i + }); + + act(() => { + fireEvent.click(offRadio); + }); + + expect(updateFileContentSpy).toHaveBeenCalledTimes(0); + }); + it('removes noprotect even if not at top', () => { + subject({}, 'function setup() {}\n// noprotect'); + + const onRadio = screen.getByRole('radio', { + name: /loop protection on/i + }); + + act(() => { + fireEvent.click(onRadio); + }); + + const updatedSrc = updateFileContentSpy.mock.calls[0][1]; + expect(updatedSrc).not.toMatch(/noprotect/); + }); + }); }); describe('can toggle between general settings and accessibility tabs successfully', () => { diff --git a/client/modules/IDE/components/Preferences/index.jsx b/client/modules/IDE/components/Preferences/index.jsx index a2fa59496d..cdcc948e0e 100644 --- a/client/modules/IDE/components/Preferences/index.jsx +++ b/client/modules/IDE/components/Preferences/index.jsx @@ -29,6 +29,7 @@ import { CmControllerContext } from '../../pages/IDEView'; import Stars from '../Stars'; import Admonition from '../Admonition'; import TextArea from '../TextArea'; +import { hasNoProtect, toggleLoopProtection } from '../../utils/loopProtection'; export default function Preferences() { const { t } = useTranslation(); @@ -53,6 +54,16 @@ export default function Preferences() { const { versionInfo, indexID } = useP5Version(); const cmRef = useContext(CmControllerContext); const [showStars, setShowStars] = useState(null); + const files = useSelector((s) => s.files); + const sketchFile = files.find( + (file) => + file.fileType === 'file' && + file.name === 'sketch.js' && + file.filePath === '' + ); + const sketchSrc = sketchFile?.content; + const sketchID = sketchFile?.id; + const loopProtection = useMemo(() => !hasNoProtect(sketchSrc), [sketchSrc]); const timerRef = useRef(null); const pickerRef = useRef(null); const onChangeVersion = (version) => { @@ -65,6 +76,16 @@ export default function Preferences() { } }; + function handleLoopProtection(enabled) { + if (!sketchID || !sketchSrc) return; + + const next = toggleLoopProtection(sketchSrc, enabled); + if (next === sketchSrc) return; + + dispatch(updateFileContent(sketchID, next)); + cmRef.current?.updateFileContent(sketchID, next); + } + function onFontInputChange(event) { const INTEGER_REGEX = /^[0-9\b]+$/; if (event.target.value === '' || INTEGER_REGEX.test(event.target.value)) { @@ -414,6 +435,42 @@ export default function Preferences() { +
+

+ {t('Preferences.LoopProtection')} +

+
+ handleLoopProtection(true)} + aria-label={t('Preferences.LoopProtectionOnARIA')} + name="loopprotection" + id="loopprotection-on" + className="preference__radio-button" + value="On" + checked={loopProtection} + /> + + handleLoopProtection(false)} + aria-label={t('Preferences.LoopProtectionOffARIA')} + name="loopprotection" + id="loopprotection-off" + className="preference__radio-button" + value="Off" + checked={!loopProtection} + /> + +
+
diff --git a/client/modules/IDE/utils/loopProtection.js b/client/modules/IDE/utils/loopProtection.js new file mode 100644 index 0000000000..a6f4a6fa02 --- /dev/null +++ b/client/modules/IDE/utils/loopProtection.js @@ -0,0 +1,18 @@ +const NO_PROTECT_REGEX = /^\s*\/\/\s*noprotect\b.*\n?/m; + +export function hasNoProtect(src = '') { + return NO_PROTECT_REGEX.test(src); +} + +export function addNoProtect(src = '') { + if (hasNoProtect(src)) return src; + return `// noprotect\n${src}`; +} + +export function removeNoProtect(src = '') { + return src.replace(NO_PROTECT_REGEX, ''); +} + +export function toggleLoopProtection(src = '', enabled) { + return enabled ? removeNoProtect(src) : addNoProtect(src); +} diff --git a/translations/locales/en-US/translations.json b/translations/locales/en-US/translations.json index a1c17007c8..1144c18671 100644 --- a/translations/locales/en-US/translations.json +++ b/translations/locales/en-US/translations.json @@ -219,6 +219,9 @@ "WordWrap": "Word Wrap", "WordWrapOnARIA": "wordwrap on", "WordWrapOffARIA": "wordwrap off", + "LoopProtection": "Loop Protection", + "LoopProtectionOnARIA": "loop protection on", + "LoopProtectionOffARIA": "loop protection off", "LineNumbers": "Line numbers", "LineNumbersOnARIA": "line numbers on", "LineNumbersOffARIA": "line numbers off", From 32acd1e455909e11c5061791d45fb66bf704c27f Mon Sep 17 00:00:00 2001 From: skyash-dev Date: Wed, 6 May 2026 20:32:58 +0530 Subject: [PATCH 2/3] fix css bug causing overlay move upward --- client/styles/components/_overlay.scss | 1 - 1 file changed, 1 deletion(-) diff --git a/client/styles/components/_overlay.scss b/client/styles/components/_overlay.scss index 61828b68dd..ccedadc914 100644 --- a/client/styles/components/_overlay.scss +++ b/client/styles/components/_overlay.scss @@ -8,7 +8,6 @@ bottom: 0; z-index: 9999; background-color: rgba(0, 0, 0, 0.5); - overflow-y: hidden; } .overlay__content { From 1c4356d0c7471b964c37d5a53a1f7e6efcc6a5b3 Mon Sep 17 00:00:00 2001 From: skyash-dev Date: Mon, 25 May 2026 13:31:15 +0530 Subject: [PATCH 3/3] move loop protection directive to index.html --- .../Preferences/Preferences.unit.test.jsx | 183 +++++++++--------- .../IDE/components/Preferences/index.jsx | 28 ++- client/modules/IDE/utils/loopProtection.js | 4 +- client/modules/Preview/EmbedFrame.jsx | 20 +- client/modules/Preview/jsPreprocess.js | 5 +- 5 files changed, 122 insertions(+), 118 deletions(-) diff --git a/client/modules/IDE/components/Preferences/Preferences.unit.test.jsx b/client/modules/IDE/components/Preferences/Preferences.unit.test.jsx index de5ea083a3..d9b03cf4a1 100644 --- a/client/modules/IDE/components/Preferences/Preferences.unit.test.jsx +++ b/client/modules/IDE/components/Preferences/Preferences.unit.test.jsx @@ -3,8 +3,9 @@ import { act, fireEvent, reduxRender, screen } from '../../../../test-utils'; import { initialState } from '../../reducers/preferences'; import Preferences from './index'; import * as PreferencesActions from '../../actions/preferences'; -import * as FileActions from '../../actions/files'; import { initialState as filesInitialState } from '../../reducers/files'; +import { P5VersionProvider } from '../../hooks/useP5Version'; +import { NO_PROTECT_REGEX } from '../../utils/loopProtection'; describe('', () => { // For backwards compatibility, spy on each action creator to see when it was dispatched. @@ -15,34 +16,27 @@ describe('', () => { }) ); - const updateFileContentSpy = jest.spyOn(FileActions, 'updateFileContent'); - - const subject = ( - initialPreferences = {}, - sketchContent = ` - function setup() { - createCanvas(400, 400); - } - - function draw() { - background(220); - } - ` - ) => - reduxRender(, { - initialState: { - preferences: { - ...initialState, - ...initialPreferences - }, - files: filesInitialState().map((file) => ({ - ...file, - ...(file.fileType === 'file' && - file.name === 'sketch.js' && - file.filePath === '' && { content: sketchContent }) - })) + const subject = (initialPreferences = {}, indexContent) => + reduxRender( + + + , + { + initialState: { + preferences: { + ...initialState, + ...initialPreferences + }, + files: filesInitialState().map((file) => ({ + ...file, + ...(indexContent && + file.fileType === 'file' && + file.name === 'index.html' && + file.filePath === '' && { content: indexContent }) + })) + } } - }); + ); afterEach(() => { jest.clearAllMocks(); @@ -507,101 +501,108 @@ describe('', () => { }); }); describe('loop protection toggle', () => { - it('is ON by default when no noprotect comment exists', () => { - subject({}, 'function setup() {}'); + const getIndexFile = (store) => + store + .getState() + .files.find((f) => f.fileType === 'file' && f.name === 'index.html'); - const onRadio = screen.getByRole('radio', { - name: /loop protection on/i - }); + const getOnRadio = () => + screen.getByRole('radio', { name: /loop protection on/i }); + + const getOffRadio = () => + screen.getByRole('radio', { name: /loop protection off/i }); - expect(onRadio.checked).toBe(true); + it('is ON by default when no noprotect comment exists', () => { + subject(); + + expect(getOnRadio().checked).toBe(true); + expect(getOffRadio().checked).toBe(false); }); it('is OFF when noprotect comment exists', () => { - subject({}, '// noprotect\nfunction setup() {}'); - - const offRadio = screen.getByRole('radio', { - name: /loop protection off/i - }); + subject( + {}, + ` + + + + +` + ); - expect(offRadio.checked).toBe(true); + expect(getOnRadio().checked).toBe(false); + expect(getOffRadio().checked).toBe(true); }); it('adds noprotect comment when turning OFF', () => { - subject({}, 'function setup() {}'); - - const offRadio = screen.getByRole('radio', { - name: /loop protection off/i - }); + const { store } = subject(); + const initialIndexFile = getIndexFile(store); + expect(initialIndexFile.content).not.toMatch(NO_PROTECT_REGEX); act(() => { - fireEvent.click(offRadio); + fireEvent.click(getOffRadio()); }); - expect(updateFileContentSpy).toHaveBeenCalledTimes(1); - - const updatedSrc = updateFileContentSpy.mock.calls[0][1]; - expect(updatedSrc).toMatch(/^\/\/ noprotect\b/); - expect(updatedSrc.match(/\/\/ noprotect/g)?.length).toBe(1); + const updatedIndexFile = getIndexFile(store); + expect(updatedIndexFile.content).toMatch(NO_PROTECT_REGEX); + expect( + updatedIndexFile.content.match(//g)?.length + ).toBe(1); }); it('removes noprotect comment when turning ON', () => { - subject({}, '// noprotect\nfunction setup() {}'); - - const onRadio = screen.getByRole('radio', { - name: /loop protection on/i - }); - - act(() => { - fireEvent.click(onRadio); - }); - - expect(updateFileContentSpy).toHaveBeenCalledTimes(1); - - const updatedSrc = updateFileContentSpy.mock.calls[0][1]; - expect(updatedSrc).not.toMatch(/\/\/\s*noprotect/); - }); - - it('does not dispatch when clicking already selected state (ON)', () => { - subject({}, 'function setup() {}'); - - const onRadio = screen.getByRole('radio', { - name: /loop protection on/i - }); + const { store } = subject( + {}, + ` + + + + +` + ); + const initialIndexFile = getIndexFile(store); + expect(initialIndexFile.content).toMatch(NO_PROTECT_REGEX); act(() => { - fireEvent.click(onRadio); + fireEvent.click(getOnRadio()); }); - expect(updateFileContentSpy).toHaveBeenCalledTimes(0); + const updatedIndexFile = getIndexFile(store); + expect(updatedIndexFile.content).not.toMatch(NO_PROTECT_REGEX); }); - it('does not duplicate noprotect comment when already OFF', () => { - subject({}, '// noprotect\nfunction setup() {}'); - - const offRadio = screen.getByRole('radio', { - name: /loop protection off/i - }); + it('does not change index.html when clicking already selected ON', () => { + const { store } = subject(); + const initialIndexFile = getIndexFile(store); + const initialContent = initialIndexFile.content; act(() => { - fireEvent.click(offRadio); + fireEvent.click(getOnRadio()); }); - expect(updateFileContentSpy).toHaveBeenCalledTimes(0); + const updatedIndexFile = getIndexFile(store); + expect(updatedIndexFile.content).toBe(initialContent); }); - it('removes noprotect even if not at top', () => { - subject({}, 'function setup() {}\n// noprotect'); - const onRadio = screen.getByRole('radio', { - name: /loop protection on/i - }); + it('does not change index.html when clicking already selected OFF', () => { + const { store } = subject( + {}, + ` + + + + +` + ); + const initialIndexFile = getIndexFile(store); + const initialContent = initialIndexFile.content; act(() => { - fireEvent.click(onRadio); + fireEvent.click(getOffRadio()); }); - const updatedSrc = updateFileContentSpy.mock.calls[0][1]; - expect(updatedSrc).not.toMatch(/noprotect/); + const updatedIndexFile = getIndexFile(store); + expect(updatedIndexFile.content).toBe(initialContent); }); }); }); diff --git a/client/modules/IDE/components/Preferences/index.jsx b/client/modules/IDE/components/Preferences/index.jsx index cdcc948e0e..5eb55cbbc6 100644 --- a/client/modules/IDE/components/Preferences/index.jsx +++ b/client/modules/IDE/components/Preferences/index.jsx @@ -55,15 +55,14 @@ export default function Preferences() { const cmRef = useContext(CmControllerContext); const [showStars, setShowStars] = useState(null); const files = useSelector((s) => s.files); - const sketchFile = files.find( + const indexFile = files.find( (file) => file.fileType === 'file' && - file.name === 'sketch.js' && + file.name === 'index.html' && file.filePath === '' ); - const sketchSrc = sketchFile?.content; - const sketchID = sketchFile?.id; - const loopProtection = useMemo(() => !hasNoProtect(sketchSrc), [sketchSrc]); + const indexSrc = indexFile?.content; + const loopProtection = useMemo(() => !hasNoProtect(indexSrc), [indexSrc]); const timerRef = useRef(null); const pickerRef = useRef(null); const onChangeVersion = (version) => { @@ -76,16 +75,6 @@ export default function Preferences() { } }; - function handleLoopProtection(enabled) { - if (!sketchID || !sketchSrc) return; - - const next = toggleLoopProtection(sketchSrc, enabled); - if (next === sketchSrc) return; - - dispatch(updateFileContent(sketchID, next)); - cmRef.current?.updateFileContent(sketchID, next); - } - function onFontInputChange(event) { const INTEGER_REGEX = /^[0-9\b]+$/; if (event.target.value === '' || INTEGER_REGEX.test(event.target.value)) { @@ -136,6 +125,15 @@ export default function Preferences() { cmRef.current?.updateFileContent(indexID, src); }; + function handleLoopProtection(enabled) { + if (!indexID || !indexSrc) return; + + const next = toggleLoopProtection(indexSrc, enabled); + if (next === indexSrc) return; + + updateHTML(next); + } + const markdownComponents = useMemo(() => { // eslint-disable-next-line react/no-unstable-nested-components const ExternalLink = ({ children, ...props }) => ( diff --git a/client/modules/IDE/utils/loopProtection.js b/client/modules/IDE/utils/loopProtection.js index a6f4a6fa02..49e591496a 100644 --- a/client/modules/IDE/utils/loopProtection.js +++ b/client/modules/IDE/utils/loopProtection.js @@ -1,4 +1,4 @@ -const NO_PROTECT_REGEX = /^\s*\/\/\s*noprotect\b.*\n?/m; +export const NO_PROTECT_REGEX = /^\s*\s*\n?/m; export function hasNoProtect(src = '') { return NO_PROTECT_REGEX.test(src); @@ -6,7 +6,7 @@ export function hasNoProtect(src = '') { export function addNoProtect(src = '') { if (hasNoProtect(src)) return src; - return `// noprotect\n${src}`; + return `\n${src}`; } export function removeNoProtect(src = '') { diff --git a/client/modules/Preview/EmbedFrame.jsx b/client/modules/Preview/EmbedFrame.jsx index 02c766bd47..2b1a00ca1b 100644 --- a/client/modules/Preview/EmbedFrame.jsx +++ b/client/modules/Preview/EmbedFrame.jsx @@ -32,6 +32,10 @@ const Frame = styled.iframe` `} `; +function getHtmlFile(files) { + return files.filter((file) => file.name.match(/.*\.html$/i))[0]; +} + function resolveCSSLinksInString(content, files) { let newContent = content; let cssFileStrings = content.match(STRING_REGEX); @@ -55,6 +59,8 @@ function resolveCSSLinksInString(content, files) { } function resolveJSLinksInString(content, files) { + const indexFile = getHtmlFile(files); + const indexSrc = indexFile?.content; let newContent = content; let jsFileStrings = content.match(STRING_REGEX); jsFileStrings = jsFileStrings || []; @@ -80,7 +86,7 @@ function resolveJSLinksInString(content, files) { } }); - return jsPreprocess(newContent); + return jsPreprocess(newContent, indexSrc); } function resolveScripts(sketchDoc, files) { @@ -166,11 +172,11 @@ function resolveJSAndCSSLinks(files) { return newFiles; } -function addLoopProtect(sketchDoc) { +function addLoopProtect(sketchDoc, indexSrc) { const scriptsInHTML = sketchDoc.getElementsByTagName('script'); const scriptsInHTMLArray = Array.prototype.slice.call(scriptsInHTML); scriptsInHTMLArray.forEach((script) => { - script.innerHTML = jsPreprocess(script.innerHTML); // eslint-disable-line + script.innerHTML = jsPreprocess(script.innerHTML, indexSrc); // eslint-disable-line }); } @@ -182,6 +188,8 @@ function injectLocalFiles(files, htmlFile, options) { const resolvedFiles = resolveJSAndCSSLinks(files); const parser = new DOMParser(); const sketchDoc = parser.parseFromString(htmlFile.content, 'text/html'); + const indexFile = getHtmlFile(files); + const indexSrc = indexFile?.content; const base = sketchDoc.createElement('base'); base.href = `${window.origin}${basePath}${basePath.length > 1 && '/'}`; @@ -229,16 +237,12 @@ p5.prototype.registerMethod('afterSetup', p5.prototype.ensureAccessibleCanvas);` window.objectPaths = ${JSON.stringify(objectPaths)}; window.editorOrigin = '${getConfig('EDITOR_URL')}'; `; - addLoopProtect(sketchDoc); + addLoopProtect(sketchDoc, indexSrc); sketchDoc.head.prepend(consoleErrorsScript); return `\n${sketchDoc.documentElement.outerHTML}`; } -function getHtmlFile(files) { - return files.filter((file) => file.name.match(/.*\.html$/i))[0]; -} - function EmbedFrame({ files, isPlaying, basePath, gridOutput, textOutput }) { const iframe = useRef(); const htmlFile = useMemo(() => getHtmlFile(files), [files]); diff --git a/client/modules/Preview/jsPreprocess.js b/client/modules/Preview/jsPreprocess.js index 6acd46e6e8..e417285655 100644 --- a/client/modules/Preview/jsPreprocess.js +++ b/client/modules/Preview/jsPreprocess.js @@ -1,6 +1,7 @@ import * as acorn from 'acorn'; import * as walk from 'acorn-walk'; import escodegen from 'escodegen'; +import { hasNoProtect } from '../IDE/utils/loopProtection'; const LOOP_TIMEOUT_MS = 100; @@ -201,8 +202,8 @@ function parseJs(jsText) { } } -export function jsPreprocess(jsText) { - if (/\/\/\s*noprotect/.test(jsText)) { +export function jsPreprocess(jsText, indexSrc) { + if (hasNoProtect(indexSrc)) { return jsText; }