From e526042bcacee333e6025d336cc2b0d462df177f Mon Sep 17 00:00:00 2001 From: alexcjohnson Date: Thu, 2 Feb 2017 12:59:25 -0500 Subject: [PATCH 1/4] fix vanishing titles issue --- src/components/titles/index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/titles/index.js b/src/components/titles/index.js index c4b18c7505e..8029d35a289 100644 --- a/src/components/titles/index.js +++ b/src/components/titles/index.js @@ -192,8 +192,7 @@ Titles.draw = function(gd, titleClass, options) { opacity = 0; isplaceholder = true; txt = placeholderText; - fullLayout._infolayer.select('.' + titleClass) - .attr({'data-unformatted': txt}) + el.attr({'data-unformatted': txt}) .text(txt) .on('mouseover.opacity', function() { d3.select(this).transition() @@ -207,6 +206,7 @@ Titles.draw = function(gd, titleClass, options) { if(gd._context.editable) { if(!txt) setPlaceholder(); + else el.on('.opacity', null); el.call(svgTextUtils.makeEditable) .on('edit', function(text) { From f0115255ced9e18a8b93c805b8bb5b335636244d Mon Sep 17 00:00:00 2001 From: alexcjohnson Date: Thu, 2 Feb 2017 14:04:57 -0500 Subject: [PATCH 2/4] test title hover effects --- src/components/titles/index.js | 5 +- src/constants/interactions.js | 18 ++++++ test/jasmine/tests/titles_test.js | 103 ++++++++++++++++++++++++++++++ 3 files changed, 124 insertions(+), 2 deletions(-) create mode 100644 src/constants/interactions.js create mode 100644 test/jasmine/tests/titles_test.js diff --git a/src/components/titles/index.js b/src/components/titles/index.js index 8029d35a289..6fa0e64c6af 100644 --- a/src/components/titles/index.js +++ b/src/components/titles/index.js @@ -18,6 +18,7 @@ var Lib = require('../../lib'); var Drawing = require('../drawing'); var Color = require('../color'); var svgTextUtils = require('../../lib/svg_text_utils'); +var interactConstants = require('../../constants/interactions'); var Titles = module.exports = {}; @@ -196,11 +197,11 @@ Titles.draw = function(gd, titleClass, options) { .text(txt) .on('mouseover.opacity', function() { d3.select(this).transition() - .duration(100).style('opacity', 1); + .duration(interactConstants.SHOW_PLACEHOLDER).style('opacity', 1); }) .on('mouseout.opacity', function() { d3.select(this).transition() - .duration(1000).style('opacity', 0); + .duration(interactConstants.HIDE_PLACEHOLDER).style('opacity', 0); }); } diff --git a/src/constants/interactions.js b/src/constants/interactions.js new file mode 100644 index 00000000000..132e9ca4c37 --- /dev/null +++ b/src/constants/interactions.js @@ -0,0 +1,18 @@ +/** +* Copyright 2012-2017, 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'; + + +module.exports = { + /** + * Timing information for interactive elements + */ + SHOW_PLACEHOLDER: 100, + HIDE_PLACEHOLDER: 1000 +}; diff --git a/test/jasmine/tests/titles_test.js b/test/jasmine/tests/titles_test.js new file mode 100644 index 00000000000..211827a9aed --- /dev/null +++ b/test/jasmine/tests/titles_test.js @@ -0,0 +1,103 @@ +var d3 = require('d3'); + +var Plotly = require('@lib/index'); +var interactConstants = require('@src/constants/interactions'); + +var createGraphDiv = require('../assets/create_graph_div'); +var destroyGraphDiv = require('../assets/destroy_graph_div'); +var mouseEvent = require('../assets/mouse_event'); + +describe('editable titles', function() { + 'use strict'; + + var data = [{x: [1, 2, 3], y: [1, 2, 3]}]; + + var gd; + + afterEach(destroyGraphDiv); + + beforeEach(function() { + gd = createGraphDiv(); + }); + + function checkTitle(letter, text, opacityOut, opacityIn) { + var titleEl = d3.select('.' + letter + 'title'); + expect(titleEl.text()).toBe(text); + expect(+titleEl.style('opacity')).toBe(opacityOut); + + var bb = titleEl.node().getBoundingClientRect(), + xCenter = (bb.left + bb.right) / 2, + yCenter = (bb.top + bb.bottom) / 2, + done, + promise = new Promise(function(resolve) { done = resolve; }); + + mouseEvent('mouseover', xCenter, yCenter); + setTimeout(function() { + expect(+titleEl.style('opacity')).toBe(opacityIn); + + mouseEvent('mouseout', xCenter, yCenter); + setTimeout(function() { + expect(+titleEl.style('opacity')).toBe(opacityOut); + done(); + }, interactConstants.HIDE_PLACEHOLDER + 50); + }, interactConstants.SHOW_PLACEHOLDER + 50); + + return promise; + } + + it('shows default titles semi-opaque with no hover effects', function(done) { + Plotly.plot(gd, data, {}, {editable: true}) + .then(function() { + return Promise.all([ + // Check all three titles in parallel. This only works because + // we're using synthetic events, not a real mouse. It's a big + // win though because the test takes 1.2 seconds with the + // animations... + checkTitle('x', 'Click to enter X axis title', 0.2, 0.2), + checkTitle('y', 'Click to enter Y axis title', 0.2, 0.2), + checkTitle('g', 'Click to enter Plot title', 0.2, 0.2) + ]); + }) + .then(done); + }); + + it('has hover effects for blank titles', function(done) { + Plotly.plot(gd, data, { + xaxis: {title: ''}, + yaxis: {title: ''}, + title: '' + }, {editable: true}) + .then(function() { + return Promise.all([ + checkTitle('x', 'Click to enter X axis title', 0, 1), + checkTitle('y', 'Click to enter Y axis title', 0, 1), + checkTitle('g', 'Click to enter Plot title', 0, 1) + ]); + }) + .then(done); + }); + + it('has no hover effects for titles that used to be blank', function(done) { + Plotly.plot(gd, data, { + xaxis: {title: ''}, + yaxis: {title: ''}, + title: '' + }, {editable: true}) + .then(function() { + return Plotly.relayout(gd, { + 'xaxis.title': 'XXX', + 'yaxis.title': 'YYY', + 'title': 'TTT' + }); + }) + .then(function() { + return Promise.all([ + checkTitle('x', 'XXX', 1, 1), + checkTitle('y', 'YYY', 1, 1), + checkTitle('g', 'TTT', 1, 1) + ]); + }) + .then(done); + }); + +}); From b8a5e896d1b31ee949a0664119bb075a8e544a6e Mon Sep 17 00:00:00 2001 From: alexcjohnson Date: Thu, 2 Feb 2017 15:48:49 -0500 Subject: [PATCH 3/4] update titles test to include GUI editing --- test/jasmine/tests/titles_test.js | 31 ++++++++++++++++++++++++++----- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/test/jasmine/tests/titles_test.js b/test/jasmine/tests/titles_test.js index 211827a9aed..72be2ed2819 100644 --- a/test/jasmine/tests/titles_test.js +++ b/test/jasmine/tests/titles_test.js @@ -6,6 +6,7 @@ var interactConstants = require('@src/constants/interactions'); var createGraphDiv = require('../assets/create_graph_div'); var destroyGraphDiv = require('../assets/destroy_graph_div'); var mouseEvent = require('../assets/mouse_event'); +var Plots = require('@src/plots/plots'); describe('editable titles', function() { 'use strict'; @@ -45,6 +46,24 @@ describe('editable titles', function() { return promise; } + function editTitle(letter, attr, text) { + return new Promise(function(resolve) { + gd.once('plotly_relayout', function(eventData) { + expect(eventData[attr]).toEqual(text, [letter, attr, eventData]); + setTimeout(resolve, 10); + }); + + var textNode = document.querySelector('.' + letter + 'title'); + textNode.dispatchEvent(new window.MouseEvent('click')); + + var editNode = document.querySelector('.plugin-editable.editable'); + editNode.dispatchEvent(new window.FocusEvent('focus')); + editNode.textContent = text; + editNode.dispatchEvent(new window.FocusEvent('focus')); + editNode.dispatchEvent(new window.FocusEvent('blur')); + }); + } + it('shows default titles semi-opaque with no hover effects', function(done) { Plotly.plot(gd, data, {}, {editable: true}) .then(function() { @@ -84,11 +103,13 @@ describe('editable titles', function() { title: '' }, {editable: true}) .then(function() { - return Plotly.relayout(gd, { - 'xaxis.title': 'XXX', - 'yaxis.title': 'YYY', - 'title': 'TTT' - }); + return editTitle('x', 'xaxis.title', 'XXX'); + }) + .then(function() { + return editTitle('y', 'yaxis.title', 'YYY'); + }) + .then(function() { + return editTitle('g', 'title', 'TTT'); }) .then(function() { return Promise.all([ From 8c72936ba20d0b2bc502508acf6e4b1aefcc83d2 Mon Sep 17 00:00:00 2001 From: alexcjohnson Date: Thu, 2 Feb 2017 16:06:53 -0500 Subject: [PATCH 4/4] :see_no_evil: eslint --- test/jasmine/tests/titles_test.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/jasmine/tests/titles_test.js b/test/jasmine/tests/titles_test.js index 72be2ed2819..7e6b58e64d5 100644 --- a/test/jasmine/tests/titles_test.js +++ b/test/jasmine/tests/titles_test.js @@ -6,7 +6,6 @@ var interactConstants = require('@src/constants/interactions'); var createGraphDiv = require('../assets/create_graph_div'); var destroyGraphDiv = require('../assets/destroy_graph_div'); var mouseEvent = require('../assets/mouse_event'); -var Plots = require('@src/plots/plots'); describe('editable titles', function() { 'use strict';