Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
feat(all): contain inline css prop setting
Browse files Browse the repository at this point in the history
Removed setting css props, which works as same way.
There's no need to set explicitly.

- display: block
- visibility: visible
- opacity: 1
- fill-opacity: 1

Ref naver#2076
netil committed May 18, 2021
1 parent b064cc1 commit 12cae9e
Showing 28 changed files with 152 additions and 162 deletions.
2 changes: 1 addition & 1 deletion src/Chart/api/show.ts
Original file line number Diff line number Diff line change
@@ -20,7 +20,7 @@ function showHide(show: boolean, targetIdsValue: string[], options: any): void {
$$[`${show ? "remove" : "add"}HiddenTargetIds`](targetIds);

const targets = $$.$el.svg.selectAll($$.selectorTargets(targetIds));
const opacity = show ? "1" : "0";
const opacity = show ? null : "0";

show && targets.style("display", null);

2 changes: 1 addition & 1 deletion src/Chart/api/subchart.ts
Original file line number Diff line number Diff line change
@@ -48,7 +48,7 @@ export default {
$target = subchart.main.selectAll(`.${CLASS.target}`);
}

$target.style("opacity", "1");
$target.style("opacity", null);
subchart.main.style("display", null);

this.flush();
10 changes: 5 additions & 5 deletions src/ChartInternal/Axis/Axis.ts
Original file line number Diff line number Diff line change
@@ -116,7 +116,7 @@ class Axis {
return res;
})
.attr("transform", $$.getTranslate(v))
.style("visibility", config[`axis_${v}_show`] ? "visible" : "hidden");
.style("visibility", config[`axis_${v}_show`] ? null : "hidden");

axis[v].append("text")
.attr("class", classLabel)
@@ -219,7 +219,7 @@ class Axis {
if (g.empty()) {
g = main.append("g")
.attr("class", className)
.style("visibility", config[`axis_${id}_show`] ? "visible" : "hidden")
.style("visibility", config[`axis_${id}_show`] ? null : "hidden")
.call(v);
} else {
axesConfig[i].domain && scale.domain(axesConfig[i].domain);
@@ -862,7 +862,7 @@ class Axis {
redraw(transitions, isHidden, isInit) {
const $$ = this.owner;
const {config, $el} = $$;
const opacity = isHidden ? "0" : "1";
const opacity = isHidden ? "0" : null;

["x", "y", "y2", "subX"].forEach(id => {
const axis = this[id];
@@ -989,10 +989,10 @@ class Axis {
}

tickText.each(function(d) {
this.style.display = tickValues.indexOf(d) % intervalForCulling ? "none" : "block";
this.style.display = tickValues.indexOf(d) % intervalForCulling ? "none" : null;
});
} else {
tickText.style("display", "block");
tickText.style("display", null);
}

// set/unset x_axis_tick_clippath
5 changes: 2 additions & 3 deletions src/ChartInternal/Axis/AxisRenderer.ts
Original file line number Diff line number Diff line change
@@ -121,8 +121,7 @@ export default class AxisRenderer {
const tickEnter = tick
.enter()
.insert("g", ".domain")
.attr("class", "tick")
.style("opacity", "1");
.attr("class", "tick");

// MEMO: No exit transition. The reason is this transition affects max tick width calculation because old tick will be included in the ticks.
const tickExit = tick.exit().remove();
@@ -216,7 +215,7 @@ export default class AxisRenderer {
}

tickTransform(tickEnter, scale0);
tickTransform(helper.transitionise(tick).style("opacity", "1"), scale1);
tickTransform(helper.transitionise(tick).style("opacity", null), scale1);
}
});

6 changes: 3 additions & 3 deletions src/ChartInternal/ChartInternal.ts
Original file line number Diff line number Diff line change
@@ -583,7 +583,7 @@ export default class ChartInternal {
.filter(d => $$.isTargetToShow(d.id))
.transition()
.duration(config.transition_duration)
.style("opacity", "1");
.style("opacity", null);
}

getWithOption(options) {
@@ -616,12 +616,12 @@ export default class ChartInternal {
return withOptions;
}

initialOpacity(d): "1" | "0" {
initialOpacity(d): null | "0" {
const $$ = <any> this;
const {withoutFadeIn} = $$.state;

return $$.getBaseValue(d) !== null &&
withoutFadeIn[d.id] ? "1" : "0";
withoutFadeIn[d.id] ? null : "0";
}

bindResize(): void {
4 changes: 2 additions & 2 deletions src/ChartInternal/interactions/subchart.ts
Original file line number Diff line number Diff line change
@@ -131,7 +131,7 @@ export default {
return;
}

const visibility = config.subchart_show ? "visible" : "hidden";
const visibility = config.subchart_show ? null : "hidden";
const clipId = `${clip.id}-subchart`;
const clipPath = $$.getClipPath(clipId);

@@ -289,7 +289,7 @@ export default {
const {config, $el: {subchart: {main}}, state} = $$;
const withTransition = !!duration;

main.style("visibility", config.subchart_show ? "visible" : "hidden");
main.style("visibility", config.subchart_show ? null : "hidden");

// subchart
if (config.subchart_show) {
16 changes: 8 additions & 8 deletions src/ChartInternal/internals/grid.ts
Original file line number Diff line number Diff line change
@@ -125,7 +125,7 @@ export default {
grid.attr(id, state.xgridAttr[id])
.style("opacity", () => (
grid.attr(isRotated ? "y1" : "x1") === (isRotated ? state.height : 0) ?
"0" : "1"
"0" : null
));
});
});
@@ -166,7 +166,7 @@ export default {
!gridLines.main && $$.initGridLines();

// hide if arc type
grid.main.style("visibility", $$.hasArcType() ? "hidden" : "visible");
grid.main.style("visibility", $$.hasArcType() ? "hidden" : null);

$$.hideGridFocus();
$$.updateXGridLines(duration);
@@ -217,7 +217,7 @@ export default {
.duration(duration)
.text(d => d.text)
.transition()
.style("opacity", "1");
.style("opacity", null);

gridLines.x = xLines;
},
@@ -270,7 +270,7 @@ export default {
.attr("y1", isRotated ? 0 : yv)
.attr("y2", isRotated ? height : yv)
.transition()
.style("opacity", "1");
.style("opacity", null);

ygridLines.select("text")
.attr("text-anchor", getGridTextAnchor)
@@ -282,7 +282,7 @@ export default {
.attr("y", yv)
.text(d => d.text)
.transition()
.style("opacity", "1");
.style("opacity", null);

$el.gridLines.y = ygridLines;
},
@@ -311,8 +311,8 @@ export default {
.text(d => d.text);

return [
lines.style("opacity", "1"),
texts.style("opacity", "1")
lines.style("opacity", null),
texts.style("opacity", null)
];
},

@@ -372,7 +372,7 @@ export default {
const xx = $$.xx.bind($$);

focusEl
.style("visibility", "visible")
.style("visibility", null)
.data(dataToShow.concat(dataToShow))
.each(function(d) {
const el = d3Select(this);
31 changes: 8 additions & 23 deletions src/ChartInternal/internals/legend.ts
Original file line number Diff line number Diff line change
@@ -244,16 +244,6 @@ export default {
) : 0;
},

/**
* Get the opacity of the legend
* @param {d3.selection} legendItem Legend item node
* @returns {string|null} opacity
* @private
*/
opacityForLegend(legendItem): string | null {
return legendItem.classed(CLASS.legendItemHidden) ? null : "1";
},

/**
* Get the opacity of the legend that is unfocused
* @param {d3.selection} legendItem Legend item node
@@ -281,8 +271,8 @@ export default {
.transition()
.duration(100)
.style("opacity", function() {
return (focus ? $$.opacityForLegend : $$.opacityForUnfocusedLegend)
.call($$, d3Select(this));
return focus ? null :
$$.opacityForUnfocusedLegend.call($$, d3Select(this));
});
},

@@ -298,9 +288,7 @@ export default {
.classed(CLASS.legendItemFocused, false)
.transition()
.duration(100)
.style("opacity", function() {
return $$.opacityForLegend(d3Select(this));
});
.style("opacity", null);
},

/**
@@ -316,7 +304,7 @@ export default {
config.legend_show = true;

$el.legend ?
$el.legend.style("visibility", "visible") :
$el.legend.style("visibility", null) :
$$.initLegend();

!$$.state.legendHasRendered && $$.updateLegend();
@@ -325,11 +313,9 @@ export default {
$$.removeHiddenLegendIds(targetIds);

$el.legend.selectAll($$.selectorLegends(targetIds))
.style("visibility", "visible")
.style("visibility", null)
.transition()
.style("opacity", function() {
return $$.opacityForLegend(d3Select(this));
});
.style("opacity", null);
},

/**
@@ -399,7 +385,7 @@ export default {

return itemClass + $$.generateClass(CLASS.legendItem, id);
})
.style("visibility", id => ($$.isLegendToShow(id) ? "visible" : "hidden"));
.style("visibility", id => ($$.isLegendToShow(id) ? null : "hidden"));

if (config.interaction_enabled) {
item
@@ -413,8 +399,7 @@ export default {
api.toggle(id);

d3Select(this)
.classed(CLASS.legendItemFocused, false)
.style("opacity", null);
.classed(CLASS.legendItemFocused, false);
}
}

4 changes: 2 additions & 2 deletions src/ChartInternal/internals/region.ts
Original file line number Diff line number Diff line change
@@ -27,7 +27,7 @@ export default {
}

// hide if arc type
region.main.style("visibility", $$.hasArcType() ? "hidden" : "visible");
region.main.style("visibility", $$.hasArcType() ? "hidden" : null);
// select <g> element

let list = region.main
@@ -64,7 +64,7 @@ export default {

return [
(withTransition ? regions.transition() : regions)
.style("fill-opacity", d => (isValue(d.opacity) ? d.opacity : "0.1"))
.style("fill-opacity", d => (isValue(d.opacity) ? d.opacity : null))
.on("end", function() {
// remove unnecessary rect after transition
d3Select(this.parentNode)
4 changes: 2 additions & 2 deletions src/ChartInternal/internals/text.ts
Original file line number Diff line number Diff line change
@@ -13,12 +13,12 @@ import {AxisType} from "../../../types/types";


export default {
opacityForText(d): "1" | "0" {
opacityForText(d): null | "0" {
const $$ = this;

return $$.isBarType(d) && !$$.meetsLabelThreshold(
Math.abs($$.getRatio("bar", d),), "bar"
) ? "0" : ($$.hasDataLabel ? "1" : "0");
) ? "0" : ($$.hasDataLabel ? null : "0");
},

/**
2 changes: 1 addition & 1 deletion src/ChartInternal/internals/tooltip.ts
Original file line number Diff line number Diff line change
@@ -75,7 +75,7 @@ export default {
if (!config.tooltip_contents.bindto) {
$el.tooltip.style("top", config.tooltip_init_position.top)
.style("left", config.tooltip_init_position.left)
.style("display", "block");
.style("display", null);
}
}
},
10 changes: 5 additions & 5 deletions src/ChartInternal/shape/arc.ts
Original file line number Diff line number Diff line change
@@ -407,7 +407,7 @@ export default {
.attr("d", $$.svgArc);

svg.selectAll(`${CLASS.arc}`)
.style("opacity", "1");
.style("opacity", null);
},

/**
@@ -581,7 +581,7 @@ export default {
mainArc
.attr("transform", d => (!$$.isGaugeType(d.data) && withTransform ? "scale(0)" : ""))
.style("opacity", function(d) {
return d === this._current ? "0" : "1";
return d === this._current ? "0" : null;
})
.each(() => {
state.transiting = true;
@@ -630,7 +630,7 @@ export default {
return color;
})
// Where gauge reading color would receive customization.
.style("opacity", "1")
.style("opacity", null)
.call(endall, function() {
if ($$.levelColor) {
const path = d3Select(this);
@@ -830,13 +830,13 @@ export default {
))
.transition()
.duration(duration)
.style("opacity", d => ($$.isTargetToShow(d.data.id) && $$.isArcType(d.data) ? "1" : "0"));
.style("opacity", d => ($$.isTargetToShow(d.data.id) && $$.isArcType(d.data) ? null : "0"));

hasMultiArcGauge && text.attr("dy", "-.1em");
}

main.select(`.${CLASS.chartArcsTitle}`)
.style("opacity", $$.hasType("donut") || hasGauge ? "1" : "0");
.style("opacity", $$.hasType("donut") || hasGauge ? null : "0");

if (hasGauge) {
const isFullCircle = config.gauge_fullCircle;
2 changes: 1 addition & 1 deletion src/ChartInternal/shape/bar.ts
Original file line number Diff line number Diff line change
@@ -85,7 +85,7 @@ export default {
(withTransition ? bar.transition(getRandom()) : bar)
.attr("d", drawFn)
.style("fill", this.color)
.style("opacity", "1")
.style("opacity", null)
];
},

2 changes: 1 addition & 1 deletion src/ChartInternal/shape/candlestick.ts
Original file line number Diff line number Diff line change
@@ -233,7 +233,7 @@ export default {

drawFn(d, i, g);
})
.style("opacity", "1")
.style("opacity", null)
];
},

2 changes: 1 addition & 1 deletion src/ChartInternal/shape/gauge.ts
Original file line number Diff line number Diff line change
@@ -64,7 +64,7 @@ export default {

mainArcLabelLine
.style("fill", d => ($$.levelColor ? $$.levelColor(d.data.values[0].value) : $$.color(d.data)))
.style("display", config.gauge_label_show ? "" : "none")
.style("display", config.gauge_label_show ? null : "none")
.each(function(d) {
let lineLength = 0;
const lineThickness = 2;
2 changes: 1 addition & 1 deletion src/ChartInternal/shape/line.ts
Original file line number Diff line number Diff line change
@@ -95,7 +95,7 @@ export default {
(withTransition ? line.transition(getRandom()) : line)
.attr("d", drawFn)
.style("stroke", this.color)
.style("opacity", "1")
.style("opacity", null)
];
},

4 changes: 2 additions & 2 deletions src/ChartInternal/shape/point.ts
Original file line number Diff line number Diff line change
@@ -42,7 +42,7 @@ export default {
let opacity = config.point_opacity;

if (isUndefined(opacity)) {
opacity = config.point_show && !config.point_focus_only ? "1" : "0";
opacity = config.point_show && !config.point_focus_only ? null : "0";

opacity = isValue(this.getBaseValue(d)) ?
(this.isBubbleType(d) || this.isScatterType(d) ?
@@ -203,7 +203,7 @@ export default {

circle
.attr("class", this.updatePointClass.bind(this))
.style("opacity", "1")
.style("opacity", null)
.each(function(d) {
const {id, index, value} = d;
let visibility = "hidden";
4 changes: 2 additions & 2 deletions src/Plugin/stanford/Elements.ts
Original file line number Diff line number Diff line change
@@ -63,7 +63,7 @@ export default class Elements {
.attr("y1", d => (isRotated ? xvCustom(d, "x1") : yvCustom(d, "y1")))
.attr("y2", d => (isRotated ? xvCustom(d, "x2") : yvCustom(d, "y2")))
.transition()
.style("opacity", "1");
.style("opacity", null);
}

updateStanfordRegions(duration: number): void {
@@ -127,7 +127,7 @@ export default class Elements {
.attr("text-anchor", "middle")
.attr("dominant-baseline", "middle")
.transition()
.style("opacity", "1");
.style("opacity", null);
}

updateStanfordElements(duration = 0): void {
168 changes: 87 additions & 81 deletions test/api/focus-spec.ts

Large diffs are not rendered by default.

10 changes: 5 additions & 5 deletions test/api/legend-spec.ts
Original file line number Diff line number Diff line change
@@ -38,18 +38,18 @@ describe("API legend", () => {
setTimeout(() => {
chart.internal.$el.svg.selectAll(`.${CLASS.legendItem}`).each(function() {

expect(+this.style.opacity).to.be.equal(1);
expect(this.style.opacity).to.be.equal("");
});

done();
}, 500)
}, 300)
});

it("it should hide 'data1' legend", () => {
chart.legend.hide("data1");

chart.internal.$el.svg.selectAll(`.${CLASS.legendItem}`).each(function(v) {
expect(+this.style.opacity).to.be.equal(v === "data1" ? 0 : 1);
expect(this.style.opacity).to.be.equal(v === "data1" ? "0" : "");
});
});

@@ -59,10 +59,10 @@ describe("API legend", () => {

setTimeout(() => {
chart.internal.$el.svg.selectAll(`.${CLASS.legendItem}`).each(function(v) {
expect(+this.style.opacity).to.be.equal(1);
expect(this.style.opacity).to.be.equal("");
});

done();
}, 500);
}, 300);
});
});
10 changes: 5 additions & 5 deletions test/api/show-spec.ts
Original file line number Diff line number Diff line change
@@ -149,15 +149,15 @@ describe("API show", () => {

setTimeout(() => {
main.selectAll(`.${CLASS.chartLine}`).each(function() {
expect(+this.style.opacity).to.be.equal(1);
expect(this.style.opacity).to.be.equal("");
});

legend = internal.$el.svg.selectAll(`.${CLASS.legendItemHidden}`);

expect(+legend.size()).to.be.equal(0);

legend.each(function() {
expect(+d3Select(this).style("opacity")).to.be.equal(1);
expect(d3Select(this).style("opacity")).to.be.equal("");
});

done();
@@ -176,7 +176,7 @@ describe("API show", () => {

setTimeout(() => {
main.selectAll(`.${CLASS.chartLine}`).each(function() {
expect(+this.style.opacity).to.be.equal(1);
expect(this.style.opacity).to.be.equal("");
});

const legend = internal.$el.svg.selectAll(`.${CLASS.legendItemHidden}`);
@@ -220,15 +220,15 @@ describe("API show", () => {

setTimeout(() => {
main.selectAll(`.${CLASS.chartLine}`).each(function() {
expect(+this.style.opacity).to.be.equal(1);
expect(this.style.opacity).to.be.equal("");
});

legend = internal.$el.svg.selectAll(`.${CLASS.legendItemHidden}`);

expect(+legend.size()).to.be.equal(0);

legend.each(function() {
expect(+d3Select(this).style("opacity")).to.be.equal(1);
expect(d3Select(this).style("opacity")).to.be.equal("");
});

done();
2 changes: 1 addition & 1 deletion test/interactions/zoom-spec.ts
Original file line number Diff line number Diff line change
@@ -917,7 +917,7 @@ describe("ZOOM", function() {
chart.zoom([4,5]);

const tickTexts = chart.$.main.selectAll(`.${CLASS.axisY} .tick text`)
.filter(function() { return this.style.display === "block"});
.filter(function() { return this.style.display === ""});

expect(tickTexts.size()).to.be.equal(args.axis.y.tick.culling.max);
});
2 changes: 1 addition & 1 deletion test/internals/axis-spec.ts
Original file line number Diff line number Diff line change
@@ -2419,7 +2419,7 @@ describe("AXIS", function() {
["subX", "x", "y", "y2"].forEach(v => {
const data = chart.internal.$el.axis[v]
.selectAll(".tick text").filter(function() {
return this.style.display === "block";
return this.style.display === "";
}).data();

expect(data).to.be.deep.equal(expected[v === "subX" ? "x" : v]);
2 changes: 1 addition & 1 deletion test/internals/bb-spec.ts
Original file line number Diff line number Diff line change
@@ -196,7 +196,7 @@ describe("Interface & initialization", () => {
setTimeout(() => {
expect(+chart.internal.$el.svg.attr("height")).to.be.equal(chartHeight);
done();
}, 200);
}, 500);
});

it("should be resizing all generated chart elements", function(done) {
2 changes: 1 addition & 1 deletion test/shape/area-spec.ts
Original file line number Diff line number Diff line change
@@ -161,7 +161,7 @@ describe("SHAPE AREA", () => {

// null data points, shouldn't be showing
chart.$.circles.filter(d => d.id === dataName).each(function(d, i) {
expect(+this.style.opacity).to.be.equal(i > 1 ? 1 : 0);
expect(this.style.opacity).to.be.equal(i > 1 ? '' : "0");
})
};

2 changes: 1 addition & 1 deletion test/shape/bar-spec.ts
Original file line number Diff line number Diff line change
@@ -705,7 +705,7 @@ describe("SHAPE BAR", () => {
const hiddenIds = chart.internal.state.hiddenTargetIds;

const res = chart.$.text.texts.filter(function(d) {
return hiddenIds.indexOf(d.id) === -1 && this.style.fillOpacity === "1";
return hiddenIds.indexOf(d.id) === -1 && this.style.fillOpacity === "";
}).nodes().map(n => +n.textContent);

expect(res).to.be.deep.equal(expected);
2 changes: 1 addition & 1 deletion test/shape/line-spec.ts
Original file line number Diff line number Diff line change
@@ -101,7 +101,7 @@ describe("SHAPE LINE", () => {

setTimeout(() => {
target.each(function(d, i) {
expect(this.style.opacity).to.be.equal(d.index === 1 ? "0" : "1");
expect(this.style.opacity).to.be.equal(d.index === 1 ? "0" : "");
});

done();
2 changes: 1 addition & 1 deletion test/shape/point-spec.ts
Original file line number Diff line number Diff line change
@@ -321,7 +321,7 @@ describe("SHAPE POINT", () => {
chart.tooltip.show({x});

circles.each(function(d) {
expect(+this.style.opacity).to.be.equal(1);
expect(this.style.opacity).to.be.equal("");
expect(d.x).to.be.equal(x);
expect(+this.getAttribute("cx")).to.be.equal(cx);
});

0 comments on commit 12cae9e

Please sign in to comment.