Skip to content
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

Little refactorings for tutorial.js #139

Merged
merged 7 commits into from
Jul 14, 2020

Conversation

musicEnfanthen
Copy link
Member

@musicEnfanthen musicEnfanthen commented Jun 18, 2020

This PR proposes some light refactorings for tutorials.js, including

It also adds another log if a stepNumber is thrown with a value of NaN to improve debugging of #136 .

This fix increments the step number of the nextStepButton to the next step number,
so the loadTutorialStep() function just has to call this step number without incrementation.
This change moves the changeListener for the ACE editor from the handle EditorChanges() function
to the loadTutorialStep() function. Thus it avoids the necessity to stop change propagation and recursive call.
It also removes some unnecessary parameters from handleEditorChanges() as well as it simplifies the mechanism
to disallow/allow download and/or continuation with next step.
…ered

As mentioned by @KristinaRichts and @krHERO ,
the current error message when an encoding cannot be rendered in the tutorials
was misleading ("Please adjust your encoding …").
This change adds a console log for any NaN values to improve debugging of music-encoding#136.
@musicEnfanthen
Copy link
Member Author

@KristinaRichts @krHERO
Could you have a look if you are fine with the changes in 9333d9e ?

@bwbohl
Copy link
Member

bwbohl commented Jul 13, 2020

@KristinaRichts and @krHERO any chance you have a look at this?

@KristinaRichts
Copy link

I also think, that the error message you suggested is better than the previous one, but in the long run, the reactions should be so clear that the user can understand why the coding cannot be rendered.

@bwbohl
Copy link
Member

bwbohl commented Jul 14, 2020

thx @krHERO and @KristinaRichts merged for now and noted for future improvement in #150

@bwbohl bwbohl closed this Jul 14, 2020
@bwbohl bwbohl reopened this Jul 14, 2020
@bwbohl bwbohl merged commit 290186b into music-encoding:master Jul 14, 2020
@musicEnfanthen musicEnfanthen deleted the improve-tutorial-js branch September 30, 2020 08:15
bwbohl added a commit that referenced this pull request May 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error message for renderings in tutorials is misleading
4 participants