Skip to content

Commit

Permalink
BUG: Make all l.w. inputs go thru parseAndValidate
Browse files Browse the repository at this point in the history
Closes biocore#135! It would be nice to have some Empress / side-panel /
animator tests that verify things work properly, but i think this is
ok for now.

Also of note: now, the default line width is 0. This will be important
for biocore#144.
  • Loading branch information
fedarko committed Jul 18, 2020
1 parent 29c831a commit 86d4eaa
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 35 deletions.
15 changes: 4 additions & 11 deletions empress/support_files/js/animation-panel-handler.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
define(["Colorer"], function (Colorer) {
define(["Colorer", "util"], function (Colorer, util) {
/**
*
* @class AnimationPanel
Expand Down Expand Up @@ -142,16 +142,9 @@ define(["Colorer"], function (Colorer) {
* Sets line width parameter in animation state machine.
*/
this.lWidth.onchange = function () {
var val = scope.lWidth.value;

// make sure line width is positve
if (val < 1) {
val = 1;
scope.lWidth = val;
}

var lw = util.parseAndValidateLineWidth(scope.lWidth);
// pass line width to state machine
scope.animator.setLineWidth(val);
scope.animator.setLineWidth(lw);
};

/**
Expand All @@ -178,7 +171,7 @@ define(["Colorer"], function (Colorer) {
gradient,
cm,
hide,
lWidth - 1
lWidth
);

// start animation
Expand Down
4 changes: 2 additions & 2 deletions empress/support_files/js/animator.js
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ define(["Colorer", "util"], function (Colorer, util) {
* @param {String} cm The color map to use for the animation
* @param {Boolean} hide Tells animator to hide uncolored branches
* @param {Number} lWidth Tells animator how thick to make colored tree
* branches
* branches
*/
Animator.prototype.setAnimationParameters = function (
trajectory,
Expand Down Expand Up @@ -229,7 +229,7 @@ define(["Colorer", "util"], function (Colorer, util) {
this.legend.clearAllLegends();
this.legend.addColorKey(name, keyInfo, "node", false);

//draw tree
// draw tree
this.empress.resetTree();
this.empress._colorTree(obs, this.cm);
this.empress.thickenSameSampleLines(this.lWidth);
Expand Down
28 changes: 12 additions & 16 deletions empress/support_files/js/empress.js
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ define([
* @type{Number}
* The line width used for drawing "thick" lines.
*/
this._currentLineWidth = 1;
this._currentLineWidth = 0;

/**
* @type{CanvasEvents}
Expand Down Expand Up @@ -566,22 +566,21 @@ define([
};

/**
* Thickens the branches that belong to unique sample categories
* (i.e. features that are only in gut)
* Thickens the colored branches of the tree.
*
* @param {Number} level - Desired line thickness (note that this will be
* applied on both sides of the line -- so if
* level = 1 here then the drawn thick line will
* have a width of 1 + 1 = 2).
* @param {Number} level Amount of thickness to use. If this is <= 0, this
* function won't do anything.
*/
Empress.prototype.thickenSameSampleLines = function (level) {
// we do this because SidePanel._updateSample() calls this function
// with lWidth - 1, so in order to make sure we're setting this
// properly we add 1 to this value.
this._currentLineWidth = level + 1;
// If level isn't > 0, then we don't thicken colored lines at all --
// we just leave them at their default width.
if (level <= 0) {
return;
}
this._currentLineWidth = level;
var tree = this._tree;

// the coordinate of the tree.
// the coordinates of the tree
var coords = [];
this._drawer.loadSampleThickBuf([]);

Expand Down Expand Up @@ -1057,10 +1056,7 @@ define([
// in drawTree(). Doing these calls out of order (draw tree,
// then call thickenSameSampleLines()) causes the thick-line
// stuff to only change whenever the tree is redrawn.
if (this._currentLineWidth !== 1) {
// The - 1 mimics the behavior of SidePanel._updateSample()
this.thickenSameSampleLines(this._currentLineWidth - 1);
}
this.thickenSameSampleLines(this._currentLineWidth);
// this._drawer.loadNodeBuff(this.getNodeCoords());
// this.drawTree();
this.centerLayoutAvgPoint();
Expand Down
10 changes: 4 additions & 6 deletions empress/support_files/js/side-panel-handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -167,10 +167,10 @@ define(["underscore", "Colorer", "util"], function (_, Colorer, util) {
* method as an argument seems to cause
* problems due to "this" not working
* properly. This was the easiest solution.)
* @param{lwInput} HTMLElement An <input> with type="number" from which
* @param{HTMLElement} lwInput An <input> with type="number" from which
* we'll get the .value indicating the line
* width to use when thickening lines.
* @param{updateBtn} HTMLElement This element will be hidden at the end of
* @param{HTMLElement} updateBtn This element will be hidden at the end of
* this function. It should correspond to the
* "Update" button for the sample or feature
* metadata coloring tab.
Expand All @@ -191,10 +191,8 @@ define(["underscore", "Colorer", "util"], function (_, Colorer, util) {
// color tree
this[colorMethodName]();

var lWidth = parseInt(lwInput.value);
if (lWidth !== 1) {
this.empress.thickenSameSampleLines(lWidth - 1);
}
var lw = util.parseAndValidateLineWidth(lwInput);
this.empress.thickenSameSampleLines(lw);
this.empress.drawTree();
};

Expand Down

0 comments on commit 86d4eaa

Please sign in to comment.