Correctly position autowrapped text lines. Fixes Khan/live-editor#630 #10
Conversation
This file contains 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.
This splits out code which renders multiple lines from
text$4(...)for reuse intext$6(...), greatly simplifying, and fixing a positioning bug in the latter (Khan/live-editor#630).Both
text$4andtext$6are still responsible for determining where lines are broken (on newlines, and automatically based on bounds, respectively).Below are before and after renderings of applying the Processing code, demonstrating the problem and solution in action.
This PR does not, however, include automated tests - in order to execute tests on this fork of Processing on Windows, one must install VisualStudio to build Spidermonkey, prep python, make, etc...), a path I started down, but soon decided against - Processing-js has already made this easy on the upstream fork.
I'm more interested in putting effort toward (Khan/live-editor#596) to update KA's fork to at least Processingjs 1.4.3, which has been migrated to nodejs for it's build (see processing-js/processing-js@67a092cc). Input from the KA team on any plans for the processingjs sync is welcome.
Chrome, Windows7 (same behavior on IE/Firefox)
Before Patch
After Patch
