Skip to content

Commit

Permalink
Merge pull request #4 from BloomBooks/updateSpecAdjustRt
Browse files Browse the repository at this point in the history
Update data-bubble spec when adjustRoot moves midpoint
  • Loading branch information
sujeffreyl authored Oct 1, 2019
2 parents 1b02cad + c372997 commit 54aef28
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 2 deletions.
32 changes: 31 additions & 1 deletion src/bubble.ts
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,31 @@ export default class Bubble {
);
this.shape.position = contentCenter;
this.innerShape.position = contentCenter;
this.tails.forEach(tail => tail.adjustRoot(contentCenter));
// Enhance: I think we could extract from this a method updateTailSpec
// which loops over all the tails and if any tail's spec doesn't match the tail,
// it turns off the mutation observer while updating the spec to match.
// Such a method would be useful for updating the spec when the tail is dragged,
// and perhaps for other things.
let tailChanged = false;
this.tails.forEach((tail, index) => {
if (tail.adjustRoot(contentCenter)) {
const tip = this.spec.tips[index];
tip.midpointX = tail.mid.x!;
tip.midpointY = tail.mid.y!;
tailChanged = true;
}
});
if (tailChanged) {
// if no tail changed we MUST NOT modify the element,
// as doing so will trigger the mutation observer.
// Even if it did, we don't want to trigger a recursive call.
const wasMonitoring = !!this.observer;
this.stopMonitoring();
this.setBubbleSpec(this.spec);
if (wasMonitoring){
this.monitorContent();
}
}
};

// A callback for after the shape is loaded/place.
Expand Down Expand Up @@ -377,6 +401,12 @@ export default class Bubble {
midpointX: curveHandle!.position!.x!,
midpointY: curveHandle!.position!.y!
};
// todo: it isn't necessarily tip 0 that changed.
// to fix: there's only one caller of this method, drawTailAfterShapePlaced, which has only one caller,
// a loop in makeShapes() which is a foreach over the tips. Collapse that method and this into a single
// method (makeTail would be a better name than either)
// that takes the tip and tip index. Then use that index to know which tip to update.
// Consider turning off the mutation observer while we update the bubble spec, as in adjustSize.
this.spec.tips[0] = newTip; // enhance: for multiple tips, need to figure which one to update

this.setBubbleSpec(this.spec);
Expand Down
9 changes: 8 additions & 1 deletion src/tail.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,13 +95,20 @@ export class Tail {
this.makeShapes();
}

adjustRoot(newRoot: Point) {
adjustRoot(newRoot: Point) : boolean {
const delta = newRoot.subtract(this.root!).divide(2);
if (Math.abs(delta.x!) + Math.abs(delta.y!) < 0.0001) {
// hasn't moved; very likely adjustSize triggered by an irrelevant change to object;
// We MUST NOT trigger the mutation observer again, or we get an infinte loop that
// freezes the whole page.
return false;
}
const newMid = this.mid.add(delta);
this.updatePoints(newRoot, this.tip, newMid);
if (this.midHandle) {
this.midHandle.position = newMid;
}
return true;
}

// Erases the tail from the canvas
Expand Down

0 comments on commit 54aef28

Please sign in to comment.