Repository: chanind/hanzi-writer (ts)

Issue:
setCharacter adds a character instead of replacing it
Below is a post by Mikurox from issue 55. I have the same bug and I'm wondering if I'm misunderstanding how setCharacter should work. Documentation say 'Replace the currently drawn character with a new one', but instead it adds a new and disfigured character like his picture shows. 

Hi guys, I totally agree with you on the above, I was going to raise the following bug when I saw this one

- create a new HanziWriter() object to initialise it (it's still hidden so not used yet)
`this.writer = new HanziWriter(characterDiv, character, {
			width: imgWidth,
			height: imgHeight,
			padding: 5,
			showOutline: true,
			drawingWidth: 22, // thickness of the drawing
			delayBetweenStrokes: 400, // milliseconds
			showCharacter: false,
			highlightOnComplete: false
		});`
- then later call the setCharacter method in order to really set the character we want you want
`this.writer.setCharacter(character);`

It creates 2 'g' elements in the 'svg' one, with a bad overriding etc. as a consequence.

![screen shot 2018-09-18 at 09 57 23](https://user-images.githubusercontent.com/26358541/45673277-8c336880-bb2a-11e8-8bde-90689de8ef80.png)

It may not have been the "way to use it" but in my case I would like to initialise everything before hand before even knowing which character I want to use.

So I'm looking forward for a fix as described above, or at least a fix on the new + set -> error

_Originally posted by @Mikurox in https://github.com/chanind/hanzi-writer/issues/55#issuecomment-422296468_

Pull request description:
fix: calling setCharacter multiple times quickly breaks rendering
This PR fixes a bug where calling `setCharacter()` multiple times while a character is loading, then calling `setCharacter()` again causes rendering to break.

Thanks @josephjohnston for finding this bug!

fixes #129 


Patch:
diff --git a/src/LoadingManager.js b/src/LoadingManager.js
index d03c58d9..111bfdbc 100644
--- a/src/LoadingManager.js
+++ b/src/LoadingManager.js
@@ -5,7 +5,6 @@ function LoadingManager(options) {
   this._loadCounter = 0;
   this._options = options;
   this._isLoading = false;
-  this._loadingPromise = null;
 
   // use this to attribute to determine if there was a problem with loading
   this.loadingFailed = false;
@@ -25,7 +24,7 @@ LoadingManager.prototype._debouncedLoad = function(char, count) {
 };
 
 LoadingManager.prototype._setupLoadingPromise = function() {
-  this._loadingPromise = new Promise((resolve, reject) => {
+  return new Promise((resolve, reject) => {
     this._resolve = resolve;
     this._reject = reject;
   }).then((data) => {
@@ -48,14 +47,12 @@ LoadingManager.prototype._setupLoadingPromise = function() {
 
 LoadingManager.prototype.loadCharData = function(char) {
   this._loadingChar = char;
-  if (!this._isLoading) {
-    this._setupLoadingPromise();
-  }
+  const promise = this._setupLoadingPromise();
   this.loadingFailed = false;
   this._isLoading = true;
   this._loadCounter++;
   this._debouncedLoad(char, this._loadCounter);
-  return this._loadingPromise;
+  return promise;
 };
 
 module.exports = LoadingManager;


Test patch:
diff --git a/src/__tests__/LoadingManager-test.js b/src/__tests__/LoadingManager-test.js
index bbc68c54..c0b2ee87 100644
--- a/src/__tests__/LoadingManager-test.js
+++ b/src/__tests__/LoadingManager-test.js
@@ -94,15 +94,21 @@ describe('LoadingManager', () => {
 
       const loadPromise1 = manager.loadCharData('人');
       const loadPromise2 = manager.loadCharData('他');
-      // it should return the same promise for both since loading isn't complete
-      expect(loadPromise1).toBe(loadPromise2);
+      expect(loadPromise1).not.toBe(loadPromise2);
+
+      let hasPromise1Resolved = false;
+      loadPromise1.then(() => {
+        hasPromise1Resolved = true;
+      });
 
       onCompleteFns[0].call(null, ren);
       onCompleteFns[1].call(null, ta);
 
-      const data = await loadPromise1;
+      const data = await loadPromise2;
+
+      // ren should not resolve, since we requested something else before it finished loading
+      expect(hasPromise1Resolved).toBe(false);
 
-      // ren should be ignored. It's like it was never requested at all
       expect(data).toBe(ta);
       expect(onLoadCharDataSuccess.mock.calls.length).toBe(1);
       expect(onLoadCharDataSuccess.mock.calls[0][0]).toBe(ta);