-
Notifications
You must be signed in to change notification settings - Fork 289
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
[UI] Switch read only editors to code mirror #4924
Conversation
Should I remove all of the code related to Ace too? |
Ow that is a tough one! One the one hand side, I would say: yes! But if it is a lot of work, maybe it is not worth it? In any case: we want to keep the abstraction so adding another editor is easier (although I hope we will never again have to do that!!) |
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.
Looks great @jpelay!
Just a small thing, a one line editor gets a scroll bar now (macOS, Chrome):
While this is not the case in the current main:
Since it is the first code block in the first level, it looks a bit sloppy if we leve it like this, so can you see if you can fix that?
And I'd also like a sanity check on the ts from @hasan-sh because I only skimmed it!
@@ -113,3 +160,16 @@ function fisherYatesShuffle<A>(xs: A[]) { | |||
xs[i] = h; | |||
} | |||
} | |||
|
|||
export function initializeParsons() { |
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.
Really nice clean up of the Parson's code!
@@ -264,73 +264,3 @@ | |||
{% else %} | |||
<div _="on load hedyApp.toggleDevelopersMode('load', true)"></div> | |||
{% endif %} | |||
<script> | |||
// FIXME: Stop using inline 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.
Haha, partly addressed with this PR? ;)
Good catch! I'll fix it! |
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.
Thank you for contributing! Your pull request is now going on the merge train (choo choo! Do not click update from main anymore, and be sure to allow changes to be pushed to your fork). |
Thank you for contributing! Your pull request is now going on the merge train (choo choo! Do not click update from main anymore, and be sure to allow changes to be pushed to your fork). |
Description
We are currently using CodeMirror in the main editor, and Ace in the read-only editors, this situation is not ideal since we need to maintain two wildly different highlighting systems, therefore this PR switches all the editors to use CodeMirror.
This is the current state of the replacement:
About Parsons
The parsons exercises where deeply coupled with Ace, and the dragging code was difficult to understand. In order to switch these editors to CodeMirror it was necessary a redo of most of the code, therefore, this PR also fixes #4442 because we are now using the Sortable library, making the dragging easier. This is the parson interface now:
Now students can simply shuffle the lines in place, instead of dragging them from one place to the other.