Merge branch 'master' into gl3d-annotations

Author: etpinard

src/components/drawing/index.js

@@ -156,6 +156,16 @@ drawing.dashStyle = function(dash, lineWidth) {
     return dash;
 };
 
+// Same as fillGroupStyle, except in this case the selection may be a transition
+drawing.singleFillStyle = function(sel) {
+    var node = d3.select(sel.node());
+    var data = node.data();
+    var fillcolor = (((data[0] || [])[0] || {}).trace || {}).fillcolor;
+    if(fillcolor) {
+        sel.call(Color.fill, fillcolor);
+    }
+};
+
 drawing.fillGroupStyle = function(s) {
     s.style('stroke-width', 0)
     .each(function(d) {

src/lib/svg_text_utils.js

@@ -282,11 +282,36 @@ var SPLIT_TAGS = /(<[^<>]*>)/;
 
 var ONE_TAG = /<(\/?)([^ >]*)(\s+(.*))?>/i;
 
-// Style and href: pull them out of either single or double quotes.
-// Because we hack in other attributes with style (sub & sup), drop any trailing
-// semicolon in user-supplied styles so we can consistently append the tag-dependent style
+/*
+ * style and href: pull them out of either single or double quotes. Also
+ * - target: (_blank|_self|_parent|_top|framename)
+ *     note that you can't use target to get a popup but if you use popup,
+ *     a `framename` will be passed along as the name of the popup window.
+ *     per the spec, cannot contain whitespace.
+ *     for backward compatibility we default to '_blank'
+ * - popup: a custom one for us to enable popup (new window) links. String
+ *     for window.open -> strWindowFeatures, like 'menubar=yes,width=500,height=550'
+ *     note that at least in Chrome, you need to give at least one property
+ *     in this string or the page will open in a new tab anyway. We follow this
+ *     convention and will not make a popup if this string is empty.
+ *     per the spec, cannot contain whitespace.
+ *
+ * Because we hack in other attributes with style (sub & sup), drop any trailing
+ * semicolon in user-supplied styles so we can consistently append the tag-dependent style
+ */
 var STYLEMATCH = /(^|[\s"'])style\s*=\s*("([^"]*);?"|'([^']*);?')/i;
 var HREFMATCH = /(^|[\s"'])href\s*=\s*("([^"]*)"|'([^']*)')/i;
+var TARGETMATCH = /(^|[\s"'])target\s*=\s*("([^"\s]*)"|'([^'\s]*)')/i;
+var POPUPMATCH = /(^|[\s"'])popup\s*=\s*("([^"\s]*)"|'([^'\s]*)')/i;
+
+// dedicated matcher for these quoted regexes, that can return their results
+// in two different places
+function getQuotedMatch(_str, re) {
+    if(!_str) return null;
+    var match = _str.match(re);
+    return match && (match[3] || match[4]);
+}
+
 
 var COLORMATCH = /(^|;)\s*color:/;
 
@@ -297,14 +322,14 @@ exports.plainText = function(_str) {
 };
 
 function replaceFromMapObject(_str, list) {
-    var out = _str || '';
+    if(!_str) return '';
 
     for(var i = 0; i < list.length; i++) {
         var item = list[i];
-        out = out.replace(item.regExp, item.sub);
+        _str = _str.replace(item.regExp, item.sub);
     }
 
-    return out;
+    return _str;
 }
 
 function convertEntities(_str) {
@@ -354,8 +379,7 @@ function convertToSVG(_str) {
 
                 // anchor is the only tag that doesn't turn into a tspan
                 if(tag === 'a') {
-                    var hrefMatch = extra && extra.match(HREFMATCH);
-                    var href = hrefMatch && (hrefMatch[3] || hrefMatch[4]);
+                    var href = getQuotedMatch(extra, HREFMATCH);
 
                     out = '<a';
 
@@ -364,7 +388,34 @@ function convertToSVG(_str) {
                         var dummyAnchor = document.createElement('a');
                         dummyAnchor.href = href;
                         if(PROTOCOLS.indexOf(dummyAnchor.protocol) !== -1) {
-                            out += ' xlink:show="new" xlink:href="' + encodeForHTML(href) + '"';
+                            href = encodeForHTML(href);
+
+                            // look for target and popup specs
+                            var target = encodeForHTML(getQuotedMatch(extra, TARGETMATCH)) || '_blank';
+                            var popup = encodeForHTML(getQuotedMatch(extra, POPUPMATCH));
+
+                            /*
+                             * xlink:show is for backward compatibility only,
+                             * newer browsers allow target just like html links.
+                             */
+                            var xlinkShow = (target === '_blank' || target.charAt(0) !== '_') ?
+                                'new' : 'replace';
+
+                            out += ' xlink:show="' + xlinkShow + '" target="' + target +
+                                '" xlink:href="' + href + '"';
+
+                            if(popup) {
+                                /*
+                                 * Add the window.open call to create a popup
+                                 * Even when popup is specified, we leave the original
+                                 * href and target in place in case javascript is
+                                 * unavailable in this context (like downloaded svg)
+                                 * and for accessibility and so users can see where
+                                 * the link will lead.
+                                 */
+                                out += ' onclick="window.open(\'' + href + '\',\'' + target +
+                                    '\',\'' + popup + '\');return false;"';
+                            }
                         }
                     }
                 }
@@ -380,8 +431,7 @@ function convertToSVG(_str) {
                 // now add style, from both the tag name and any extra css
                 // Most of the svg css that users will care about is just like html,
                 // but font color is different (uses fill). Let our users ignore this.
-                var cssMatch = extra && extra.match(STYLEMATCH);
-                var css = cssMatch && (cssMatch[3] || cssMatch[4]);
+                var css = getQuotedMatch(extra, STYLEMATCH);
                 if(css) {
                     css = encodeForHTML(css.replace(COLORMATCH, '$1 fill:'));
                     if(tagStyle) css += ';' + tagStyle;

src/traces/scatter/plot.js

@@ -340,10 +340,12 @@ function plotOne(gd, idx, plotinfo, cdscatter, cdscatterAll, element, transition
                     // For the sake of animations, wrap the points around so that
                     // the points on the axes are the first two points. Otherwise
                     // animations get a little crazy if the number of points changes.
-                    transition(ownFillEl3).attr('d', 'M' + pt1 + 'L' + pt0 + 'L' + fullpath.substr(1));
+                    transition(ownFillEl3).attr('d', 'M' + pt1 + 'L' + pt0 + 'L' + fullpath.substr(1))
+                        .call(Drawing.singleFillStyle);
                 } else {
                     // fill to self: just join the path to itself
-                    transition(ownFillEl3).attr('d', fullpath + 'Z');
+                    transition(ownFillEl3).attr('d', fullpath + 'Z')
+                        .call(Drawing.singleFillStyle);
                 }
             }
         }
@@ -354,15 +356,17 @@ function plotOne(gd, idx, plotinfo, cdscatter, cdscatterAll, element, transition
                 // contours, we just add the two paths closed on themselves.
                 // This makes strange results if one path is *not* entirely
                 // inside the other, but then that is a strange usage.
-                transition(tonext).attr('d', fullpath + 'Z' + prevRevpath + 'Z');
+                transition(tonext).attr('d', fullpath + 'Z' + prevRevpath + 'Z')
+                    .call(Drawing.singleFillStyle);
             }
             else {
                 // tonextx/y: for now just connect endpoints with lines. This is
                 // the correct behavior if the endpoints are at the same value of
                 // y/x, but if they *aren't*, we should ideally do more complicated
                 // things depending on whether the new endpoint projects onto the
                 // existing curve or off the end of it
-                transition(tonext).attr('d', fullpath + 'L' + prevRevpath.substr(1) + 'Z');
+                transition(tonext).attr('d', fullpath + 'L' + prevRevpath.substr(1) + 'Z')
+                    .call(Drawing.singleFillStyle);
             }
             trace._polygons = trace._polygons.concat(prevPolygons);
         }

test/jasmine/tests/scatter_test.js

@@ -575,6 +575,27 @@ describe('end-to-end scatter tests', function() {
         .catch(fail)
         .then(done);
     });
+
+    it('animates fillcolor', function(done) {
+        function fill() {
+            return d3.selectAll('.js-fill').node().style.fill;
+        }
+
+        Plotly.plot(gd, [{
+            x: [1, 2, 3, 4, 5, 6, 7],
+            y: [2, 3, 4, 5, 6, 7, 8],
+            fill: 'tozeroy',
+            fillcolor: 'rgb(255, 0, 0)',
+        }]).then(function() {
+            expect(fill()).toEqual('rgb(255, 0, 0)');
+            return Plotly.animate(gd,
+                [{data: [{fillcolor: 'rgb(0, 0, 255)'}]}],
+                {frame: {duration: 0, redraw: false}}
+            );
+        }).then(function() {
+            expect(fill()).toEqual('rgb(0, 0, 255)');
+        }).catch(fail).then(done);
+    });
 });
 
 describe('scatter hoverPoints', function() {

test/jasmine/tests/svg_text_utils_test.js

@@ -18,38 +18,49 @@ describe('svg+text utils', function() {
                 .attr('transform', 'translate(50,50)');
         }
 
-        function assertAnchorLink(node, href) {
+        function assertAnchorLink(node, href, target, show, msg) {
             var a = node.select('a');
 
-            expect(a.attr('xlink:href')).toBe(href);
-            expect(a.attr('xlink:show')).toBe(href === null ? null : 'new');
+            if(target === undefined) target = href === null ? null : '_blank';
+            if(show === undefined) show = href === null ? null : 'new';
+
+            expect(a.attr('xlink:href')).toBe(href, msg);
+            expect(a.attr('target')).toBe(target, msg);
+            expect(a.attr('xlink:show')).toBe(show, msg);
         }
 
-        function assertTspanStyle(node, style) {
+        function assertTspanStyle(node, style, msg) {
             var tspan = node.select('tspan');
-            expect(tspan.attr('style')).toBe(style);
+            expect(tspan.attr('style')).toBe(style, msg);
         }
 
-        function assertAnchorAttrs(node, style) {
+        function assertAnchorAttrs(node, expectedAttrs, msg) {
             var a = node.select('a');
 
-            var WHITE_LIST = ['xlink:href', 'xlink:show', 'style'],
+            if(!expectedAttrs) expectedAttrs = {};
+
+            var WHITE_LIST = ['xlink:href', 'xlink:show', 'style', 'target', 'onclick'],
                 attrs = listAttributes(a.node());
 
             // check that no other attribute are found in anchor,
             // which can be lead to XSS attacks.
 
-            var hasWrongAttr = attrs.some(function(attr) {
-                return WHITE_LIST.indexOf(attr) === -1;
+            var wrongAttrs = [];
+            attrs.forEach(function(attr) {
+                if(WHITE_LIST.indexOf(attr) === -1) wrongAttrs.push(attr);
             });
 
-            expect(hasWrongAttr).toBe(false);
+            expect(wrongAttrs).toEqual([], msg);
 
+            var style = expectedAttrs.style || '';
             var fullStyle = style || '';
             if(style) fullStyle += ';';
             fullStyle += 'cursor:pointer';
 
-            expect(a.attr('style')).toBe(fullStyle);
+            expect(a.attr('style')).toBe(fullStyle, msg);
+
+            expect(a.attr('onclick')).toBe(expectedAttrs.onclick || null, msg);
+
         }
 
         function listAttributes(node) {
@@ -137,7 +148,7 @@ describe('svg+text utils', function() {
                 var node = mockTextSVGElement(textCase);
 
                 expect(node.text()).toEqual('Subtitle');
-                assertAnchorAttrs(node, 'font-size:300px');
+                assertAnchorAttrs(node, {style: 'font-size:300px'});
                 assertAnchorLink(node, 'XSS');
             });
         });
@@ -157,11 +168,31 @@ describe('svg+text utils', function() {
                 var node = mockTextSVGElement(textCase);
 
                 expect(node.text()).toEqual('z');
-                assertAnchorAttrs(node, 'y');
+                assertAnchorAttrs(node, {style: 'y'});
                 assertAnchorLink(node, 'x');
             });
         });
 
+        it('accepts `target` with links and tries to translate it to `xlink:show`', function() {
+            var specs = [
+                {target: '_blank', show: 'new'},
+                {target: '_self', show: 'replace'},
+                {target: '_parent', show: 'replace'},
+                {target: '_top', show: 'replace'},
+                {target: 'some_frame_name', show: 'new'}
+            ];
+            specs.forEach(function(spec) {
+                var node = mockTextSVGElement('<a href="x" target="' + spec.target + '">link</a>');
+                assertAnchorLink(node, 'x', spec.target, spec.show, spec.target);
+            });
+        });
+
+        it('attaches onclick if popup is specified', function() {
+            var node = mockTextSVGElement('<a href="x" target="fred" popup="width=500,height=400">link</a>');
+            assertAnchorLink(node, 'x', 'fred', 'new');
+            assertAnchorAttrs(node, {onclick: 'window.open(\'x\',\'fred\',\'width=500,height=400\');return false;'});
+        });
+
         it('keeps query parameters in href', function() {
             var textCases = [
                 '<a href="https://abc.com/myFeature.jsp?name=abc&pwd=def">abc.com?shared-key</a>',
@@ -171,9 +202,9 @@ describe('svg+text utils', function() {
             textCases.forEach(function(textCase) {
                 var node = mockTextSVGElement(textCase);
 
-                assertAnchorAttrs(node);
-                expect(node.text()).toEqual('abc.com?shared-key');
-                assertAnchorLink(node, 'https://abc.com/myFeature.jsp?name=abc&pwd=def');
+                assertAnchorAttrs(node, {}, textCase);
+                expect(node.text()).toEqual('abc.com?shared-key', textCase);
+                assertAnchorLink(node, 'https://abc.com/myFeature.jsp?name=abc&pwd=def', undefined, undefined, textCase);
             });
         });
 

test/jasmine/tests/transform_sort_test.js

@@ -258,7 +258,7 @@ describe('Test sort transform interactions:', function() {
         .then(done);
     });
 
-    it('does not preserve hover/click `pointNumber` value', function(done) {
+    it('does not preserve event data `pointNumber` value', function(done) {
         var gd = createGraphDiv();
 
         function getPxPos(gd, id) {
@@ -273,32 +273,18 @@ describe('Test sort transform interactions:', function() {
         }
 
         function hover(gd, id) {
-            return new Promise(function(resolve) {
+            return new Promise(function(resolve, reject) {
                 gd.once('plotly_hover', function(eventData) {
+                    delete gd._lastHoverTime;
                     resolve(eventData);
                 });
 
                 var pos = getPxPos(gd, id);
                 mouseEvent('mousemove', pos[0], pos[1]);
-            });
-        }
-
-        function click(gd, id) {
-            return new Promise(function(resolve) {
-                gd.once('plotly_click', function(eventData) {
-                    resolve(eventData);
-                });
-
-                var pos = getPxPos(gd, id);
-                mouseEvent('mousemove', pos[0], pos[1]);
-                mouseEvent('mousedown', pos[0], pos[1]);
-                mouseEvent('mouseup', pos[0], pos[1]);
-            });
-        }
 
-        function wait() {
-            return new Promise(function(resolve) {
-                setTimeout(resolve, 100);
+                setTimeout(function() {
+                    reject('plotly_hover did not get called!');
+                }, 100);
             });
         }
 
@@ -334,21 +320,18 @@ describe('Test sort transform interactions:', function() {
         .then(function(eventData) {
             assertPt(eventData, 0, 1, 3, 'D');
         })
-        .then(wait)
-        .then(function() { return click(gd, 'G'); })
+        .then(function() { return hover(gd, 'G'); })
         .then(function(eventData) {
             assertPt(eventData, 1, 1, 6, 'G');
         })
-        .then(wait)
         .then(function() {
             return Plotly.restyle(gd, 'transforms[0].enabled', true);
         })
         .then(function() { return hover(gd, 'D'); })
         .then(function(eventData) {
             assertPt(eventData, 0, 1, 1, 'D');
         })
-        .then(wait)
-        .then(function() { return click(gd, 'G'); })
+        .then(function() { return hover(gd, 'G'); })
         .then(function(eventData) {
             assertPt(eventData, 1, 1, 5, 'G');
         })
@@ -359,8 +342,7 @@ describe('Test sort transform interactions:', function() {
         .then(function(eventData) {
             assertPt(eventData, 0, 1, 1, 'D');
         })
-        .then(wait)
-        .then(function() { return click(gd, 'G'); })
+        .then(function() { return hover(gd, 'G'); })
         .then(function(eventData) {
             assertPt(eventData, 1, 1, 5, 'G');
         })