From 23569cd2d275e7644092343d51c048565aae4f44 Mon Sep 17 00:00:00 2001 From: Mojtaba Samimi Date: Mon, 19 Sep 2022 15:25:55 -0400 Subject: [PATCH 1/5] handle transparent colors in rangeslider --- src/components/rangeslider/draw.js | 15 +++++++++++++-- test/jasmine/tests/range_slider_test.js | 20 ++++++++++---------- 2 files changed, 23 insertions(+), 12 deletions(-) diff --git a/src/components/rangeslider/draw.js b/src/components/rangeslider/draw.js index 348c45697e9..798aa509f44 100644 --- a/src/components/rangeslider/draw.js +++ b/src/components/rangeslider/draw.js @@ -1,5 +1,6 @@ 'use strict'; +var tinycolor = require('tinycolor2'); var d3 = require('@plotly/d3'); var Registry = require('../../registry'); @@ -395,12 +396,22 @@ function drawBg(rangeSlider, gd, axisOpts, opts) { var offsetShift = -opts._offsetShift; var lw = Drawing.crispRound(gd, opts.borderwidth); + var fillColor = tinycolor(opts.bgcolor); + var fillAlpha = fillColor.getAlpha(); + var fillRGB = Color.tinyRGB(fillColor); + + var strokeColor = tinycolor(opts.bordercolor); + var strokeAlpha = strokeColor.getAlpha(); + var strokeRGB = Color.tinyRGB(strokeColor); + bg.attr({ width: opts._width + borderCorrect, height: opts._height + borderCorrect, transform: strTranslate(offsetShift, offsetShift), - fill: opts.bgcolor, - stroke: opts.bordercolor, + fill: fillRGB, + 'fill-opacity': fillAlpha, + stroke: strokeRGB, + 'stroke-opacity': strokeAlpha, 'stroke-width': lw }); } diff --git a/test/jasmine/tests/range_slider_test.js b/test/jasmine/tests/range_slider_test.js index 197d46f946d..37779d75332 100644 --- a/test/jasmine/tests/range_slider_test.js +++ b/test/jasmine/tests/range_slider_test.js @@ -75,8 +75,8 @@ describe('Visible rangesliders', function() { expect(+bg.getAttribute('width')).toEqual(expectedWidth); expect(+bg.getAttribute('height')).toEqual(66); - expect(bg.getAttribute('fill')).toBe('#fafafa'); - expect(bg.getAttribute('stroke')).toBe('black'); + expect(bg.getAttribute('fill')).toBe('rgb(250, 250, 250)'); + expect(bg.getAttribute('stroke')).toBe('rgb(0, 0, 0)'); expect(+bg.getAttribute('stroke-width')).toBe(2); return Plotly.relayout(gd, { @@ -91,8 +91,8 @@ describe('Visible rangesliders', function() { expect(+bg.getAttribute('height')).toEqual(32); - expect(bg.getAttribute('fill')).toBe('#ffff80'); - expect(bg.getAttribute('stroke')).toBe('#404040'); + expect(bg.getAttribute('fill')).toBe('rgb(255, 255, 128)'); + expect(bg.getAttribute('stroke')).toBe('rgb(64, 64, 64)'); expect(+bg.getAttribute('stroke-width')).toBe(1); }) .then(done, done.fail); @@ -366,8 +366,8 @@ describe('Visible rangesliders', function() { expect(+maskMin.getAttribute('width')).toEqual(maskMinWidth); expect(+maskMax.getAttribute('width')).toEqual(maskMaxWidth); - expect(bg.getAttribute('fill')).toBe('red'); - expect(bg.getAttribute('stroke')).toBe('black'); + expect(bg.getAttribute('fill')).toBe('rgb(255, 0, 0)'); + expect(bg.getAttribute('stroke')).toBe('rgb(0, 0, 0)'); expect(bg.getAttribute('stroke-width')).toBe('2'); return Plotly.relayout(gd, 'xaxis.rangeslider.bordercolor', 'blue'); @@ -376,8 +376,8 @@ describe('Visible rangesliders', function() { expect(+maskMin.getAttribute('width')).toEqual(maskMinWidth); expect(+maskMax.getAttribute('width')).toEqual(maskMaxWidth); - expect(bg.getAttribute('fill')).toBe('red'); - expect(bg.getAttribute('stroke')).toBe('blue'); + expect(bg.getAttribute('fill')).toBe('rgb(255, 0, 0)'); + expect(bg.getAttribute('stroke')).toBe('rgb(0, 0, 255)'); expect(bg.getAttribute('stroke-width')).toBe('2'); return Plotly.relayout(gd, 'xaxis.rangeslider.borderwidth', 3); @@ -386,8 +386,8 @@ describe('Visible rangesliders', function() { expect(+maskMin.getAttribute('width')).toEqual(maskMinWidth); expect(+maskMax.getAttribute('width')).toEqual(maskMaxWidth); - expect(bg.getAttribute('fill')).toBe('red'); - expect(bg.getAttribute('stroke')).toBe('blue'); + expect(bg.getAttribute('fill')).toBe('rgb(255, 0, 0)'); + expect(bg.getAttribute('stroke')).toBe('rgb(0, 0, 255)'); expect(bg.getAttribute('stroke-width')).toBe('3'); }) .then(done, done.fail); From e00cde4cbbb548cc50e243e299ab2b1252f4d6ce Mon Sep 17 00:00:00 2001 From: Mojtaba Samimi Date: Mon, 19 Sep 2022 17:10:17 -0400 Subject: [PATCH 2/5] handle transparent colors in violin --- src/traces/violin/hover.js | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/traces/violin/hover.js b/src/traces/violin/hover.js index 883350e4032..5832694c53b 100644 --- a/src/traces/violin/hover.js +++ b/src/traces/violin/hover.js @@ -1,5 +1,8 @@ 'use strict'; +var tinycolor = require('tinycolor2'); + +var Color = require('../../components/color'); var Lib = require('../../lib'); var Axes = require('../../plots/cartesian/axes'); var boxHoverPoints = require('../box/hover'); @@ -75,7 +78,15 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode, opts) { closeData.push(kdePointData); - violinLineAttrs = {stroke: pointData.color}; + var strokeC = pointData.color; + var strokeColor = tinycolor(strokeC); + var strokeAlpha = strokeColor.getAlpha(); + var strokeRGB = Color.tinyRGB(strokeColor); + + violinLineAttrs = { + stroke: strokeRGB, + 'stroke-opacity': strokeAlpha + }; violinLineAttrs[pLetter + '1'] = Lib.constrain(paOffset + pOnPath[0], paOffset, paOffset + paLength); violinLineAttrs[pLetter + '2'] = Lib.constrain(paOffset + pOnPath[1], paOffset, paOffset + paLength); violinLineAttrs[vLetter + '1'] = violinLineAttrs[vLetter + '2'] = vAxis._offset + vValPx; From 6111dc0a4ab06503b3acfde6269c297ca4929f43 Mon Sep 17 00:00:00 2001 From: Mojtaba Samimi Date: Mon, 19 Sep 2022 17:48:06 -0400 Subject: [PATCH 3/5] handle transparent colors in selections --- src/components/selections/select.js | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/src/components/selections/select.js b/src/components/selections/select.js index 9155f5417cd..288c874950d 100644 --- a/src/components/selections/select.js +++ b/src/components/selections/select.js @@ -1,5 +1,6 @@ 'use strict'; +var tinycolor = require('tinycolor2'); var polybool = require('polybooljs'); var pointInPolygon = require('point-in-polygon/nested'); // could we use contains lib/polygon instead? @@ -112,17 +113,30 @@ function prepSelect(evt, startX, startY, dragOptions, mode) { fullLayout.newshape : fullLayout.newselection; + var fillC = (isDrawMode && !isOpenMode) ? newStyle.fillcolor : 'rgba(0,0,0,0)'; + var fillColor = tinycolor(fillC); + var fillAlpha = fillColor.getAlpha(); + var fillRGB = Color.tinyRGB(fillColor); + + var strokeC = newStyle.line.color || ( + isCartesian ? + Color.contrast(gd._fullLayout.plot_bgcolor) : + '#7f7f7f' // non-cartesian subplot + ); + + var strokeColor = tinycolor(strokeC); + var strokeAlpha = strokeColor.getAlpha(); + var strokeRGB = Color.tinyRGB(strokeColor); + outlines.enter() .append('path') .attr('class', 'select-outline select-outline-' + plotinfo.id) .style({ opacity: isDrawMode ? newStyle.opacity / 2 : 1, - fill: (isDrawMode && !isOpenMode) ? newStyle.fillcolor : 'none', - stroke: newStyle.line.color || ( - isCartesian ? - Color.contrast(gd._fullLayout.plot_bgcolor) : - '#7f7f7f' // non-cartesian subplot - ), + fill: fillRGB, + 'fill-opacity': fillAlpha, + stroke: strokeRGB, + 'stroke-opacity': strokeAlpha, 'stroke-dasharray': dashStyle(newStyle.line.dash, newStyle.line.width), 'stroke-width': newStyle.line.width + 'px', 'shape-rendering': 'crispEdges' From 200733aa23f35a6d5280905ae7aba9d05c517daa Mon Sep 17 00:00:00 2001 From: Mojtaba Samimi Date: Thu, 6 Oct 2022 10:21:25 -0400 Subject: [PATCH 4/5] revisit rangeslider & selections stroke and fill style --- src/components/rangeslider/draw.js | 17 +++-------------- src/components/selections/select.js | 14 ++------------ test/jasmine/tests/range_slider_test.js | 20 ++++++++++---------- 3 files changed, 15 insertions(+), 36 deletions(-) diff --git a/src/components/rangeslider/draw.js b/src/components/rangeslider/draw.js index 798aa509f44..97f9d40e61d 100644 --- a/src/components/rangeslider/draw.js +++ b/src/components/rangeslider/draw.js @@ -1,6 +1,5 @@ 'use strict'; -var tinycolor = require('tinycolor2'); var d3 = require('@plotly/d3'); var Registry = require('../../registry'); @@ -396,24 +395,14 @@ function drawBg(rangeSlider, gd, axisOpts, opts) { var offsetShift = -opts._offsetShift; var lw = Drawing.crispRound(gd, opts.borderwidth); - var fillColor = tinycolor(opts.bgcolor); - var fillAlpha = fillColor.getAlpha(); - var fillRGB = Color.tinyRGB(fillColor); - - var strokeColor = tinycolor(opts.bordercolor); - var strokeAlpha = strokeColor.getAlpha(); - var strokeRGB = Color.tinyRGB(strokeColor); - bg.attr({ width: opts._width + borderCorrect, height: opts._height + borderCorrect, transform: strTranslate(offsetShift, offsetShift), - fill: fillRGB, - 'fill-opacity': fillAlpha, - stroke: strokeRGB, - 'stroke-opacity': strokeAlpha, 'stroke-width': lw - }); + }) + .call(Color.stroke, opts.bordercolor) + .call(Color.fill, opts.bgcolor); } function addClipPath(rangeSlider, gd, axisOpts, opts) { diff --git a/src/components/selections/select.js b/src/components/selections/select.js index 288c874950d..0926e2363dd 100644 --- a/src/components/selections/select.js +++ b/src/components/selections/select.js @@ -1,6 +1,5 @@ 'use strict'; -var tinycolor = require('tinycolor2'); var polybool = require('polybooljs'); var pointInPolygon = require('point-in-polygon/nested'); // could we use contains lib/polygon instead? @@ -114,9 +113,6 @@ function prepSelect(evt, startX, startY, dragOptions, mode) { fullLayout.newselection; var fillC = (isDrawMode && !isOpenMode) ? newStyle.fillcolor : 'rgba(0,0,0,0)'; - var fillColor = tinycolor(fillC); - var fillAlpha = fillColor.getAlpha(); - var fillRGB = Color.tinyRGB(fillColor); var strokeC = newStyle.line.color || ( isCartesian ? @@ -124,23 +120,17 @@ function prepSelect(evt, startX, startY, dragOptions, mode) { '#7f7f7f' // non-cartesian subplot ); - var strokeColor = tinycolor(strokeC); - var strokeAlpha = strokeColor.getAlpha(); - var strokeRGB = Color.tinyRGB(strokeColor); - outlines.enter() .append('path') .attr('class', 'select-outline select-outline-' + plotinfo.id) .style({ opacity: isDrawMode ? newStyle.opacity / 2 : 1, - fill: fillRGB, - 'fill-opacity': fillAlpha, - stroke: strokeRGB, - 'stroke-opacity': strokeAlpha, 'stroke-dasharray': dashStyle(newStyle.line.dash, newStyle.line.width), 'stroke-width': newStyle.line.width + 'px', 'shape-rendering': 'crispEdges' }) + .call(Color.stroke, strokeC) + .call(Color.fill, fillC) .attr('fill-rule', 'evenodd') .classed('cursor-move', isDrawMode ? true : false) .attr('transform', transform) diff --git a/test/jasmine/tests/range_slider_test.js b/test/jasmine/tests/range_slider_test.js index 37779d75332..115915a10a0 100644 --- a/test/jasmine/tests/range_slider_test.js +++ b/test/jasmine/tests/range_slider_test.js @@ -75,8 +75,8 @@ describe('Visible rangesliders', function() { expect(+bg.getAttribute('width')).toEqual(expectedWidth); expect(+bg.getAttribute('height')).toEqual(66); - expect(bg.getAttribute('fill')).toBe('rgb(250, 250, 250)'); - expect(bg.getAttribute('stroke')).toBe('rgb(0, 0, 0)'); + expect(bg.style.fill).toBe('rgb(250, 250, 250)'); + expect(bg.style.stroke).toBe('rgb(0, 0, 0)'); expect(+bg.getAttribute('stroke-width')).toBe(2); return Plotly.relayout(gd, { @@ -91,8 +91,8 @@ describe('Visible rangesliders', function() { expect(+bg.getAttribute('height')).toEqual(32); - expect(bg.getAttribute('fill')).toBe('rgb(255, 255, 128)'); - expect(bg.getAttribute('stroke')).toBe('rgb(64, 64, 64)'); + expect(bg.style.fill).toBe('rgb(255, 255, 128)'); + expect(bg.style.stroke).toBe('rgb(64, 64, 64)'); expect(+bg.getAttribute('stroke-width')).toBe(1); }) .then(done, done.fail); @@ -366,8 +366,8 @@ describe('Visible rangesliders', function() { expect(+maskMin.getAttribute('width')).toEqual(maskMinWidth); expect(+maskMax.getAttribute('width')).toEqual(maskMaxWidth); - expect(bg.getAttribute('fill')).toBe('rgb(255, 0, 0)'); - expect(bg.getAttribute('stroke')).toBe('rgb(0, 0, 0)'); + expect(bg.style.fill).toBe('rgb(255, 0, 0)'); + expect(bg.style.stroke).toBe('rgb(0, 0, 0)'); expect(bg.getAttribute('stroke-width')).toBe('2'); return Plotly.relayout(gd, 'xaxis.rangeslider.bordercolor', 'blue'); @@ -376,8 +376,8 @@ describe('Visible rangesliders', function() { expect(+maskMin.getAttribute('width')).toEqual(maskMinWidth); expect(+maskMax.getAttribute('width')).toEqual(maskMaxWidth); - expect(bg.getAttribute('fill')).toBe('rgb(255, 0, 0)'); - expect(bg.getAttribute('stroke')).toBe('rgb(0, 0, 255)'); + expect(bg.style.fill).toBe('rgb(255, 0, 0)'); + expect(bg.style.stroke).toBe('rgb(0, 0, 255)'); expect(bg.getAttribute('stroke-width')).toBe('2'); return Plotly.relayout(gd, 'xaxis.rangeslider.borderwidth', 3); @@ -386,8 +386,8 @@ describe('Visible rangesliders', function() { expect(+maskMin.getAttribute('width')).toEqual(maskMinWidth); expect(+maskMax.getAttribute('width')).toEqual(maskMaxWidth); - expect(bg.getAttribute('fill')).toBe('rgb(255, 0, 0)'); - expect(bg.getAttribute('stroke')).toBe('rgb(0, 0, 255)'); + expect(bg.style.fill).toBe('rgb(255, 0, 0)'); + expect(bg.style.stroke).toBe('rgb(0, 0, 255)'); expect(bg.getAttribute('stroke-width')).toBe('3'); }) .then(done, done.fail); From a529f1b6deb0bc5640a868a30af44e2ae10fbaae Mon Sep 17 00:00:00 2001 From: Mojtaba Samimi Date: Thu, 6 Oct 2022 10:30:41 -0400 Subject: [PATCH 5/5] revisit violin stroke style --- src/traces/violin/hover.js | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/src/traces/violin/hover.js b/src/traces/violin/hover.js index 5832694c53b..f93945d6756 100644 --- a/src/traces/violin/hover.js +++ b/src/traces/violin/hover.js @@ -1,7 +1,5 @@ 'use strict'; -var tinycolor = require('tinycolor2'); - var Color = require('../../components/color'); var Lib = require('../../lib'); var Axes = require('../../plots/cartesian/axes'); @@ -78,15 +76,7 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode, opts) { closeData.push(kdePointData); - var strokeC = pointData.color; - var strokeColor = tinycolor(strokeC); - var strokeAlpha = strokeColor.getAlpha(); - var strokeRGB = Color.tinyRGB(strokeColor); - - violinLineAttrs = { - stroke: strokeRGB, - 'stroke-opacity': strokeAlpha - }; + violinLineAttrs = {}; violinLineAttrs[pLetter + '1'] = Lib.constrain(paOffset + pOnPath[0], paOffset, paOffset + paLength); violinLineAttrs[pLetter + '2'] = Lib.constrain(paOffset + pOnPath[1], paOffset, paOffset + paLength); violinLineAttrs[vLetter + '1'] = violinLineAttrs[vLetter + '2'] = vAxis._offset + vValPx; @@ -109,7 +99,7 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode, opts) { .classed('violinline-' + trace.uid, true) .attr('stroke-width', 1.5); violinLine.exit().remove(); - violinLine.attr(violinLineAttrs); + violinLine.attr(violinLineAttrs).call(Color.stroke, pointData.color); // same combine logic as box hoverPoints if(hovermode === 'closest') {