-
-
Notifications
You must be signed in to change notification settings - Fork 89
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
Fix no previewer for no-code demos #820
Conversation
src/window.js
Outdated
// unless the language is JavaScript, because in this case | ||
// this is probably a demo without code. | ||
const text = lang.document.code_view.buffer.text.trim(); | ||
if (text === "") { | ||
if (text === "" && language !== "JavaScript") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like "probably"
If I understand correctly the fix should rather be somewhere in await panel_ui.update();
above ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From what I can tell, no. Update is correctly running through and starting the previewer from what I can tell, but there seems to be some code path that, when a new demo opens with JS, stops the previewer again afterwards and expects the compile method to launch it again.
I will trace this further and see why the previewer is stopped again and see if I can prevent that instead.
3b2094b
to
d1c89e0
Compare
@sonnyp I think I found a solution now that is cleaner. The code is now pretty much doing the same in "runCode" as in "load". |
When these changes are applied to current HEAD 20e525f the code demos don't run - for example the button is not added to the "Welcome" demo even in python. |
I have a WIP to rework preview/run I will try to fix this issue at the same time but if not I suggest we wait until after it lands as it moves around a few bits. |
Fixes #812 Closes #820 This is the culprit https://github.com/workbenchdev/Workbench/blob/58b40d9802d14cbcc613a4f25489858783b808b2/src/window.js#L268-L272C6 For that particular code path (javascript selected, demo without js) it wouldn't update the preview One of the issue is that demos without code shouldn't have `autorun: true`. I will fix that. But in any case - I don't want Workbench to misbehave even if the demo json is incorrect.
If there is no code but the language is JavaScript, the "compile" logic should still run, since this updates the previewer when a demo window is first opened.
Curiously when switching languages (or not starting with JavaScript when the window was opened) this bug fixed itself, even though compile surely still returned early, so some other part of the code must make the previewer work in that case. Maybe this logic could use some tracing / refactoring to make sure the code takes the same paths when loading a demo as when switching languages. I'd say this fix is still fine for now, since the check here is only really relevant for compiled languages anyay.
Fixes #812