Skip to content

Vanishing titles #1351

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Feb 3, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions src/components/titles/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {};
Expand Down Expand Up @@ -192,21 +193,21 @@ 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()
.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);
});
}

if(gd._context.editable) {
if(!txt) setPlaceholder();
else el.on('.opacity', null);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fix is just this line. All the rest is cleanup and testing :)


el.call(svgTextUtils.makeEditable)
.on('edit', function(text) {
Expand Down
18 changes: 18 additions & 0 deletions src/constants/interactions.js
Original file line number Diff line number Diff line change
@@ -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
};
123 changes: 123 additions & 0 deletions test/jasmine/tests/titles_test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nicely done. 🎉

If you're up for a challenge, you could try adding a test for title edits like I did here for legend/trace name edits.

}

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);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@etpinard nice editing tests in finance! And good call to include a test like that here too. The only thing I did differently is this: put resolve in a short setTimeout instead of manually removing the node. Not sure why makeEditable uses a duration-0 transition rather than just removing the element synchronously... but this seems to play well with it.

Copy link
Contributor

@etpinard etpinard Feb 2, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good eyes on that 0-second transition 🔬

});

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() {
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 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([
checkTitle('x', 'XXX', 1, 1),
checkTitle('y', 'YYY', 1, 1),
checkTitle('g', 'TTT', 1, 1)
]);
})
.then(done);
});

});