Skip to content

Commit

Permalink
fixing scattergl and splom symbol numbers bug issue 2507
Browse files Browse the repository at this point in the history
- add image tests with legends and using array for sizes and symbols
  • Loading branch information
archmoj committed Jan 6, 2020
1 parent 9a772c6 commit 6468acc
Show file tree
Hide file tree
Showing 8 changed files with 104 additions and 6 deletions.
7 changes: 4 additions & 3 deletions src/traces/scattergl/convert.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ var formatColor = require('../../lib/gl_format_color').formatColor;
var subTypes = require('../scatter/subtypes');
var makeBubbleSizeFn = require('../scatter/make_bubble_size_func');

var helpers = require('./helpers');
var constants = require('./constants');
var DESELECTDIM = require('../../constants/interactions').DESELECTDIM;

Expand Down Expand Up @@ -225,7 +226,7 @@ function convertMarkerStyle(trace) {
var multiLineWidth = Lib.isArrayOrTypedArray(optsIn.line.width);

var isOpen;
if(!multiSymbol) isOpen = constants.OPEN_RE.test(optsIn.symbol);
if(!multiSymbol) isOpen = helpers.isOpenSymbol(optsIn.symbol);

// prepare colors
if(multiSymbol || multiColor || multiLineColor || multiOpacity) {
Expand Down Expand Up @@ -256,7 +257,7 @@ function convertMarkerStyle(trace) {
for(i = 0; i < count; i++) {
if(multiSymbol) {
var symbol = optsIn.symbol[i];
isOpen = constants.OPEN_RE.test(symbol);
isOpen = helpers.isOpenSymbol(symbol);
}
if(isOpen) {
borderColors[i] = colors[i].slice();
Expand Down Expand Up @@ -401,7 +402,7 @@ function getSymbolSdf(symbol) {
var symbolNoDot = !!Drawing.symbolNoDot[symbolNumber % 100];
var symbolNoFill = !!Drawing.symbolNoFill[symbolNumber % 100];

var isDot = constants.DOT_RE.test(symbol);
var isDot = helpers.isDotSymbol(symbol);

// get symbol sdf from cache or generate it
if(SYMBOL_SDF[symbol]) return SYMBOL_SDF[symbol];
Expand Down
3 changes: 2 additions & 1 deletion src/traces/scattergl/defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
var Lib = require('../../lib');
var Registry = require('../../registry');

var helpers = require('./helpers');
var attributes = require('./attributes');
var constants = require('../scatter/constants');
var subTypes = require('../scatter/subtypes');
Expand All @@ -25,7 +26,7 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
return Lib.coerce(traceIn, traceOut, attributes, attr, dflt);
}

var isOpen = traceIn.marker ? /-open/.test(traceIn.marker.symbol) : false;
var isOpen = traceIn.marker ? helpers.isOpenSymbol(traceIn.marker.symbol) : false;
var isBubble = subTypes.isBubble(traceIn);

var len = handleXYDefaults(traceIn, traceOut, layout, coerce);
Expand Down
23 changes: 23 additions & 0 deletions src/traces/scattergl/helpers.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
/**
* Copyright 2012-2020, Plotly, Inc.
* All rights reserved.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

'use strict';

var constants = require('./constants');

exports.isOpenSymbol = function(symbol) {
return (typeof symbol === 'string') ?
constants.OPEN_RE.test(symbol) :
symbol % 200 > 100;
};

exports.isDotSymbol = function(symbol) {
return (typeof symbol === 'string') ?
constants.DOT_RE.test(symbol) :
symbol > 200;
};
4 changes: 2 additions & 2 deletions src/traces/splom/defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ var attributes = require('./attributes');
var subTypes = require('../scatter/subtypes');
var handleMarkerDefaults = require('../scatter/marker_defaults');
var mergeLength = require('../parcoords/merge_length');
var OPEN_RE = /-open/;
var isOpenSymbol = require('../scattergl/helpers').isOpenSymbol;

module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout) {
function coerce(attr, dflt) {
Expand Down Expand Up @@ -44,7 +44,7 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout

handleMarkerDefaults(traceIn, traceOut, defaultColor, layout, coerce);

var isOpen = OPEN_RE.test(traceOut.marker.symbol);
var isOpen = isOpenSymbol(traceOut.marker.symbol);
var isBubble = subTypes.isBubble(traceOut);
coerce('marker.line.width', isOpen || isBubble ? 1 : 0);

Expand Down
Binary file added test/image/baselines/gl2d_symbol_numbers.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added test/image/baselines/splom_symbol_numbers.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
32 changes: 32 additions & 0 deletions test/image/mocks/gl2d_symbol_numbers.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
{
"data": [
{
"y": [
0,
0,
0
],
"type": "scattergl",
"marker": {
"size": 24,
"symbol": "diamond-open-dot"
}
},
{
"y": [
1,
1,
1
],
"type": "scattergl",
"marker": {
"size": 24,
"symbol": 302
}
}
],
"layout": {
"width": 600,
"height": 400
}
}
41 changes: 41 additions & 0 deletions test/image/mocks/splom_symbol_numbers.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
{
"data": [
{
"type": "splom",
"marker": {
"size": [
12,
16,
24,
32
],
"symbol": [
1,
102,
203,
304
]
},
"dimensions": [
{
"values": [
0,
1,
2,
3
],
"label": "A"
},
{
"values": [
0,
2,
5,
6
],
"label": "B"
}
]
}
]
}

0 comments on commit 6468acc

Please sign in to comment.