Conversation
theCapypara
approved these changes
Jan 14, 2024
Contributor
theCapypara
left a comment
There was a problem hiding this comment.
Oh this is much better.
I was missing the insight into the inner workings of how it all works to properly do that, but I think separating them is exactly the way to go.
You may want to check if that cleanup I did for the console clearing is still necessary with this. When I did the original PR I noticed that compiler errors would immediately get cleared after refactoring, I'm not 100% sure this couldn't happen now as well, but I think it might.
Contributor
|
also the CI failure is strange 🤔 |
Contributor
Author
I'm not following - but we can chat about it for clarification. I'll merge and we can address this separately if needed. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #812
Closes #820
@theCapypara thanks a lot for the bug report and PR
Took me a while to wrap my head around what was going on. 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.
This is a slightly different approach to what you suggested that instead separates
loadandrunin a way thatloadis always called beforerun.