Merge pull request #8069 from Yahiro025/fix/quiver-discrete-marker-color-8026

fix(quiver): honor discrete marker.color arrays

Author: camdecoster

draftlogs/8069_fix.md

@@ -0,0 +1 @@
+ - Fix quiver arrow colors for a discrete `marker.color` array [[#8069](https://github.com/plotly/plotly.js/pull/8069)], with thanks to @Yahiro025 for the contribution!

src/traces/quiver/plot.js

@@ -4,7 +4,7 @@ var d3 = require('@plotly/d3');
 
 var Lib = require('../../lib');
 var Drawing = require('../../components/drawing');
-var colorscaleStroke = require('./style').colorscaleStroke;
+const { strokeArrowColors } = require('./style');
 
 // Fixed arrowhead wedge angle (radians). Arrow direction is fully
 // determined by u,v (see angPx below); this constant only controls the
@@ -182,12 +182,12 @@ function plotOne(gd, idx, plotinfo, cdscatter, cdscatterAll, element, transition
     // Apply styling using Plotly's standard styling system
     var marker = trace.marker || {};
     var markerLine = marker.line || {};
-    var lineColor = Lib.isArrayOrTypedArray(marker.color) ? undefined : marker.color;
+    const lineColor = Lib.isArrayOrTypedArray(marker.color) ? undefined : marker.color;
     Drawing.lineGroupStyle(lineSegments, markerLine.width, lineColor, markerLine.dash);
 
-    // If colorscale present, color arrows by marker.color or magnitude |(u,v)|.
+    // Color arrows from a colorscale or from a discrete marker.color array.
     // Shared with style.js so the static render and restyle stay in sync.
-    if(trace._hasColorscale) colorscaleStroke(lineSegments, trace);
+    strokeArrowColors(lineSegments, trace);
 
     // Render text labels at data points
     var textGroup = d3.select(element).selectAll('g.text')

src/traces/quiver/style.js

@@ -5,6 +5,7 @@ var Lib = require('../../lib');
 
 var Drawing = require('../../components/drawing');
 var Colorscale = require('../../components/colorscale');
+const Color = require('../../components/color');
 var DESELECTDIM = require('../../constants/interactions').DESELECTDIM;
 
 // Stroke each arrow path according to the trace colorscale, using marker.color
@@ -33,6 +34,29 @@ function colorscaleStroke(paths, trace) {
     });
 }
 
+// A color-string array is not a colorscale. Apply each string here, because
+// `lineGroupStyle` paints an array as black.
+const strokeArrowColors = (paths, trace) => {
+    if (trace._hasColorscale) {
+        colorscaleStroke(paths, trace);
+        return;
+    }
+
+    const markerColor = (trace.marker || {}).color;
+    if (!Lib.isArrayOrTypedArray(markerColor)) return;
+
+    paths.style('stroke', function (cdi) {
+        if (markerColor.length > cdi.i) {
+            const color = markerColor[cdi.i];
+            if (Color.isValid(color)) {
+                cdi.mcc = color;
+                return color;
+            }
+        }
+        return this.style.stroke;
+    });
+};
+
 function style(gd) {
     var s = d3.select(gd).selectAll('g.trace.quiver');
     s.each(function(d) {
@@ -49,7 +73,6 @@ function styleArrows(gd, cd, sel) {
     var marker = trace.marker || {};
     var markerLine = marker.line || {};
     var lineColor = Lib.isArrayOrTypedArray(marker.color) ? undefined : marker.color;
-    var hasColorscale = trace._hasColorscale;
 
     if(!sel) return;
 
@@ -78,7 +101,7 @@ function styleArrows(gd, cd, sel) {
                 // color, so we keep each arrow's color and dim unselected arrows
                 // via stroke-opacity instead.
                 Drawing.lineGroupStyle(path, lineWidth, lineColor, markerLine.dash);
-                if(hasColorscale) colorscaleStroke(path, trace);
+                strokeArrowColors(path, trace);
                 path.style('stroke-opacity', dim ? DESELECTDIM : 1);
             }
         });
@@ -87,14 +110,15 @@ function styleArrows(gd, cd, sel) {
     } else {
         var paths = sel.selectAll('path.js-line');
         paths.call(Drawing.lineGroupStyle, markerLine.width, lineColor, markerLine.dash);
-        if(hasColorscale) colorscaleStroke(paths, trace);
+        strokeArrowColors(paths, trace);
         paths.style('stroke-opacity', 1);
         Drawing.textPointStyle(sel.selectAll('text'), trace, gd);
     }
 }
 
 module.exports = {
-    style: style,
-    styleOnSelect: styleOnSelect,
-    colorscaleStroke: colorscaleStroke
+    style,
+    styleOnSelect,
+    colorscaleStroke,
+    strokeArrowColors
 };

test/jasmine/tests/quiver_test.js

@@ -9,6 +9,7 @@ var mouseEvent = require('../assets/mouse_event');
 
 var customAssertions = require('../assets/custom_assertions');
 var assertHoverLabelContent = customAssertions.assertHoverLabelContent;
+var quiverStyle = require('../../../src/traces/quiver/style');
 
 describe('Test quiver defaults', function() {
     var gd;
@@ -305,6 +306,52 @@ describe('Test quiver interactions', function() {
         expect(window.getComputedStyle(arrow).stroke).toBe('rgb(33, 145, 140)', 'arrow');
     });
 
+    it('should color each arrow from a discrete marker.color array', async () => {
+        const colors = ['rgb(23, 103, 194)', '#FF4136', '#2ECC40'];
+        const expected = ['rgb(23, 103, 194)', 'rgb(255, 65, 54)', 'rgb(46, 204, 64)'];
+
+        await Plotly.newPlot(gd, [{
+            type: 'quiver',
+            x: [1, 2, 3],
+            y: [1, 2, 3],
+            u: [1, 1, 1],
+            v: [0, 1, 0],
+            marker: {
+                color: colors,
+                line: { width: 3 }
+            }
+        }]);
+
+        function arrowStrokes() {
+            const arrows = gd.querySelectorAll('g.trace.quiver path.js-line');
+            expect(arrows.length).toBe(3);
+            const strokes = [];
+            for(let i = 0; i < arrows.length; i++) {
+                strokes.push(window.getComputedStyle(arrows[i]).stroke);
+            }
+            return strokes;
+        }
+
+        expect(arrowStrokes()).toEqual(expected);
+
+        // A style-only edit must keep the per-arrow colors.
+        await Plotly.restyle(gd, 'marker.line.width', 4);
+        expect(arrowStrokes()).toEqual(expected);
+
+        // Selection styling must keep each arrow color and only dim the rest.
+        const cd = gd.calcdata[0];
+        cd[0].trace.selectedpoints = [0, 2];
+        cd[0].selected = 1;
+        cd[2].selected = 1;
+        quiverStyle.styleOnSelect(gd, cd, cd[0].node3);
+
+        expect(arrowStrokes()).toEqual(expected);
+        const arrows = gd.querySelectorAll('g.trace.quiver path.js-line');
+        expect(window.getComputedStyle(arrows[0]).strokeOpacity).toBe('1');
+        expect(window.getComputedStyle(arrows[1]).strokeOpacity).toBe('0.2');
+        expect(window.getComputedStyle(arrows[2]).strokeOpacity).toBe('1');
+    });
+
     it('should render multiple quiver traces', function(done) {
         Plotly.newPlot(gd, [{
             type: 'quiver',
@@ -330,6 +377,12 @@ describe('Test quiver interactions', function() {
         .then(function() {
             var quiverLayers = gd._fullLayout._plots.xy.plot.selectAll('.trace.quiver');
             expect(quiverLayers.size()).toBe(2);
+
+            var arrows = gd.querySelectorAll('g.trace.quiver path.js-line');
+            expect(window.getComputedStyle(arrows[0]).stroke).toBe('rgb(255, 0, 0)');
+            expect(window.getComputedStyle(arrows[1]).stroke).toBe('rgb(255, 0, 0)');
+            expect(window.getComputedStyle(arrows[2]).stroke).toBe('rgb(0, 0, 255)');
+            expect(window.getComputedStyle(arrows[3]).stroke).toBe('rgb(0, 0, 255)');
         })
         .then(done, done.fail);
     });