From 66870344d99a1ab507687ae99be76febc593d1b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=B6stl?= Date: Tue, 30 Oct 2018 10:34:47 +0100 Subject: [PATCH 01/49] Add test of current plot title alignment [882] --- test/jasmine/tests/titles_test.js | 36 +++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/test/jasmine/tests/titles_test.js b/test/jasmine/tests/titles_test.js index 0e81784bb29..e8fbe93b71b 100644 --- a/test/jasmine/tests/titles_test.js +++ b/test/jasmine/tests/titles_test.js @@ -7,6 +7,42 @@ var createGraphDiv = require('../assets/create_graph_div'); var destroyGraphDiv = require('../assets/destroy_graph_div'); var mouseEvent = require('../assets/mouse_event'); + +describe('Plot title', function() { + 'use strict'; + + var data = [{x: [1, 2, 3], y: [1, 2, 3]}]; + var layout = {title: 'Plotly line chart'}; + var gd; + + beforeEach(function() { + gd = createGraphDiv(); + }); + + afterEach(destroyGraphDiv); + + it('is centered horizontally and vertically above the plot by default', function() { + Plotly.plot(gd, data, layout); + + var containerBB = gd.getBoundingClientRect(); + + expect(titleX()).toBe(containerBB.width / 2); + expect(titleY()).toBe(gd._fullLayout.margin.t / 2); + }); + + function titleX() { + return Number.parseFloat(titleSel().attr('x')); + } + + function titleY() { + return Number.parseFloat(titleSel().attr('y')); + } + + function titleSel() { + return d3.select('.infolayer .g-gtitle .gtitle'); + } +}); + describe('editable titles', function() { 'use strict'; From a7e3a9d7f641a5174d7e650f49101f599da62c16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=B6stl?= Date: Tue, 30 Oct 2018 11:55:02 +0100 Subject: [PATCH 02/49] Transition layout titles from being strings to be objects [882] --- src/components/titles/index.js | 3 +- src/plot_api/helpers.js | 17 ++++++++++ src/plot_api/subroutines.js | 2 +- src/plots/cartesian/axes.js | 2 +- src/plots/cartesian/axis_defaults.js | 2 +- src/plots/cartesian/layout_attributes.js | 10 +++--- src/plots/layout_attributes.js | 14 ++++---- src/plots/plots.js | 2 +- test/jasmine/tests/titles_test.js | 41 +++++++++++++++++------- 9 files changed, 66 insertions(+), 27 deletions(-) diff --git a/src/components/titles/index.js b/src/components/titles/index.js index e2211dd88df..7c2926664db 100644 --- a/src/components/titles/index.js +++ b/src/components/titles/index.js @@ -74,7 +74,8 @@ function draw(gd, titleClass, options) { var opacity = 1; var isplaceholder = false; - var txt = (cont.title || '').trim(); + var title = cont.title; + var txt = (title && title.text ? title.text : '').trim(); // only make this title editable if we positively identify its property // as one that has editing enabled. diff --git a/src/plot_api/helpers.js b/src/plot_api/helpers.js index 4866edcd6ec..678e8eee9e7 100644 --- a/src/plot_api/helpers.js +++ b/src/plot_api/helpers.js @@ -176,6 +176,23 @@ exports.cleanLayout = function(layout) { } } + // Check for old-style title definitions + if(!Lib.isPlainObject(layout.title)) { + layout.title = { + text: layout.title + }; + } + if(layout.xaxis && !Lib.isPlainObject(layout.xaxis.title)) { + layout.xaxis.title = { + text: layout.xaxis.title + }; + } + if(layout.yaxis && !Lib.isPlainObject(layout.yaxis.title)) { + layout.yaxis.title = { + text: layout.yaxis.title + }; + } + /* * Moved from rotate -> orbit for dragmode */ diff --git a/src/plot_api/subroutines.js b/src/plot_api/subroutines.js index 9a6bcfe5814..04f4abac80d 100644 --- a/src/plot_api/subroutines.js +++ b/src/plot_api/subroutines.js @@ -452,7 +452,7 @@ exports.drawMainTitle = function(gd) { Titles.draw(gd, 'gtitle', { propContainer: fullLayout, - propName: 'title', + propName: 'title.text', placeholder: fullLayout._dfltTitle.plot, attributes: { x: fullLayout.width / 2, diff --git a/src/plots/cartesian/axes.js b/src/plots/cartesian/axes.js index 3f1464aa9fb..a9e91a2c594 100644 --- a/src/plots/cartesian/axes.js +++ b/src/plots/cartesian/axes.js @@ -2387,7 +2387,7 @@ axes.drawTitle = function(gd, ax) { Titles.draw(gd, axId + 'title', { propContainer: ax, - propName: ax._name + '.title', + propName: ax._name + '.title.text', placeholder: fullLayout._dfltTitle[axLetter], avoid: avoid, transform: transform, diff --git a/src/plots/cartesian/axis_defaults.js b/src/plots/cartesian/axis_defaults.js index f3691bf8003..f4fe6d820fe 100644 --- a/src/plots/cartesian/axis_defaults.js +++ b/src/plots/cartesian/axis_defaults.js @@ -68,7 +68,7 @@ module.exports = function handleAxisDefaults(containerIn, containerOut, coerce, // try to get default title from splom trace, fallback to graph-wide value var dfltTitle = splomStash.label || layoutOut._dfltTitle[letter]; - coerce('title', dfltTitle); + coerce('title.text', dfltTitle); Lib.coerceFont(coerce, 'titlefont', { family: font.family, size: Math.round(font.size * 1.2), diff --git a/src/plots/cartesian/layout_attributes.js b/src/plots/cartesian/layout_attributes.js index 404f023eb71..251e3870200 100644 --- a/src/plots/cartesian/layout_attributes.js +++ b/src/plots/cartesian/layout_attributes.js @@ -41,10 +41,12 @@ module.exports = { ].join(' ') }, title: { - valType: 'string', - role: 'info', - editType: 'ticks', - description: 'Sets the title of this axis.' + text: { + valType: 'string', + role: 'info', + editType: 'ticks', + description: 'Sets the title of this axis.' + } }, titlefont: fontAttrs({ editType: 'ticks', diff --git a/src/plots/layout_attributes.js b/src/plots/layout_attributes.js index 103a0b234f7..81961359da2 100644 --- a/src/plots/layout_attributes.js +++ b/src/plots/layout_attributes.js @@ -26,12 +26,14 @@ globalFont.color.dflt = colorAttrs.defaultLine; module.exports = { font: globalFont, title: { - valType: 'string', - role: 'info', - editType: 'layoutstyle', - description: [ - 'Sets the plot\'s title.' - ].join(' ') + text: { + valType: 'string', + role: 'info', + editType: 'layoutstyle', + description: [ + 'Sets the plot\'s title.' + ].join(' ') + } }, titlefont: fontAttrs({ editType: 'layoutstyle', diff --git a/src/plots/plots.js b/src/plots/plots.js index 7d8947ff6b9..6f58209ff26 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -1324,7 +1324,7 @@ plots.supplyLayoutGlobalDefaults = function(layoutIn, layoutOut, formatObj) { var globalFont = Lib.coerceFont(coerce, 'font'); - coerce('title', layoutOut._dfltTitle.plot); + coerce('title.text', layoutOut._dfltTitle.plot); Lib.coerceFont(coerce, 'titlefont', { family: globalFont.family, diff --git a/test/jasmine/tests/titles_test.js b/test/jasmine/tests/titles_test.js index e8fbe93b71b..bd8f1312f1b 100644 --- a/test/jasmine/tests/titles_test.js +++ b/test/jasmine/tests/titles_test.js @@ -12,7 +12,11 @@ describe('Plot title', function() { 'use strict'; var data = [{x: [1, 2, 3], y: [1, 2, 3]}]; - var layout = {title: 'Plotly line chart'}; + var layout = { + title: { + text: 'Plotly line chart' + } + }; var gd; beforeEach(function() { @@ -24,11 +28,22 @@ describe('Plot title', function() { it('is centered horizontally and vertically above the plot by default', function() { Plotly.plot(gd, data, layout); + expectDefaultCenteredPosition(); + }); + + it('can still be defined as `layout.title` to ensure backwards-compatibility', function() { + Plotly.plot(gd, data, {title: 'Plotly line chart'}); + + expect(titleSel().text()).toBe('Plotly line chart'); + expectDefaultCenteredPosition(); + }); + + function expectDefaultCenteredPosition() { var containerBB = gd.getBoundingClientRect(); expect(titleX()).toBe(containerBB.width / 2); expect(titleY()).toBe(gd._fullLayout.margin.t / 2); - }); + } function titleX() { return Number.parseFloat(titleSel().attr('x')); @@ -39,7 +54,9 @@ describe('Plot title', function() { } function titleSel() { - return d3.select('.infolayer .g-gtitle .gtitle'); + var titleSel = d3.select('.infolayer .g-gtitle .gtitle'); + expect(titleSel.empty()).toBe(false, 'Title element missing'); + return titleSel; } }); @@ -117,9 +134,9 @@ describe('editable titles', function() { it('has hover effects for blank titles', function(done) { Plotly.plot(gd, data, { - xaxis: {title: ''}, - yaxis: {title: ''}, - title: '' + xaxis: {title: {text: ''}}, + yaxis: {title: {text: ''}}, + title: {text: ''} }, {editable: true}) .then(function() { return Promise.all([ @@ -133,18 +150,18 @@ describe('editable titles', function() { it('has no hover effects for titles that used to be blank', function(done) { Plotly.plot(gd, data, { - xaxis: {title: ''}, - yaxis: {title: ''}, - title: '' + xaxis: {title: {text: ''}}, + yaxis: {title: {text: ''}}, + title: {text: ''} }, {editable: true}) .then(function() { - return editTitle('x', 'xaxis.title', 'XXX'); + return editTitle('x', 'xaxis.title.text', 'XXX'); }) .then(function() { - return editTitle('y', 'yaxis.title', 'YYY'); + return editTitle('y', 'yaxis.title.text', 'YYY'); }) .then(function() { - return editTitle('g', 'title', 'TTT'); + return editTitle('g', 'title.text', 'TTT'); }) .then(function() { return Promise.all([ From 50a1e9b5a9fde39aab5597ab123c32961f702f4e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=B6stl?= Date: Wed, 31 Oct 2018 08:55:50 +0100 Subject: [PATCH 03/49] Support updating titles through `relayout` and `update` [882] --- src/plot_api/plot_api.js | 26 ++++ src/plots/cartesian/layout_attributes.js | 3 +- src/plots/layout_attributes.js | 3 +- test/jasmine/tests/titles_test.js | 163 ++++++++++++++++++++--- 4 files changed, 174 insertions(+), 21 deletions(-) diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js index 93dd63c3cd4..6c4979cfa43 100644 --- a/src/plot_api/plot_api.js +++ b/src/plot_api/plot_api.js @@ -1686,6 +1686,28 @@ function _restyle(gd, aobj, traces) { }; } +/** + * Maps deprecated layout "string attributes" to + * "string attributes" of the current API to ensure backwards compatibility. + * + * E.g. Maps {'xaxis.title': 'A chart'} to {'xaxis.title.text': 'A chart'} + * + * @param layoutObj + */ +function mapDeprecatedLayoutAttributeStrings(layoutObj) { + if(layoutObj.title && !Lib.isPlainObject(layoutObj.title)) { + layoutObj.title = {text: layoutObj.title}; + } + if(layoutObj['xaxis.title'] !== undefined) { + layoutObj['xaxis.title.text'] = layoutObj['xaxis.title']; + delete layoutObj['xaxis.title']; + } + if(layoutObj['yaxis.title'] !== undefined) { + layoutObj['yaxis.title.text'] = layoutObj['yaxis.title']; + delete layoutObj['yaxis.title']; + } +} + /** * relayout: update layout attributes of an existing plot * @@ -1726,6 +1748,8 @@ exports.relayout = function relayout(gd, astr, val) { if(Object.keys(aobj).length) gd.changed = true; + mapDeprecatedLayoutAttributeStrings(aobj); + var specs = _relayout(gd, aobj); var flags = specs.flags; @@ -2203,6 +2227,8 @@ exports.update = function update(gd, traceUpdate, layoutUpdate, _traces) { var restyleSpecs = _restyle(gd, Lib.extendFlat({}, traceUpdate), traces); var restyleFlags = restyleSpecs.flags; + mapDeprecatedLayoutAttributeStrings(layoutUpdate); + var relayoutSpecs = _relayout(gd, Lib.extendFlat({}, layoutUpdate)); var relayoutFlags = relayoutSpecs.flags; diff --git a/src/plots/cartesian/layout_attributes.js b/src/plots/cartesian/layout_attributes.js index 251e3870200..ce0a44f65ed 100644 --- a/src/plots/cartesian/layout_attributes.js +++ b/src/plots/cartesian/layout_attributes.js @@ -46,7 +46,8 @@ module.exports = { role: 'info', editType: 'ticks', description: 'Sets the title of this axis.' - } + }, + editType: 'ticks' }, titlefont: fontAttrs({ editType: 'ticks', diff --git a/src/plots/layout_attributes.js b/src/plots/layout_attributes.js index 81961359da2..0ad7e7627cf 100644 --- a/src/plots/layout_attributes.js +++ b/src/plots/layout_attributes.js @@ -33,7 +33,8 @@ module.exports = { description: [ 'Sets the plot\'s title.' ].join(' ') - } + }, + editType: 'layoutstyle' }, titlefont: fontAttrs({ editType: 'layoutstyle', diff --git a/test/jasmine/tests/titles_test.js b/test/jasmine/tests/titles_test.js index bd8f1312f1b..a2f7cd8bb54 100644 --- a/test/jasmine/tests/titles_test.js +++ b/test/jasmine/tests/titles_test.js @@ -2,12 +2,12 @@ var d3 = require('d3'); var Plotly = require('@lib/index'); var interactConstants = require('@src/constants/interactions'); +var Lib = require('@src/lib'); var createGraphDiv = require('../assets/create_graph_div'); var destroyGraphDiv = require('../assets/destroy_graph_div'); var mouseEvent = require('../assets/mouse_event'); - describe('Plot title', function() { 'use strict'; @@ -28,39 +28,164 @@ describe('Plot title', function() { it('is centered horizontally and vertically above the plot by default', function() { Plotly.plot(gd, data, layout); - expectDefaultCenteredPosition(); + expectDefaultCenteredPosition(gd); }); it('can still be defined as `layout.title` to ensure backwards-compatibility', function() { Plotly.plot(gd, data, {title: 'Plotly line chart'}); - expect(titleSel().text()).toBe('Plotly line chart'); - expectDefaultCenteredPosition(); + expectTitle('Plotly line chart'); + expectDefaultCenteredPosition(gd); }); - function expectDefaultCenteredPosition() { - var containerBB = gd.getBoundingClientRect(); + it('can be updated via `relayout`', function(done) { + Plotly.plot(gd, data, {title: 'Plotly line chart'}) + .then(expectTitleFn('Plotly line chart')) + .then(function() { + return Plotly.relayout(gd, {title: {text: 'Some other title'}}); + }) + .then(expectTitleFn('Some other title')) + .catch(fail) + .then(done); + }); - expect(titleX()).toBe(containerBB.width / 2); - expect(titleY()).toBe(gd._fullLayout.margin.t / 2); - } + it('preserves alignment when text is updated via `Plotly.relayout` using an attribute string', function() { + // TODO implement once alignment is implemented + }); - function titleX() { - return Number.parseFloat(titleSel().attr('x')); - } + it('preserves alignment when text is updated via `Plotly.update` using an attribute string', function() { + // TODO implement once alignment is implemented + }); + + it('discards alignment when text is updated via `Plotly.relayout` by passing a new title object', function() { + // TODO implement once alignment is implemented + }); + + it('discards alignment when text is updated via `Plotly.update` by passing a new title object', function() { + // TODO implement once alignment is implemented + }); +}); + +describe('Titles can be updated', function() { + 'use strict'; + + var data = [{x: [1, 2, 3], y: [1, 2, 3]}]; + var NEW_TITLE = 'Weight over years'; + var NEW_XTITLE = 'Age in years'; + var NEW_YTITLE = 'Average weight'; + var gd; + + beforeEach(function() { + var layout = { + title: {text: 'Plotly line chart'}, + xaxis: {title: {text: 'Age'}}, + yaxis: {title: {text: 'Weight'}} + }; + gd = createGraphDiv(); + Plotly.plot(gd, data, Lib.extendDeep({}, layout)); + + expectTitles('Plotly line chart', 'Age', 'Weight'); + }); - function titleY() { - return Number.parseFloat(titleSel().attr('y')); + afterEach(destroyGraphDiv); + + [ + { + desc: 'by replacing the entire title objects', + update: { + title: {text: NEW_TITLE}, + xaxis: {title: {text: NEW_XTITLE}}, + yaxis: {title: {text: NEW_YTITLE}} + } + }, + { + desc: 'by using attribute strings', + update: { + 'title.text': NEW_TITLE, + 'xaxis.title.text': NEW_XTITLE, + 'yaxis.title.text': NEW_YTITLE + } + }, + { + desc: 'despite passing title only as a string (backwards-compatibility)', + update: { + title: NEW_TITLE, + xaxis: {title: NEW_XTITLE}, + yaxis: {title: NEW_YTITLE} + } + }, + { + desc: 'despite passing title only as a string using string attributes ' + + '(backwards-compatibility)', + update: { + 'title': NEW_TITLE, + 'xaxis.title': NEW_XTITLE, + 'yaxis.title': NEW_YTITLE + } + } + ].forEach(function(testCase) { + it('via `Plotly.relayout` ' + testCase.desc, function() { + Plotly.relayout(gd, testCase.update); + + expectChangedTitles(); + }); + + it('via `Plotly.update` ' + testCase.desc, function() { + Plotly.update(gd, {}, testCase.update); + + expectChangedTitles(); + }); + }); + + function expectChangedTitles() { + expectTitles(NEW_TITLE, NEW_XTITLE, NEW_YTITLE); } - function titleSel() { - var titleSel = d3.select('.infolayer .g-gtitle .gtitle'); - expect(titleSel.empty()).toBe(false, 'Title element missing'); - return titleSel; + function expectTitles(expTitle, expXTitle, expYTitle) { + expectTitle(expTitle); + + var xTitleSel = d3.select('.xtitle'); + expect(xTitleSel.empty()).toBe(false, 'X-axis title element missing'); + expect(xTitleSel.text()).toBe(expXTitle); + + var yTitleSel = d3.select('.ytitle'); + expect(yTitleSel.empty()).toBe(false, 'Y-axis title element missing'); + expect(yTitleSel.text()).toBe(expYTitle); } }); -describe('editable titles', function() { +function expectTitle(expTitle) { + expectTitleFn(expTitle)(); +} + +function expectTitleFn(expTitle) { + return function() { + expect(titleSel().text()).toBe(expTitle); + }; +} + +function expectDefaultCenteredPosition(gd) { + var containerBB = gd.getBoundingClientRect(); + + expect(titleX()).toBe(containerBB.width / 2); + expect(titleY()).toBe(gd._fullLayout.margin.t / 2); +} + +function titleX() { + return Number.parseFloat(titleSel().attr('x')); +} + +function titleY() { + return Number.parseFloat(titleSel().attr('y')); +} + +function titleSel() { + var titleSel = d3.select('.infolayer .g-gtitle .gtitle'); + expect(titleSel.empty()).toBe(false, 'Title element missing'); + return titleSel; +} + +describe('Editable titles', function() { 'use strict'; var data = [{x: [1, 2, 3], y: [1, 2, 3]}]; From 4d21886afc004ea341c428673226a62cd2045a30 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=B6stl?= Date: Mon, 5 Nov 2018 14:33:46 +0100 Subject: [PATCH 04/49] Deprecate `*.titlefont` attributes in favor of `*.title.font` [882] --- src/components/colorbar/draw.js | 5 +- src/components/titles/index.js | 12 +- src/plot_api/helpers.js | 16 ++ src/plot_api/plot_api.js | 63 +++++- src/plots/cartesian/axes.js | 2 +- src/plots/cartesian/axis_defaults.js | 2 +- src/plots/cartesian/layout_attributes.js | 12 +- src/plots/layout_attributes.js | 8 +- src/plots/plots.js | 2 +- test/jasmine/tests/titles_test.js | 271 ++++++++++++++++++++++- 10 files changed, 358 insertions(+), 35 deletions(-) diff --git a/src/components/colorbar/draw.js b/src/components/colorbar/draw.js index a87982e3628..c30d6078f5f 100644 --- a/src/components/colorbar/draw.js +++ b/src/components/colorbar/draw.js @@ -184,7 +184,6 @@ module.exports = function draw(gd, id) { showticksuffix: opts.showticksuffix, ticksuffix: opts.ticksuffix, title: opts.title, - titlefont: opts.titlefont, showline: true, anchor: 'free', side: 'right', @@ -290,7 +289,7 @@ module.exports = function draw(gd, id) { // when we squish the axis. This one only applies to // top or bottom titles, not right side. var x = gs.l + (opts.x + xpadFrac) * gs.w, - fontSize = cbAxisOut.titlefont.size, + fontSize = cbAxisOut.title.font.size, y; if(opts.titleside === 'top') { @@ -460,7 +459,7 @@ module.exports = function draw(gd, id) { }, function() { if(['top', 'bottom'].indexOf(opts.titleside) === -1) { - var fontSize = cbAxisOut.titlefont.size, + var fontSize = cbAxisOut.title.font.size, y = cbAxisOut._offset + cbAxisOut._length / 2, x = gs.l + (cbAxisOut.position || 0) * gs.w + ((cbAxisOut.side === 'right') ? 10 + fontSize * ((cbAxisOut.showticklabels ? 1 : 0.5)) : diff --git a/src/components/titles/index.js b/src/components/titles/index.js index 7c2926664db..d596044c08d 100644 --- a/src/components/titles/index.js +++ b/src/components/titles/index.js @@ -67,19 +67,21 @@ function draw(gd, titleClass, options) { var group = options.containerGroup; var fullLayout = gd._fullLayout; - var titlefont = cont.titlefont || {}; - var font = titlefont.family; - var fontSize = titlefont.size; - var fontColor = titlefont.color; var opacity = 1; var isplaceholder = false; var title = cont.title; var txt = (title && title.text ? title.text : '').trim(); + var font = title && title.font ? title.font : {}; + var fontFamily = font.family; + var fontSize = font.size; + var fontColor = font.color; + // only make this title editable if we positively identify its property // as one that has editing enabled. var editAttr; + // TODO 882 Probably compare against 'title.text' if(prop === 'title') editAttr = 'titleText'; else if(prop.indexOf('axis') !== -1) editAttr = 'axisTitleText'; else if(prop.indexOf('colorbar' !== -1)) editAttr = 'colorbarTitleText'; @@ -138,7 +140,7 @@ function draw(gd, titleClass, options) { titleEl.attr('transform', transformVal); titleEl.style({ - 'font-family': font, + 'font-family': fontFamily, 'font-size': d3.round(fontSize, 2) + 'px', fill: Color.rgb(fontColor), opacity: opacity * Color.opacity(fontColor), diff --git a/src/plot_api/helpers.js b/src/plot_api/helpers.js index 678e8eee9e7..0ec7dd10b37 100644 --- a/src/plot_api/helpers.js +++ b/src/plot_api/helpers.js @@ -193,6 +193,22 @@ exports.cleanLayout = function(layout) { }; } + // Check for old-style title font definitions + if(Lib.isPlainObject(layout.titlefont) && !Lib.isPlainObject(layout.title.font)) { + layout.title.font = layout.titlefont; + delete layout.titlefont; + } + if(layout.xaxis && + Lib.isPlainObject(layout.xaxis.titlefont) && !Lib.isPlainObject(layout.xaxis.title.font)) { + layout.xaxis.title.font = layout.xaxis.titlefont; + delete layout.xaxis.titlefont; + } + if(layout.yaxis && + Lib.isPlainObject(layout.yaxis.titlefont) && !Lib.isPlainObject(layout.yaxis.title.font)) { + layout.yaxis.title.font = layout.yaxis.titlefont; + delete layout.yaxis.titlefont; + } + /* * Moved from rotate -> orbit for dragmode */ diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js index 6c4979cfa43..6a46e24c603 100644 --- a/src/plot_api/plot_api.js +++ b/src/plot_api/plot_api.js @@ -1686,6 +1686,34 @@ function _restyle(gd, aobj, traces) { }; } +/** + * Maps deprecated layout attributes that need to be converted + * to the current API at this stage to ensure backwards compatibility. + * + * @param layoutObj + */ +function mapDeprecatedLayoutAttributes(layoutObj) { + + // Support old-style update of the title's font + var isTitlefontSet = layoutObj.titlefont; + var isFontInTitleSet = layoutObj.title && layoutObj.title.font; + var isFontInTitleNotSet = !isFontInTitleSet; + if(isTitlefontSet && isFontInTitleNotSet) { + + // Use string attribute because initiating a new title object + // to be able to specify a font property on it would require to + // know the potentially existing `title.text` property. + layoutObj['title.font'] = layoutObj.titlefont; + delete layoutObj.titlefont; + } + + // Note, that updating x-axis and y-axis title fonts + // was never supported unless (i) using string + // attributes or (ii) passing `xaxis.title` / `yaxis.title` + // again. And these cases are covered by + // helpers.cleanLayout anyways. +} + /** * Maps deprecated layout "string attributes" to * "string attributes" of the current API to ensure backwards compatibility. @@ -1698,13 +1726,32 @@ function mapDeprecatedLayoutAttributeStrings(layoutObj) { if(layoutObj.title && !Lib.isPlainObject(layoutObj.title)) { layoutObj.title = {text: layoutObj.title}; } - if(layoutObj['xaxis.title'] !== undefined) { - layoutObj['xaxis.title.text'] = layoutObj['xaxis.title']; - delete layoutObj['xaxis.title']; - } - if(layoutObj['yaxis.title'] !== undefined) { - layoutObj['yaxis.title.text'] = layoutObj['yaxis.title']; - delete layoutObj['yaxis.title']; + replace('xaxis.title', 'xaxis.title.text'); + replace('yaxis.title', 'yaxis.title.text'); + + // Note: Only "nested" (dot notation) attributes + // need to be converted. For example 'titlefont' + // was a top-level attribute and thus is covered by + // the general compatibility layer. + replace('titlefont.color', 'title.font.color'); + replace('titlefont.family', 'title.font.family'); + replace('titlefont.size', 'title.font.size'); + + replace('xaxis.titlefont', 'xaxis.title.font'); + replace('xaxis.titlefont.color', 'xaxis.title.font.color'); + replace('xaxis.titlefont.family', 'xaxis.title.font.family'); + replace('xaxis.titlefont.size', 'xaxis.title.font.size'); + + replace('yaxis.titlefont', 'yaxis.title.font'); + replace('yaxis.titlefont.color', 'yaxis.title.font.color'); + replace('yaxis.titlefont.family', 'yaxis.title.font.family'); + replace('yaxis.titlefont.size', 'yaxis.title.font.size'); + + function replace(oldKey, newKey) { + if(layoutObj[oldKey] !== undefined) { + layoutObj[newKey] = layoutObj[oldKey]; + delete layoutObj[oldKey]; + } } } @@ -1748,6 +1795,7 @@ exports.relayout = function relayout(gd, astr, val) { if(Object.keys(aobj).length) gd.changed = true; + mapDeprecatedLayoutAttributes(aobj); mapDeprecatedLayoutAttributeStrings(aobj); var specs = _relayout(gd, aobj); @@ -2227,6 +2275,7 @@ exports.update = function update(gd, traceUpdate, layoutUpdate, _traces) { var restyleSpecs = _restyle(gd, Lib.extendFlat({}, traceUpdate), traces); var restyleFlags = restyleSpecs.flags; + mapDeprecatedLayoutAttributes(layoutUpdate); mapDeprecatedLayoutAttributeStrings(layoutUpdate); var relayoutSpecs = _relayout(gd, Lib.extendFlat({}, layoutUpdate)); diff --git a/src/plots/cartesian/axes.js b/src/plots/cartesian/axes.js index a9e91a2c594..7170991727b 100644 --- a/src/plots/cartesian/axes.js +++ b/src/plots/cartesian/axes.js @@ -2337,7 +2337,7 @@ axes.drawTitle = function(gd, ax) { var axLetter = axId.charAt(0); var offsetBase = 1.5; var gs = fullLayout._size; - var fontSize = ax.titlefont.size; + var fontSize = ax.title.font.size; var transform, counterAxis, x, y; diff --git a/src/plots/cartesian/axis_defaults.js b/src/plots/cartesian/axis_defaults.js index f4fe6d820fe..dacf3dff14d 100644 --- a/src/plots/cartesian/axis_defaults.js +++ b/src/plots/cartesian/axis_defaults.js @@ -69,7 +69,7 @@ module.exports = function handleAxisDefaults(containerIn, containerOut, coerce, var dfltTitle = splomStash.label || layoutOut._dfltTitle[letter]; coerce('title.text', dfltTitle); - Lib.coerceFont(coerce, 'titlefont', { + Lib.coerceFont(coerce, 'title.font', { family: font.family, size: Math.round(font.size * 1.2), color: dfltFontColor diff --git a/src/plots/cartesian/layout_attributes.js b/src/plots/cartesian/layout_attributes.js index ce0a44f65ed..a7bbe0f5727 100644 --- a/src/plots/cartesian/layout_attributes.js +++ b/src/plots/cartesian/layout_attributes.js @@ -47,14 +47,14 @@ module.exports = { editType: 'ticks', description: 'Sets the title of this axis.' }, + font: fontAttrs({ + editType: 'ticks', + description: [ + 'Sets this axis\' title font.' + ].join(' ') + }), editType: 'ticks' }, - titlefont: fontAttrs({ - editType: 'ticks', - description: [ - 'Sets this axis\' title font.' - ].join(' ') - }), type: { valType: 'enumerated', // '-' means we haven't yet run autotype or couldn't find any data diff --git a/src/plots/layout_attributes.js b/src/plots/layout_attributes.js index 0ad7e7627cf..affbae67c90 100644 --- a/src/plots/layout_attributes.js +++ b/src/plots/layout_attributes.js @@ -34,12 +34,12 @@ module.exports = { 'Sets the plot\'s title.' ].join(' ') }, + font: fontAttrs({ + editType: 'layoutstyle', + description: 'Sets the title font.' + }), editType: 'layoutstyle' }, - titlefont: fontAttrs({ - editType: 'layoutstyle', - description: 'Sets the title font.' - }), autosize: { valType: 'boolean', role: 'info', diff --git a/src/plots/plots.js b/src/plots/plots.js index 6f58209ff26..d60e627d91a 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -1326,7 +1326,7 @@ plots.supplyLayoutGlobalDefaults = function(layoutIn, layoutOut, formatObj) { coerce('title.text', layoutOut._dfltTitle.plot); - Lib.coerceFont(coerce, 'titlefont', { + Lib.coerceFont(coerce, 'title.font', { family: globalFont.family, size: Math.round(globalFont.size * 1.4), color: globalFont.color diff --git a/test/jasmine/tests/titles_test.js b/test/jasmine/tests/titles_test.js index a2f7cd8bb54..f8f567c5262 100644 --- a/test/jasmine/tests/titles_test.js +++ b/test/jasmine/tests/titles_test.js @@ -3,6 +3,7 @@ var d3 = require('d3'); var Plotly = require('@lib/index'); var interactConstants = require('@src/constants/interactions'); var Lib = require('@src/lib'); +var rgb = require('@src/components/color').rgb; var createGraphDiv = require('../assets/create_graph_div'); var destroyGraphDiv = require('../assets/destroy_graph_div'); @@ -144,13 +145,238 @@ describe('Titles can be updated', function() { function expectTitles(expTitle, expXTitle, expYTitle) { expectTitle(expTitle); - var xTitleSel = d3.select('.xtitle'); - expect(xTitleSel.empty()).toBe(false, 'X-axis title element missing'); - expect(xTitleSel.text()).toBe(expXTitle); + var xSel = xTitleSel(); + expect(xSel.empty()).toBe(false, 'X-axis title element missing'); + expect(xSel.text()).toBe(expXTitle); - var yTitleSel = d3.select('.ytitle'); - expect(yTitleSel.empty()).toBe(false, 'Y-axis title element missing'); - expect(yTitleSel.text()).toBe(expYTitle); + var ySel = yTitleSel(); + expect(ySel.empty()).toBe(false, 'Y-axis title element missing'); + expect(ySel.text()).toBe(expYTitle); + } +}); + +describe('Titles support setting custom font properties', function() { + 'use strict'; + + var data = [{x: [1, 2, 3], y: [1, 2, 3]}]; + var gd; + + beforeEach(function() { + gd = createGraphDiv(); + }); + + afterEach(destroyGraphDiv); + + it('through defining a `font` property in the respective title attribute', function() { + var layout = { + title: { + text: 'Plotly line chart', + font: { + color: 'blue', + family: 'serif', + size: 24 + } + }, + xaxis: { + title: { + text: 'X-Axis', + font: { + color: '#333', + family: 'sans-serif', + size: 20 + } + } + }, + yaxis: { + title: { + text: 'Y-Axis', + font: { + color: '#666', + family: 'Arial', + size: 16 + } + } + } + }; + Plotly.plot(gd, data, layout); + + expectTitleFont('blue', 'serif', 24); + expectXAxisTitleFont('#333', 'sans-serif', 20); + expectYAxisTitleFont('#666', 'Arial', 16); + }); + + it('through using the deprecated `titlefont` properties (backwards-compatibility)', function() { + var layout = { + title: { + text: 'Plotly line chart', + }, + titlefont: { + color: 'blue', + family: 'serif', + size: 24 + }, + xaxis: { + title: { + text: 'X-Axis', + }, + titlefont: { + color: '#333', + family: 'sans-serif', + size: 20 + } + }, + yaxis: { + title: { + text: 'Y-Axis', + }, + titlefont: { + color: '#666', + family: 'Arial', + size: 16 + } + } + }; + Plotly.plot(gd, data, layout); + + expectTitleFont('blue', 'serif', 24); + expectXAxisTitleFont('#333', 'sans-serif', 20); + expectYAxisTitleFont('#666', 'Arial', 16); + }); +}); + +describe('Title fonts can be updated', function() { + 'use strict'; + + var data = [{x: [1, 2, 3], y: [1, 2, 3]}]; + var NEW_TITLE = 'Weight over years'; + var NEW_XTITLE = 'Age in years'; + var NEW_YTITLE = 'Average weight'; + var NEW_TITLE_FONT = {color: '#333', family: 'serif', size: 28}; + var NEW_XTITLE_FONT = {color: '#666', family: 'sans-serif', size: 18}; + var NEW_YTITLE_FONT = {color: '#999', family: 'serif', size: 12}; + var gd; + + beforeEach(function() { + var layout = { + title: { + text: 'Plotly line chart', + font: {color: 'black', family: 'sans-serif', size: 24} + }, + xaxis: { + title: { + text: 'Age', + font: {color: 'red', family: 'serif', size: 20} + } + }, + yaxis: { + title: { + text: 'Weight', + font: {color: 'green', family: 'monospace', size: 16} + } + } + }; + gd = createGraphDiv(); + Plotly.plot(gd, data, Lib.extendDeep({}, layout)); + + expectTitleFont('black', 'sans-serif', 24); + expectXAxisTitleFont('red', 'serif', 20); + expectYAxisTitleFont('green', 'monospace', 16); + }); + + afterEach(destroyGraphDiv); + + [ + { + desc: 'by replacing the entire title objects', + update: { + title: { + text: NEW_TITLE, + font: NEW_TITLE_FONT + }, + xaxis: { + title: { + text: NEW_XTITLE, + font: NEW_XTITLE_FONT + } + }, + yaxis: { + title: { + text: NEW_YTITLE, + font: NEW_YTITLE_FONT + } + } + } + }, + { + desc: 'by using attribute strings', + update: { + 'title.font.color': NEW_TITLE_FONT.color, + 'title.font.family': NEW_TITLE_FONT.family, + 'title.font.size': NEW_TITLE_FONT.size, + 'xaxis.title.font.color': NEW_XTITLE_FONT.color, + 'xaxis.title.font.family': NEW_XTITLE_FONT.family, + 'xaxis.title.font.size': NEW_XTITLE_FONT.size, + 'yaxis.title.font.color': NEW_YTITLE_FONT.color, + 'yaxis.title.font.family': NEW_YTITLE_FONT.family, + 'yaxis.title.font.size': NEW_YTITLE_FONT.size + } + }, + { + desc: 'despite passing deprecated `titlefont` properties (backwards-compatibility)', + update: { + titlefont: NEW_TITLE_FONT, + xaxis: { + title: NEW_XTITLE, + titlefont: NEW_XTITLE_FONT + }, + yaxis: { + title: NEW_YTITLE, + titlefont: NEW_YTITLE_FONT + } + } + }, + { + desc: 'despite using string attributes representing the deprecated structure ' + + '(backwards-compatibility)', + update: { + 'titlefont.color': NEW_TITLE_FONT.color, + 'titlefont.family': NEW_TITLE_FONT.family, + 'titlefont.size': NEW_TITLE_FONT.size, + 'xaxis.titlefont.color': NEW_XTITLE_FONT.color, + 'xaxis.titlefont.family': NEW_XTITLE_FONT.family, + 'xaxis.titlefont.size': NEW_XTITLE_FONT.size, + 'yaxis.titlefont.color': NEW_YTITLE_FONT.color, + 'yaxis.titlefont.family': NEW_YTITLE_FONT.family, + 'yaxis.titlefont.size': NEW_YTITLE_FONT.size + } + }, + { + desc: 'despite using string attributes replacing deprecated `titlefont` attributes ' + + '(backwards-compatibility)', + update: { + 'titlefont': NEW_TITLE_FONT, + 'xaxis.titlefont': NEW_XTITLE_FONT, + 'yaxis.titlefont': NEW_YTITLE_FONT + } + } + ].forEach(function(testCase) { + it('via `Plotly.relayout` ' + testCase.desc, function() { + Plotly.relayout(gd, testCase.update); + + expectChangedTitleFonts(); + }); + + it('via `Plotly.update` ' + testCase.desc, function() { + Plotly.update(gd, {}, testCase.update); + + expectChangedTitleFonts(); + }); + }); + + function expectChangedTitleFonts() { + expectTitleFont(NEW_TITLE_FONT.color, NEW_TITLE_FONT.family, NEW_TITLE_FONT.size); + expectXAxisTitleFont(NEW_XTITLE_FONT.color, NEW_XTITLE_FONT.family, NEW_XTITLE_FONT.size); + expectYAxisTitleFont(NEW_YTITLE_FONT.color, NEW_YTITLE_FONT.family, NEW_YTITLE_FONT.size); } }); @@ -164,6 +390,25 @@ function expectTitleFn(expTitle) { }; } +function expectTitleFont(color, family, size) { + expectFont(titleSel(), color, family, size); +} + +function expectXAxisTitleFont(color, family, size) { + expectFont(xTitleSel(), color, family, size); +} + +function expectYAxisTitleFont(color, family, size) { + expectFont(yTitleSel(), color, family, size); +} + +function expectFont(sel, color, family, size) { + var node = sel.node(); + expect(node.style.fill).toBe(rgb(color)); + expect(node.style.fontFamily).toBe(family); + expect(node.style.fontSize).toBe(size + 'px'); +} + function expectDefaultCenteredPosition(gd) { var containerBB = gd.getBoundingClientRect(); @@ -181,10 +426,22 @@ function titleY() { function titleSel() { var titleSel = d3.select('.infolayer .g-gtitle .gtitle'); - expect(titleSel.empty()).toBe(false, 'Title element missing'); + expect(titleSel.empty()).toBe(false, 'Plot title element missing'); return titleSel; } +function xTitleSel() { + var xTitleSel = d3.select('.xtitle'); + expect(xTitleSel.empty()).toBe(false, 'X-axis title element missing'); + return xTitleSel; +} + +function yTitleSel() { + var yTitleSel = d3.select('.ytitle'); + expect(yTitleSel.empty()).toBe(false, 'Y-axis title element missing'); + return yTitleSel; +} + describe('Editable titles', function() { 'use strict'; From 66a8154fd31f976fd86c42b315eb6ef233be09c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=B6stl?= Date: Wed, 7 Nov 2018 14:57:49 +0100 Subject: [PATCH 05/49] Implement basic chart title alignment options [882] - Not cleaned up. - Padding attributes missing. - `auto` values missing. --- src/constants/alignment.js | 11 +- src/plot_api/subroutines.js | 48 +++++- src/plots/layout_attributes.js | 64 ++++++++ src/plots/plots.js | 7 + test/jasmine/tests/titles_test.js | 256 +++++++++++++++++++++++++++++- 5 files changed, 374 insertions(+), 12 deletions(-) diff --git a/src/constants/alignment.js b/src/constants/alignment.js index a63cc18266d..120e9dc2361 100644 --- a/src/constants/alignment.js +++ b/src/constants/alignment.js @@ -41,12 +41,17 @@ module.exports = { // multiple of fontSize to get the vertical offset between lines LINE_SPACING: 1.3, - // multiple of fontSize to shift from the baseline to the midline + // multiple of fontSize to shift from the baseline + // to the cap (captical letter) line // (to use when we don't calculate this shift from Drawing.bBox) - // To be precise this should be half the cap height (capital letter) - // of the font, and according to wikipedia: + // This is an approximation since in reality cap height can differ + // from font to font. However, according to Wikipedia // an "average" font might have a cap height of 70% of the em // https://en.wikipedia.org/wiki/Em_(typography)#History + CAP_SHIFT: 0.70, + + // half the cap height (distance between baseline and cap line) + // of an "average" font (for more info see above). MID_SHIFT: 0.35, OPPOSITE_SIDE: { diff --git a/src/plot_api/subroutines.js b/src/plot_api/subroutines.js index 04f4abac80d..d3193d8baf5 100644 --- a/src/plot_api/subroutines.js +++ b/src/plot_api/subroutines.js @@ -449,19 +449,61 @@ function findCounterAxisLineWidth(ax, side, counterAx, axList) { exports.drawMainTitle = function(gd) { var fullLayout = gd._fullLayout; + var textAnchorDict = { + 'left': 'start', + 'center': 'middle', + 'right': 'end', + 'auto': 'middle' // TODO That needs to be calculated based on x + }; + var dyDict = { + 'top': alignmentConstants.CAP_SHIFT + 'em', + 'middle': alignmentConstants.MID_SHIFT + 'em', + 'bottom': '0em', + }; Titles.draw(gd, 'gtitle', { propContainer: fullLayout, propName: 'title.text', placeholder: fullLayout._dfltTitle.plot, attributes: { - x: fullLayout.width / 2, - y: fullLayout._size.t / 2, - 'text-anchor': 'middle' + x: getMainTitleX(fullLayout), + y: getMainTitleY(fullLayout), + 'text-anchor': textAnchorDict[fullLayout.title.xanchor], + dy: dyDict[fullLayout.title.yanchor] } }); }; +function getMainTitleX(fullLayout) { + var title = fullLayout.title; + var _size = fullLayout._size; + + switch(title.xref) { + case 'paper': + return _size.l + _size.w * title.x; + case 'container': + default: + return fullLayout.width * title.x; + } +} + +function getMainTitleY(fullLayout) { + var title = fullLayout.title; + var _size = fullLayout._size; + + if(title.y === 'auto') { + return _size.t / 2; + } else { + switch(title.yref) { + case 'paper': + return _size.t + _size.h - _size.h * title.y; + case 'container': + default: + return fullLayout.height - fullLayout.height * title.y; + } + } +} + exports.doTraceStyle = function(gd) { var calcdata = gd.calcdata; var editStyleCalls = []; diff --git a/src/plots/layout_attributes.js b/src/plots/layout_attributes.js index affbae67c90..706c14915f5 100644 --- a/src/plots/layout_attributes.js +++ b/src/plots/layout_attributes.js @@ -38,6 +38,70 @@ module.exports = { editType: 'layoutstyle', description: 'Sets the title font.' }), + xref: { + valType: 'enumerated', + dflt: 'container', + values: ['container', 'paper'], + role: 'info', + editType: 'layoutstyle', + description: [ + 'Some', + 'docu', // TODO document + ].join(' ') + }, + yref: { + valType: 'enumerated', + dflt: 'container', + values: ['container', 'paper'], + role: 'info', + editType: 'layoutstyle', + description: [ + 'Some', + 'docu', // TODO document + ].join(' ') + }, + x: { + valType: 'number', + dflt: '0.5', + role: 'style', + editType: 'layoutstyle', + description: [ + 'Some', + 'docu', // TODO document + ].join(' ') + }, + y: { + valType: 'any', + dflt: 'auto', + role: 'style', + editType: 'layoutstyle', + description: [ + 'Some', + 'docu', + ].join(' ') + }, + xanchor: { + valType: 'enumerated', + dflt: 'auto', + values: ['auto', 'left', 'center', 'right'], + role: 'info', + editType: 'layoutstyle', + description: [ + 'Some', + 'docu', // TODO document + ].join(' ') + }, + yanchor: { + valType: 'enumerated', + dflt: 'auto', + values: ['auto', 'top', 'middle', 'bottom'], + role: 'info', + editType: 'layoutstyle', + description: [ + 'Some', + 'docu', // TODO document + ].join(' ') + }, editType: 'layoutstyle' }, autosize: { diff --git a/src/plots/plots.js b/src/plots/plots.js index d60e627d91a..5b7ec913b8f 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -1332,6 +1332,13 @@ plots.supplyLayoutGlobalDefaults = function(layoutIn, layoutOut, formatObj) { color: globalFont.color }); + coerce('title.xref'); + coerce('title.yref'); + coerce('title.x'); // TODO restrict to [-2, 3]? + coerce('title.y'); // TODO restrict to [-2, 3]? + coerce('title.xanchor'); + coerce('title.yanchor'); + // Make sure that autosize is defaulted to *true* // on layouts with no set width and height for backward compatibly, // in particular https://plot.ly/javascript/responsive-fluid-layout/ diff --git a/test/jasmine/tests/titles_test.js b/test/jasmine/tests/titles_test.js index f8f567c5262..04040f90033 100644 --- a/test/jasmine/tests/titles_test.js +++ b/test/jasmine/tests/titles_test.js @@ -1,6 +1,7 @@ var d3 = require('d3'); var Plotly = require('@lib/index'); +var alignmentConstants = require('@src/constants/alignment'); var interactConstants = require('@src/constants/interactions'); var Lib = require('@src/lib'); var rgb = require('@src/components/color').rgb; @@ -13,11 +14,6 @@ describe('Plot title', function() { 'use strict'; var data = [{x: [1, 2, 3], y: [1, 2, 3]}]; - var layout = { - title: { - text: 'Plotly line chart' - } - }; var gd; beforeEach(function() { @@ -26,8 +22,25 @@ describe('Plot title', function() { afterEach(destroyGraphDiv); + var containerElemSelector = { + desc: 'container', + select: function() { + return d3.selectAll('.svg-container').node(); + } + }; + + var paperElemSelector = { + desc: 'plot area', + select: function() { + var bgLayer = d3.selectAll('.bglayer .bg'); + expect(bgLayer.empty()).toBe(false, + 'No background layer found representing the size of the plot area'); + return bgLayer.node(); + } + }; + it('is centered horizontally and vertically above the plot by default', function() { - Plotly.plot(gd, data, layout); + Plotly.plot(gd, data, {title: {text: 'Plotly line chart'}}); expectDefaultCenteredPosition(gd); }); @@ -50,6 +63,98 @@ describe('Plot title', function() { .then(done); }); + // Horizontal alignment + [ + { + selector: containerElemSelector, + xref: 'container' + }, + { + selector: paperElemSelector, + xref: 'paper' + } + ].forEach(function(testCase) { + it('can be placed at the left edge of the ' + testCase.selector.desc, function() { + Plotly.plot(gd, data, extendLayout({ + xref: testCase.xref, + x: 0, + xanchor: 'left' + })); + + expectLeftEdgeAlignedTo(testCase.selector); + }); + + it('can be placed at the right edge of the ' + testCase.selector.desc, function() { + Plotly.plot(gd, data, extendLayout({ + xref: testCase.xref, + x: 1, + xanchor: 'right' + })); + + expectRightEdgeAlignedTo(testCase.selector); + }); + + it('can be placed at the center of the ' + testCase.selector.desc, function() { + Plotly.plot(gd, data, extendLayout({ + xref: testCase.xref, + x: 0.5, + xanchor: 'center' + })); + + expectCenteredWithin(testCase.selector); + }); + }); + + // Vertical alignment + [ + { + selector: containerElemSelector, + yref: 'container' + }, + { + selector: paperElemSelector, + yref: 'paper' + } + ].forEach(function(testCase) { + it('can be placed at the top edge of the ' + testCase.selector.desc, function() { + Plotly.plot(gd, data, extendLayout({ + yref: testCase.yref, + y: 1, + yanchor: 'top' + })); + + expectCapLineAlignsWithTopEdgeOf(testCase.selector); + }); + + it('can be placed at the bottom edge of the ' + testCase.selector.desc, function() { + Plotly.plot(gd, data, extendLayout({ + yref: testCase.yref, + y: 0, + yanchor: 'bottom' + })); + + expectBaselineAlignsWithBottomEdgeOf(testCase.selector); + }); + + it('can be placed in the vertical center of the ' + testCase.selector.desc, function() { + Plotly.plot(gd, data, extendLayout({ + yref: testCase.yref, + y: 0.5, + yanchor: 'middle' + })); + + expectCenteredVerticallyWithin(testCase.selector); + }); + }); + + // TODO y set to `auto` to ensure current behavior is still supported + + // TODO x-anchor auto + + // TODO y-anchor auto + + // TODO padding + it('preserves alignment when text is updated via `Plotly.relayout` using an attribute string', function() { // TODO implement once alignment is implemented }); @@ -65,6 +170,145 @@ describe('Plot title', function() { it('discards alignment when text is updated via `Plotly.update` by passing a new title object', function() { // TODO implement once alignment is implemented }); + + function extendLayout(titleAttrs) { + return { + title: Lib.extendFlat({text: 'A Chart Title'}, titleAttrs), + + // This needs to be set to have a DOM element that represents the + // exact size of the plot area. + plot_bgcolor: '#f9f9f9', + }; + } + + function expectLeftEdgeAlignedTo(elemSelector) { + expectHorizontalEdgeAligned(elemSelector, 'left'); + } + + function expectRightEdgeAlignedTo(elemSelector) { + expectHorizontalEdgeAligned(elemSelector, 'right'); + } + + function expectHorizontalEdgeAligned(elemSelector, edgeKey) { + var refElem = elemSelector.select(); + var titleElem = titleSel().node(); + var refElemBB = refElem.getBoundingClientRect(); + var titleBB = titleElem.getBoundingClientRect(); + + var tolerance = 1.1; + var msg = 'Title ' + edgeKey + ' of ' + elemSelector.desc; + expect(titleBB[edgeKey] - refElemBB[edgeKey]).toBeWithin(0, tolerance, msg); + } + + function expectCapLineAlignsWithTopEdgeOf(elemSelector) { + var refElem = elemSelector.select(); + var refElemBB = refElem.getBoundingClientRect(); + + // Note: getBoundingClientRect of an SVG element + // doesn't return the tightest bounding box of the current text + // but a rectangle that is wide enough to contain any + // possible character even though something like 'Ö' isn't + // in the current title string. Moreover getBoundingClientRect + // (with respect to SVG elements) differs from browser to + // browser and thus is unreliable for testing vertical alignment + // of SVG text. Because of that the cap line is calculated based on the + // element properties. + var capLineY = calcTextCapLineY(titleSel()); + + var msg = 'Title\'s cap line y is same as the top of ' + elemSelector.desc; + expect(capLineY).toBeWithin(refElemBB.top, 1.1, msg); + } + + function expectBaselineAlignsWithBottomEdgeOf(elemSelector) { + var refElem = elemSelector.select(); + var refElemBB = refElem.getBoundingClientRect(); + + // Note: using getBoundingClientRect is not reliable, see + // comment in `expectCapLineAlignsWithTopEdgeOf` for more info. + var baselineY = calcTextBaselineY(titleSel()); + + var msg = 'Title baseline sits on the bottom of ' + elemSelector.desc; + expect(baselineY).toBeWithin(refElemBB.bottom, 1.1, msg); + } + + function calcTextCapLineY(textSel) { + var baselineY = calcTextBaselineY(textSel); + var fontSize = textSel.node().style.fontSize; + var fontSizeVal = parseFontSizeAttr(fontSize).val; + + // CAP_SHIFT is assuming a cap height of an average font. + // One would have to analyze the font metrics of the + // used font to determine an accurate cap shift factor. + return baselineY - fontSizeVal * alignmentConstants.CAP_SHIFT; + } + + function calcTextBaselineY(textSel) { + var y = Number.parseFloat(textSel.attr('y')); + var dy = textSel.attr('dy'); + var parsedDy = /^([0-9.]*)(\w*)$/.exec(dy); + var dyNumValue, dyUnit; + var fontSize, parsedFontSize; + var yShift; + + if(parsedDy) { + dyUnit = parsedDy[2]; + dyNumValue = Number.parseFloat(parsedDy[1]); + + if(dyUnit === 'em') { + fontSize = textSel.node().style.fontSize; + parsedFontSize = parseFontSizeAttr(fontSize); + + yShift = dyNumValue * Number.parseFloat(parsedFontSize.val); + } else if(dyUnit === '') { + yShift = dyNumValue; + } else { + fail('Calculating y-shift for unit ' + dyUnit + ' not implemented in test'); + } + } else { + fail('dy value \'' + dy + '\' could not be parsed by test'); + } + + return y + yShift; + } + + function parseFontSizeAttr(fontSizeAttr) { + var parsedFontSize = /^([0-9.]*)px$/.exec(fontSizeAttr); + var isFontSizeInPx = !!parsedFontSize; + expect(isFontSizeInPx).toBe(true, 'Tests assumes font-size is set in pixel'); + + return { + val: parsedFontSize[1], + unit: parsedFontSize[2] + }; + } + + function expectCenteredWithin(elemSelector) { + var refElem = elemSelector.select(); + var titleElem = titleSel().node(); + var refElemBB = refElem.getBoundingClientRect(); + var titleBB = titleElem.getBoundingClientRect(); + + var leftDistance = titleBB.left - refElemBB.left; + var rightDistance = refElemBB.right - titleBB.right; + + var tolerance = 1.1; + var msg = 'Title in center of ' + elemSelector.desc; + expect(leftDistance).toBeWithin(rightDistance, tolerance, msg); + } + + function expectCenteredVerticallyWithin(elemSelector) { + var refElem = elemSelector.select(); + var titleElem = titleSel().node(); + var refElemBB = refElem.getBoundingClientRect(); + var titleBB = titleElem.getBoundingClientRect(); + + var topDistance = titleBB.top - refElemBB.top; + var bottomDistance = refElemBB.bottom - titleBB.bottom; + + var tolerance = 1.1; + var msg = 'Title centered vertically within ' + elemSelector.desc; + expect(topDistance).toBeWithin(bottomDistance, tolerance, msg); + } }); describe('Titles can be updated', function() { From 6f583a42b0f803a9bf98b904cb5086ccb9ecd5ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=B6stl?= Date: Thu, 8 Nov 2018 14:57:09 +0100 Subject: [PATCH 06/49] Add test to ensure current chart title position is still supported [882] - Current position means the title's baseline sits in the middle of the to margin. --- test/jasmine/tests/titles_test.js | 130 +++++++++++++++++------------- 1 file changed, 75 insertions(+), 55 deletions(-) diff --git a/test/jasmine/tests/titles_test.js b/test/jasmine/tests/titles_test.js index 04040f90033..48dafdaca64 100644 --- a/test/jasmine/tests/titles_test.js +++ b/test/jasmine/tests/titles_test.js @@ -147,7 +147,22 @@ describe('Plot title', function() { }); }); - // TODO y set to `auto` to ensure current behavior is still supported + it('provides a y \'auto\' value putting title baseline in middle ' + + 'of top margin irrespective of `yref`', function() { + + // yref: 'container' + Plotly.plot(gd, data, extendLayout({ + yref: 'container', + y: 'auto' + })); + + expectBaselineInMiddleOfTopMargin(gd); + + // yref: 'paper' + Plotly.relayout(gd, {'title.yref': 'paper'}); + + expectBaselineInMiddleOfTopMargin(gd); + }); // TODO x-anchor auto @@ -231,57 +246,6 @@ describe('Plot title', function() { expect(baselineY).toBeWithin(refElemBB.bottom, 1.1, msg); } - function calcTextCapLineY(textSel) { - var baselineY = calcTextBaselineY(textSel); - var fontSize = textSel.node().style.fontSize; - var fontSizeVal = parseFontSizeAttr(fontSize).val; - - // CAP_SHIFT is assuming a cap height of an average font. - // One would have to analyze the font metrics of the - // used font to determine an accurate cap shift factor. - return baselineY - fontSizeVal * alignmentConstants.CAP_SHIFT; - } - - function calcTextBaselineY(textSel) { - var y = Number.parseFloat(textSel.attr('y')); - var dy = textSel.attr('dy'); - var parsedDy = /^([0-9.]*)(\w*)$/.exec(dy); - var dyNumValue, dyUnit; - var fontSize, parsedFontSize; - var yShift; - - if(parsedDy) { - dyUnit = parsedDy[2]; - dyNumValue = Number.parseFloat(parsedDy[1]); - - if(dyUnit === 'em') { - fontSize = textSel.node().style.fontSize; - parsedFontSize = parseFontSizeAttr(fontSize); - - yShift = dyNumValue * Number.parseFloat(parsedFontSize.val); - } else if(dyUnit === '') { - yShift = dyNumValue; - } else { - fail('Calculating y-shift for unit ' + dyUnit + ' not implemented in test'); - } - } else { - fail('dy value \'' + dy + '\' could not be parsed by test'); - } - - return y + yShift; - } - - function parseFontSizeAttr(fontSizeAttr) { - var parsedFontSize = /^([0-9.]*)px$/.exec(fontSizeAttr); - var isFontSizeInPx = !!parsedFontSize; - expect(isFontSizeInPx).toBe(true, 'Tests assumes font-size is set in pixel'); - - return { - val: parsedFontSize[1], - unit: parsedFontSize[2] - }; - } - function expectCenteredWithin(elemSelector) { var refElem = elemSelector.select(); var titleElem = titleSel().node(); @@ -657,15 +621,71 @@ function expectDefaultCenteredPosition(gd) { var containerBB = gd.getBoundingClientRect(); expect(titleX()).toBe(containerBB.width / 2); - expect(titleY()).toBe(gd._fullLayout.margin.t / 2); + expectBaselineInMiddleOfTopMargin(gd); +} + +function expectBaselineInMiddleOfTopMargin(gd) { + var baselineY = calcTextBaselineY(titleSel()); + var topMarginHeight = gd._fullLayout.margin.t; + + expect(baselineY).toBe(topMarginHeight / 2); } function titleX() { return Number.parseFloat(titleSel().attr('x')); } -function titleY() { - return Number.parseFloat(titleSel().attr('y')); +function calcTextBaselineY(textSel) { + var y = Number.parseFloat(textSel.attr('y')); + var yShift = 0; + var dy = textSel.attr('dy'); + var parsedDy, dyNumValue, dyUnit; + var fontSize, parsedFontSize; + + if(dy) { + parsedDy = /^([0-9.]*)(\w*)$/.exec(dy); + if(parsedDy) { + dyUnit = parsedDy[2]; + dyNumValue = Number.parseFloat(parsedDy[1]); + + if(dyUnit === 'em') { + fontSize = textSel.node().style.fontSize; + parsedFontSize = parseFontSizeAttr(fontSize); + + yShift = dyNumValue * Number.parseFloat(parsedFontSize.val); + } else if(dyUnit === '') { + yShift = dyNumValue; + } else { + fail('Calculating y-shift for unit ' + dyUnit + ' not implemented in test'); + } + } else { + fail('dy value \'' + dy + '\' could not be parsed by test'); + } + } + + return y + yShift; +} + +function calcTextCapLineY(textSel) { + var baselineY = calcTextBaselineY(textSel); + var fontSize = textSel.node().style.fontSize; + var fontSizeVal = parseFontSizeAttr(fontSize).val; + + // CAP_SHIFT is assuming a cap height of an average font. + // One would have to analyze the font metrics of the + // used font to determine an accurate cap shift factor. + return baselineY - fontSizeVal * alignmentConstants.CAP_SHIFT; +} + +function parseFontSizeAttr(fontSizeAttr) { + var parsedFontSize = /^([0-9.]*)px$/.exec(fontSizeAttr); + var isFontSizeInPx = !!parsedFontSize; + expect(isFontSizeInPx).toBe(true, 'Tests assumes font-size is set in pixel'); + + return { + val: parsedFontSize[1], + unit: parsedFontSize[2] + }; } function titleSel() { From 185772bc4b8531e67c5b9406320544e784b2add1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=B6stl?= Date: Mon, 12 Nov 2018 09:43:01 +0100 Subject: [PATCH 07/49] Implement 'auto' values for title's `xanchor` and `yanchor` [882] --- src/plot_api/subroutines.js | 74 +++++++++++++++++++++++++------ test/jasmine/tests/titles_test.js | 72 +++++++++++++++++++++++++++++- 2 files changed, 131 insertions(+), 15 deletions(-) diff --git a/src/plot_api/subroutines.js b/src/plot_api/subroutines.js index d3193d8baf5..cc8738563d3 100644 --- a/src/plot_api/subroutines.js +++ b/src/plot_api/subroutines.js @@ -449,17 +449,6 @@ function findCounterAxisLineWidth(ax, side, counterAx, axList) { exports.drawMainTitle = function(gd) { var fullLayout = gd._fullLayout; - var textAnchorDict = { - 'left': 'start', - 'center': 'middle', - 'right': 'end', - 'auto': 'middle' // TODO That needs to be calculated based on x - }; - var dyDict = { - 'top': alignmentConstants.CAP_SHIFT + 'em', - 'middle': alignmentConstants.MID_SHIFT + 'em', - 'bottom': '0em', - }; Titles.draw(gd, 'gtitle', { propContainer: fullLayout, @@ -468,8 +457,8 @@ exports.drawMainTitle = function(gd) { attributes: { x: getMainTitleX(fullLayout), y: getMainTitleY(fullLayout), - 'text-anchor': textAnchorDict[fullLayout.title.xanchor], - dy: dyDict[fullLayout.title.yanchor] + 'text-anchor': getMainTitleTextAnchor(fullLayout), + dy: getMainTitleDy(fullLayout) } }); }; @@ -504,6 +493,65 @@ function getMainTitleY(fullLayout) { } } +function getMainTitleTextAnchor(fullLayout) { + var xanchor = fullLayout.title.xanchor; + + switch(xanchor) { + case 'auto': + return calcTextAnchor(fullLayout.title.x); + case 'left': + return 'start'; + case 'right': + return 'end'; + case 'center': + default: + return 'middle'; + } + + function calcTextAnchor(x) { + if(x < 1 / 3) { + return 'start'; + } else if(x > 2 / 3) { + return 'end'; + } + + return 'middle'; + } +} + +function getMainTitleDy(fullLayout) { + var yanchor = fullLayout.title.yanchor; + switch(yanchor) { + case 'auto': + return calcDy(fullLayout.title.y); + case 'middle': + return alignmentConstants.MID_SHIFT + 'em'; + case 'top': + return alignmentConstants.CAP_SHIFT + 'em'; + case 'bottom': + default: + return '0em'; + } + + function calcDy(y) { + + // Imitate behavior before "chart title alignment" was introduced + if(y === 'auto') { + return '0em'; + } else if(typeof y === 'number') { + if(y < 1 / 3) { + return '0em'; + } else if(y > 2 / 3) { + return alignmentConstants.CAP_SHIFT + 'em'; + } else { + return alignmentConstants.MID_SHIFT + 'em'; + } + } + + return 0; + } +} + exports.doTraceStyle = function(gd) { var calcdata = gd.calcdata; var editStyleCalls = []; diff --git a/test/jasmine/tests/titles_test.js b/test/jasmine/tests/titles_test.js index 48dafdaca64..903a7ac24f6 100644 --- a/test/jasmine/tests/titles_test.js +++ b/test/jasmine/tests/titles_test.js @@ -147,6 +147,7 @@ describe('Plot title', function() { }); }); + // y 'auto' value it('provides a y \'auto\' value putting title baseline in middle ' + 'of top margin irrespective of `yref`', function() { @@ -164,9 +165,76 @@ describe('Plot title', function() { expectBaselineInMiddleOfTopMargin(gd); }); - // TODO x-anchor auto + // xanchor 'auto' test + [ + {x: -0.5, expAlignment: 'start'}, + {x: 0, expAlignment: 'start'}, + {x: 0.3, expAlignment: 'start'}, + {x: 0.4, expAlignment: 'middle'}, + {x: 0.5, expAlignment: 'middle'}, + {x: 0.6, expAlignment: 'middle'}, + {x: 0.7, expAlignment: 'end'}, + {x: 1, expAlignment: 'end'}, + {x: 1.5, expAlignment: 'end'} + ].forEach(function(testCase) { + runXAnchorAutoTest(testCase, 'container'); + runXAnchorAutoTest(testCase, 'paper'); + }); + + function runXAnchorAutoTest(testCase, xref) { + var testDesc = 'with {xanchor: \'auto\', x: ' + testCase.x + ', xref: \'' + xref + + '\'} expected to be aligned ' + testCase.expAlignment; + it(testDesc, function() { + Plotly.plot(gd, data, extendLayout({ + xref: xref, + x: testCase.x, + xanchor: 'auto' + })); + + var textAnchor = titleSel().attr('text-anchor'); + expect(textAnchor).toBe(testCase.expAlignment, testDesc); + }); + } + + // yanchor 'auto' test + // + // Note: in contrast to xanchor, there's no SVG attribute like + // text-anchor we can safely assume to work in all browsers. Thus the + // dy attribute has to be used and as a consequence it's much harder to test + // arbitrary vertical alignment options. Because of that only the + // most common use cases are tested in this regard. + [ + {y: 0, expAlignment: 'bottom', expFn: expectBaselineAlignsWithBottomEdgeOf}, + {y: 0.5, expAlignment: 'middle', expFn: expectCenteredVerticallyWithin}, + {y: 1, expAlignment: 'top', expFn: expectCapLineAlignsWithTopEdgeOf}, + ].forEach(function(testCase) { + runYAnchorAutoTest(testCase, 'container', containerElemSelector); + runYAnchorAutoTest(testCase, 'paper', paperElemSelector); + }); + + function runYAnchorAutoTest(testCase, yref, elemSelector) { + var testDesc = 'with {yanchor: \'auto\', y: ' + testCase.y + ', yref: \'' + yref + + '\'} expected to be aligned ' + testCase.expAlignment; + it(testDesc, function() { + Plotly.plot(gd, data, extendLayout({ + yref: yref, + y: testCase.y, + yanchor: 'auto' + })); + + testCase.expFn(elemSelector); + }); + } + + it('{y: \'auto\'} overrules {yanchor: \'auto\'} to support behavior ' + + 'before chart title alignment was introduced', function() { + Plotly.plot(gd, data, extendLayout({ + y: 'auto', + yanchor: 'auto' + })); - // TODO y-anchor auto + expectDefaultCenteredPosition(gd); + }); // TODO padding From ac3649b0a804b9c471836cc0e9032a2d2f7874ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=B6stl?= Date: Tue, 13 Nov 2018 15:20:49 +0100 Subject: [PATCH 08/49] Support deprecated axes titles syntax for multiple axes [882] - For initial render and update through relayout/update. --- src/plot_api/helpers.js | 35 +++++------ src/plot_api/plot_api.js | 35 ++++++----- test/jasmine/tests/titles_test.js | 101 ++++++++++++++++++++++++++++-- 3 files changed, 127 insertions(+), 44 deletions(-) diff --git a/src/plot_api/helpers.js b/src/plot_api/helpers.js index 0ec7dd10b37..e4c7e7ae121 100644 --- a/src/plot_api/helpers.js +++ b/src/plot_api/helpers.js @@ -91,6 +91,19 @@ exports.cleanLayout = function(layout) { } delete ax.autotick; } + + // title -> title.text + if(!Lib.isPlainObject(ax.title)) { + ax.title = { + text: ax.title + }; + } + + // titlefont -> title.font + if(Lib.isPlainObject(ax.titlefont) && !Lib.isPlainObject(ax.title.font)) { + ax.title.font = ax.titlefont; + delete ax.titlefont; + } } // modifications for 3D scenes @@ -176,38 +189,18 @@ exports.cleanLayout = function(layout) { } } - // Check for old-style title definitions + // Check for old-style title definition if(!Lib.isPlainObject(layout.title)) { layout.title = { text: layout.title }; } - if(layout.xaxis && !Lib.isPlainObject(layout.xaxis.title)) { - layout.xaxis.title = { - text: layout.xaxis.title - }; - } - if(layout.yaxis && !Lib.isPlainObject(layout.yaxis.title)) { - layout.yaxis.title = { - text: layout.yaxis.title - }; - } // Check for old-style title font definitions if(Lib.isPlainObject(layout.titlefont) && !Lib.isPlainObject(layout.title.font)) { layout.title.font = layout.titlefont; delete layout.titlefont; } - if(layout.xaxis && - Lib.isPlainObject(layout.xaxis.titlefont) && !Lib.isPlainObject(layout.xaxis.title.font)) { - layout.xaxis.title.font = layout.xaxis.titlefont; - delete layout.xaxis.titlefont; - } - if(layout.yaxis && - Lib.isPlainObject(layout.yaxis.titlefont) && !Lib.isPlainObject(layout.yaxis.title.font)) { - layout.yaxis.title.font = layout.yaxis.titlefont; - delete layout.yaxis.titlefont; - } /* * Moved from rotate -> orbit for dragmode diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js index 6a46e24c603..24fbe1c7207 100644 --- a/src/plot_api/plot_api.js +++ b/src/plot_api/plot_api.js @@ -1723,29 +1723,30 @@ function mapDeprecatedLayoutAttributes(layoutObj) { * @param layoutObj */ function mapDeprecatedLayoutAttributeStrings(layoutObj) { - if(layoutObj.title && !Lib.isPlainObject(layoutObj.title)) { + var oldAxisTitleRegExp = /axis\d{0,2}.title$/; + var keys, i, key, value; + + if(typeof layoutObj.title === 'string') { layoutObj.title = {text: layoutObj.title}; } - replace('xaxis.title', 'xaxis.title.text'); - replace('yaxis.title', 'yaxis.title.text'); // Note: Only "nested" (dot notation) attributes - // need to be converted. For example 'titlefont' + // need to be converted. For example 'layout.titlefont' // was a top-level attribute and thus is covered by // the general compatibility layer. - replace('titlefont.color', 'title.font.color'); - replace('titlefont.family', 'title.font.family'); - replace('titlefont.size', 'title.font.size'); - - replace('xaxis.titlefont', 'xaxis.title.font'); - replace('xaxis.titlefont.color', 'xaxis.title.font.color'); - replace('xaxis.titlefont.family', 'xaxis.title.font.family'); - replace('xaxis.titlefont.size', 'xaxis.title.font.size'); - - replace('yaxis.titlefont', 'yaxis.title.font'); - replace('yaxis.titlefont.color', 'yaxis.title.font.color'); - replace('yaxis.titlefont.family', 'yaxis.title.font.family'); - replace('yaxis.titlefont.size', 'yaxis.title.font.size'); + keys = Object.keys(layoutObj); + for(i = 0; i < keys.length; i++) { + key = keys[i]; + value = layoutObj[key]; + + if(key.indexOf('titlefont') > -1) { + replace(key, key.replace('titlefont', 'title.font')); + } + + if(typeof value === 'string' && oldAxisTitleRegExp.test(key)) { + replace(key, key.replace('title', 'title.text')); + } + } function replace(oldKey, newKey) { if(layoutObj[oldKey] !== undefined) { diff --git a/test/jasmine/tests/titles_test.js b/test/jasmine/tests/titles_test.js index 903a7ac24f6..239223b0145 100644 --- a/test/jasmine/tests/titles_test.js +++ b/test/jasmine/tests/titles_test.js @@ -656,6 +656,93 @@ describe('Title fonts can be updated', function() { } }); +describe('Titles for multiple axes', function() { + 'use strict'; + + var data = [ + {x: [1, 2, 3], y: [1, 2, 3], xaxis: 'x', yaxis: 'y'}, + {x: [1, 2, 3], y: [3, 2, 1], xaxis: 'x2', yaxis: 'y2'} + ]; + var multiAxesLayout = { + xaxis: { + title: 'X-Axis 1', + titlefont: { + size: 30 + } + }, + xaxis2: { + title: 'X-Axis 2', + titlefont: { + family: 'serif' + }, + side: 'top' + }, + yaxis: { + title: 'Y-Axis 1', + titlefont: { + family: 'Roboto' + }, + }, + yaxis2: { + title: 'Y-Axis 2', + titlefont: { + color: 'blue' + }, + side: 'right' + } + }; + var gd; + + beforeEach(function() { + gd = createGraphDiv(); + }); + + afterEach(destroyGraphDiv); + + it('still support deprecated `title` and `titlefont` syntax (backwards-compatibility)', function() { + Plotly.plot(gd, data, multiAxesLayout); + + expect(xTitleSel(1).text()).toBe('X-Axis 1'); + expect(xTitleSel(1).node().style.fontSize).toBe('30px'); + + expect(xTitleSel(2).text()).toBe('X-Axis 2'); + expect(xTitleSel(2).node().style.fontFamily).toBe('serif'); + + expect(yTitleSel(1).text()).toBe('Y-Axis 1'); + expect(yTitleSel(1).node().style.fontFamily).toBe('Roboto'); + + expect(yTitleSel(2).text()).toBe('Y-Axis 2'); + expect(yTitleSel(2).node().style.fill).toBe(rgb('blue')); + }); + + it('can be updated using deprecated `title` and `titlefont` syntax (backwards-compatibility)', function() { + Plotly.plot(gd, data, multiAxesLayout); + + Plotly.relayout(gd, { + 'xaxis2.title': '2nd X-Axis', + 'xaxis2.titlefont.color': 'pink', + 'xaxis2.titlefont.family': 'sans-serif', + 'xaxis2.titlefont.size': '14', + 'yaxis2.title': '2nd Y-Axis', + 'yaxis2.titlefont.color': 'yellow', + 'yaxis2.titlefont.family': 'monospace', + 'yaxis2.titlefont.size': '5' + }); + + var x2Style = xTitleSel(2).node().style; + expect(xTitleSel(2).text()).toBe('2nd X-Axis'); + expect(x2Style.fill).toBe(rgb('pink')); + expect(x2Style.fontFamily).toBe('sans-serif'); + expect(x2Style.fontSize).toBe('14px'); + + var y2Style = yTitleSel(2).node().style; + expect(yTitleSel(2).text()).toBe('2nd Y-Axis'); + expect(y2Style.fill).toBe(rgb('yellow')); + expect(y2Style.fontFamily).toBe('monospace'); + expect(y2Style.fontSize).toBe('5px'); + }); +}); + function expectTitle(expTitle) { expectTitleFn(expTitle)(); } @@ -762,15 +849,17 @@ function titleSel() { return titleSel; } -function xTitleSel() { - var xTitleSel = d3.select('.xtitle'); - expect(xTitleSel.empty()).toBe(false, 'X-axis title element missing'); +function xTitleSel(num) { + var axIdx = num === 1 ? '' : (num || ''); + var xTitleSel = d3.select('.x' + axIdx + 'title'); + expect(xTitleSel.empty()).toBe(false, 'X-axis ' + axIdx + ' title element missing'); return xTitleSel; } -function yTitleSel() { - var yTitleSel = d3.select('.ytitle'); - expect(yTitleSel.empty()).toBe(false, 'Y-axis title element missing'); +function yTitleSel(num) { + var axIdx = num === 1 ? '' : (num || ''); + var yTitleSel = d3.select('.y' + axIdx + 'title'); + expect(yTitleSel.empty()).toBe(false, 'Y-axis ' + axIdx + ' title element missing'); return yTitleSel; } From 493b6ec05cad966aeded9cdb2873f9432d647d1a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=B6stl?= Date: Wed, 14 Nov 2018 08:06:44 +0100 Subject: [PATCH 09/49] Adapt axes tests to new `title` structure [882] --- test/jasmine/tests/axes_test.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/test/jasmine/tests/axes_test.js b/test/jasmine/tests/axes_test.js index 595b4012fb3..d0b5685774e 100644 --- a/test/jasmine/tests/axes_test.js +++ b/test/jasmine/tests/axes_test.js @@ -345,11 +345,11 @@ describe('Test axes', function() { expect(layoutOut.xaxis.zerolinecolor).toBe(undefined); }); - it('should use \'axis.color\' as default for \'axis.titlefont.color\'', function() { + it('should use \'axis.color\' as default for \'axis.title.font.color\'', function() { layoutIn = { xaxis: { color: 'red' }, yaxis: {}, - yaxis2: { titlefont: { color: 'yellow' } } + yaxis2: { title: { font: { color: 'yellow' } } } }; layoutOut.font = { color: 'blue' }; @@ -357,9 +357,9 @@ describe('Test axes', function() { layoutOut._subplots.yaxis.push('y2'); supplyLayoutDefaults(layoutIn, layoutOut, fullData); - expect(layoutOut.xaxis.titlefont.color).toEqual('red'); - expect(layoutOut.yaxis.titlefont.color).toEqual('blue'); - expect(layoutOut.yaxis2.titlefont.color).toEqual('yellow'); + expect(layoutOut.xaxis.title.font.color).toEqual('red'); + expect(layoutOut.yaxis.title.font.color).toEqual('blue'); + expect(layoutOut.yaxis2.title.font.color).toEqual('yellow'); }); it('should use \'axis.color\' as default for \'axis.linecolor\'', function() { @@ -2882,14 +2882,14 @@ describe('Test axes', function() { expect(size.t).toBe(previousSize.t); previousSize = Lib.extendDeep({}, size); - return Plotly.relayout(gd, {'yaxis.titlefont.size': 30}); + return Plotly.relayout(gd, {'yaxis.title.font.size': 30}); }) .then(function() { var size = gd._fullLayout._size; expect(size).toEqual(previousSize); previousSize = Lib.extendDeep({}, size); - return Plotly.relayout(gd, {'yaxis.title': 'hello'}); + return Plotly.relayout(gd, {'yaxis.title.text': 'hello'}); }) .then(function() { var size = gd._fullLayout._size; From 4d7c4b329cd4583eda8b02f5ee1e8c6118164374 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=B6stl?= Date: Wed, 14 Nov 2018 08:07:56 +0100 Subject: [PATCH 10/49] Adapt axes auto-margin code to new `title` structure [882] --- src/plots/cartesian/axes.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/plots/cartesian/axes.js b/src/plots/cartesian/axes.js index 7170991727b..db040a6dac5 100644 --- a/src/plots/cartesian/axes.js +++ b/src/plots/cartesian/axes.js @@ -1825,8 +1825,8 @@ axes.drawOne = function(gd, ax, opts) { push[s] += ax._boundingBox.width; } - if(ax.title !== fullLayout._dfltTitle[axLetter]) { - push[s] += ax.titlefont.size; + if(ax.title.text !== fullLayout._dfltTitle[axLetter]) { + push[s] += ax.title.font.size; } Plots.autoMargin(gd, pushKey, push); From 96df782eaffcf026ee29579990fbc81433916c8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=B6stl?= Date: Wed, 14 Nov 2018 08:37:59 +0100 Subject: [PATCH 11/49] Adapt `axes.swap` to new `title` attr structure [882] --- src/plots/cartesian/axes.js | 8 ++++---- test/jasmine/tests/axes_test.js | 12 +++++++++--- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/src/plots/cartesian/axes.js b/src/plots/cartesian/axes.js index db040a6dac5..a69b715caf4 100644 --- a/src/plots/cartesian/axes.js +++ b/src/plots/cartesian/axes.js @@ -2640,11 +2640,11 @@ function swapAxisAttrs(layout, key, xFullAxes, yFullAxes, dfltTitle) { i; if(key === 'title') { // special handling of placeholder titles - if(xVal === dfltTitle.x) { - xVal = dfltTitle.y; + if(xVal && xVal.text === dfltTitle.x) { + xVal.text = dfltTitle.y; } - if(yVal === dfltTitle.y) { - yVal = dfltTitle.x; + if(yVal && yVal.text === dfltTitle.y) { + yVal.text = dfltTitle.x; } } diff --git a/test/jasmine/tests/axes_test.js b/test/jasmine/tests/axes_test.js index d0b5685774e..5835d6ad832 100644 --- a/test/jasmine/tests/axes_test.js +++ b/test/jasmine/tests/axes_test.js @@ -29,7 +29,9 @@ describe('Test axes', function() { data: [{x: [1, 2, 3], y: [1, 2, 3]}], layout: { xaxis: { - title: 'A Title!!!', + title: { + text: 'A Title!!!' + }, type: 'log', autorange: 'reversed', rangemode: 'tozero', @@ -42,14 +44,18 @@ describe('Test axes', function() { tickcolor: '#f00' }, yaxis: { - title: 'Click to enter Y axis title', + title: { + text: 'Click to enter Y axis title' + }, type: 'date' } } }; var expectedYaxis = Lib.extendDeep({}, gd.layout.xaxis), expectedXaxis = { - title: 'Click to enter X axis title', + title: { + text: 'Click to enter X axis title' + }, type: 'date' }; From 43da5576e43adc35496ed1b09a364a9eede32db0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=B6stl?= Date: Tue, 13 Nov 2018 11:03:48 +0100 Subject: [PATCH 12/49] Adapt colorbar to new title attr structure [882] - colorbar is using the titles component to draw its title and therefore needed to adapt to the attribute structure assumed by the titles component. --- src/components/colorbar/draw.js | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/components/colorbar/draw.js b/src/components/colorbar/draw.js index c30d6078f5f..34e89aa120a 100644 --- a/src/components/colorbar/draw.js +++ b/src/components/colorbar/draw.js @@ -183,7 +183,15 @@ module.exports = function draw(gd, id) { tickprefix: opts.tickprefix, showticksuffix: opts.showticksuffix, ticksuffix: opts.ticksuffix, - title: opts.title, + + // Plot and axes titles have a different, nested attribute structure + // for defining title attributes. Since the `titles` component + // assumes that nested structure, let's adapt to it without breaking + // the existing colorbar API. + title: { + text: opts.title, + font: opts.titlefont + }, showline: true, anchor: 'free', side: 'right', From 9e2cbfe21ff0c51f09ba0ed5ef5416f1bda20a71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=B6stl?= Date: Wed, 14 Nov 2018 13:33:42 +0100 Subject: [PATCH 13/49] Adapt rangeslider to new title attr structure [882] --- src/components/rangeslider/draw.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/rangeslider/draw.js b/src/components/rangeslider/draw.js index e1d0b9f609c..cd8b404d0f4 100644 --- a/src/components/rangeslider/draw.js +++ b/src/components/rangeslider/draw.js @@ -172,7 +172,7 @@ module.exports = function(gd) { placeholder: fullLayout._dfltTitle.x, attributes: { x: axisOpts._offset + axisOpts._length / 2, - y: y + opts._height + opts._offsetShift + 10 + 1.5 * axisOpts.titlefont.size, + y: y + opts._height + opts._offsetShift + 10 + 1.5 * axisOpts.title.font.size, 'text-anchor': 'middle' } }); From f632a7d11302e17a3ac34b12fc172271c6b8437e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=B6stl?= Date: Thu, 15 Nov 2018 09:20:29 +0100 Subject: [PATCH 14/49] Transition polar plot type to new title attr structure [882] - Adapt layout attributes. - Extend compatibility code in `cleanLayout`. - Adapt polar drawing code. - Adapt tests being based on old title attr structure. --- src/plot_api/helpers.js | 26 ++++++++++++++++++++++++++ src/plots/polar/layout_attributes.js | 3 +-- src/plots/polar/layout_defaults.js | 4 ++-- src/plots/polar/polar.js | 5 ++++- test/jasmine/tests/polar_test.js | 2 +- 5 files changed, 34 insertions(+), 6 deletions(-) diff --git a/src/plot_api/helpers.js b/src/plot_api/helpers.js index e4c7e7ae121..d3c022bc9ba 100644 --- a/src/plot_api/helpers.js +++ b/src/plot_api/helpers.js @@ -53,6 +53,7 @@ exports.cleanLayout = function(layout) { } var axisAttrRegex = (Plots.subplotsRegistry.cartesian || {}).attrRegex; + var polarAttrRegex = (Plots.subplotsRegistry.polar || {}).attrRegex; var sceneAttrRegex = (Plots.subplotsRegistry.gl3d || {}).attrRegex; var keys = Object.keys(layout); @@ -92,6 +93,9 @@ exports.cleanLayout = function(layout) { delete ax.autotick; } + // TODO Should this be done with typeof as well (see radialaxes)? + // At least code would be more obvious! + // title -> title.text if(!Lib.isPlainObject(ax.title)) { ax.title = { @@ -106,6 +110,28 @@ exports.cleanLayout = function(layout) { } } + // modifications for radial axes + else if(polarAttrRegex && polarAttrRegex.test(key)) { + var polar = layout[key]; + + if(polar.radialaxis) { + + // title -> title.text + if(typeof polar.radialaxis.title === 'string') { + polar.radialaxis.title = { + text: polar.radialaxis.title + }; + } + + // titlefont -> title.font + if(Lib.isPlainObject(polar.radialaxis.titlefont) && + !Lib.isPlainObject(polar.radialaxis.title.font)) { + polar.radialaxis.title.font = polar.radialaxis.titlefont; + delete polar.radialaxis.titlefont; + } + } + } + // modifications for 3D scenes else if(sceneAttrRegex && sceneAttrRegex.test(key)) { var scene = layout[key]; diff --git a/src/plots/polar/layout_attributes.js b/src/plots/polar/layout_attributes.js index 8334b0c3d96..36340346e50 100644 --- a/src/plots/polar/layout_attributes.js +++ b/src/plots/polar/layout_attributes.js @@ -112,8 +112,7 @@ var radialAxisAttrs = { }, - title: extendFlat({}, axesAttrs.title, {editType: 'plot', dflt: ''}), - titlefont: overrideAll(axesAttrs.titlefont, 'plot', 'from-root'), + title: overrideAll(axesAttrs.title, 'plot', 'from-root'), // might need a 'titleside' and even 'titledirection' down the road hoverformat: axesAttrs.hoverformat, diff --git a/src/plots/polar/layout_defaults.js b/src/plots/polar/layout_defaults.js index 32bf5afff4b..69c031acef3 100644 --- a/src/plots/polar/layout_defaults.js +++ b/src/plots/polar/layout_defaults.js @@ -100,8 +100,8 @@ function handleDefaults(contIn, contOut, coerce, opts) { coerceAxis('side'); coerceAxis('angle', sector[0]); - coerceAxis('title'); - Lib.coerceFont(coerceAxis, 'titlefont', { + coerceAxis('title.text'); + Lib.coerceFont(coerceAxis, 'title.font', { family: opts.font.family, size: Math.round(opts.font.size * 1.2), color: dfltFontColor diff --git a/src/plots/polar/polar.js b/src/plots/polar/polar.js index d36c10e7b65..5f501f4c775 100644 --- a/src/plots/polar/polar.js +++ b/src/plots/polar/polar.js @@ -483,9 +483,12 @@ proto.updateRadialAxisTitle = function(fullLayout, polarLayout, _angle) { var sina = Math.sin(angleRad); var pad = 0; + + // Hint: no need to check if there is in fact a title.text set + // because if plot is editable, pad needs to be calculated anyways. if(radialLayout.title) { var h = Drawing.bBox(_this.layers['radial-axis'].node()).height; - var ts = radialLayout.titlefont.size; + var ts = radialLayout.title.font.size; pad = radialLayout.side === 'counterclockwise' ? -h - ts * 0.4 : h + ts * 0.8; diff --git a/test/jasmine/tests/polar_test.js b/test/jasmine/tests/polar_test.js index be0325be62b..2d48c4afd1f 100644 --- a/test/jasmine/tests/polar_test.js +++ b/test/jasmine/tests/polar_test.js @@ -134,7 +134,7 @@ describe('Test polar plots defaults:', function() { expect(layoutOut.polar.angularaxis.linecolor).toBe('red'); expect(layoutOut.polar.angularaxis.gridcolor).toBe('rgb(255, 153, 153)', 'blend by 60% with bgcolor'); - expect(layoutOut.polar.radialaxis.titlefont.color).toBe('blue'); + expect(layoutOut.polar.radialaxis.title.font.color).toBe('blue'); expect(layoutOut.polar.radialaxis.linecolor).toBe('blue'); expect(layoutOut.polar.radialaxis.gridcolor).toBe('rgb(153, 153, 255)', 'blend by 60% with bgcolor'); }); From b3367f17af88b4324771211b5858f612476d3a4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=B6stl?= Date: Thu, 15 Nov 2018 09:45:04 +0100 Subject: [PATCH 15/49] Adapt legacy polar chart to new title attr structure [882] --- src/plot_api/plot_api.js | 8 ++++---- src/plots/polar/legacy/micropolar.js | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js index 24fbe1c7207..110c81e5abc 100644 --- a/src/plot_api/plot_api.js +++ b/src/plot_api/plot_api.js @@ -157,7 +157,7 @@ exports.plot = function(gd, data, layout, config) { // Legacy polar plots if(!fullLayout._has('polar') && data && data[0] && data[0].r) { Lib.log('Legacy polar charts are deprecated!'); - return plotPolar(gd, data, layout); + return plotLegacyPolar(gd, data, layout); } // so we don't try to re-call Plotly.plot from inside @@ -494,7 +494,7 @@ function setPlotContext(gd, config) { context._hasZeroWidth = context._hasZeroWidth || gd.clientWidth === 0; } -function plotPolar(gd, data, layout) { +function plotLegacyPolar(gd, data, layout) { // build or reuse the container skeleton var plotContainer = d3.select(gd).selectAll('.plot-container') .data([0]); @@ -535,7 +535,7 @@ function plotPolar(gd, data, layout) { // editable title var opacity = 1; - var txt = gd._fullLayout.title; + var txt = gd._fullLayout.title ? gd._fullLayout.title.text : ''; if(txt === '' || !txt) opacity = 0; var titleLayout = function() { @@ -569,7 +569,7 @@ function plotPolar(gd, data, layout) { var setContenteditable = function() { this.call(svgTextUtils.makeEditable, {gd: gd}) .on('edit', function(text) { - gd.framework({layout: {title: text}}); + gd.framework({layout: {title: {text: text}}}); this.text(text) .call(titleLayout); this.call(setContenteditable); diff --git a/src/plots/polar/legacy/micropolar.js b/src/plots/polar/legacy/micropolar.js index eba2a693341..fc94cc39170 100644 --- a/src/plots/polar/legacy/micropolar.js +++ b/src/plots/polar/legacy/micropolar.js @@ -215,8 +215,8 @@ var µ = module.exports = { version: '0.2.2' }; centeringOffset[0] = Math.max(0, centeringOffset[0]); centeringOffset[1] = Math.max(0, centeringOffset[1]); svg.select('.outer-group').attr('transform', 'translate(' + centeringOffset + ')'); - if (axisConfig.title) { - var title = svg.select('g.title-group text').style(fontStyle).text(axisConfig.title); + if (axisConfig.title && axisConfig.title.text) { + var title = svg.select('g.title-group text').style(fontStyle).text(axisConfig.title.text); var titleBBox = title.node().getBBox(); title.attr({ x: chartCenter[0] - titleBBox.width / 2, From cf7f4938423753dcadff8a52d8843e0ee88e8fb3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=B6stl?= Date: Thu, 15 Nov 2018 11:19:24 +0100 Subject: [PATCH 16/49] Transition ternary plot type to new title attr structure [882] - Adapt layout attributes. - Extend compatibility code in `cleanLayout`. - Adapt ternary drawing code. - Adapt tests being based on old title attr structure. - Test relayouting title attributes. --- src/components/titles/index.js | 2 +- src/plot_api/helpers.js | 35 +++++++++++++++- src/plots/ternary/layout_attributes.js | 1 - src/plots/ternary/layout_defaults.js | 4 +- src/plots/ternary/ternary.js | 6 +-- test/jasmine/tests/ternary_test.js | 56 ++++++++++++++++++++++++-- 6 files changed, 93 insertions(+), 11 deletions(-) diff --git a/src/components/titles/index.js b/src/components/titles/index.js index d596044c08d..9315343bf60 100644 --- a/src/components/titles/index.js +++ b/src/components/titles/index.js @@ -81,7 +81,7 @@ function draw(gd, titleClass, options) { // only make this title editable if we positively identify its property // as one that has editing enabled. var editAttr; - // TODO 882 Probably compare against 'title.text' + // TODO 882 Probably compare against 'title.text' and may also adapt calling code? if(prop === 'title') editAttr = 'titleText'; else if(prop.indexOf('axis') !== -1) editAttr = 'axisTitleText'; else if(prop.indexOf('colorbar' !== -1)) editAttr = 'colorbarTitleText'; diff --git a/src/plot_api/helpers.js b/src/plot_api/helpers.js index d3c022bc9ba..664d75ec31d 100644 --- a/src/plot_api/helpers.js +++ b/src/plot_api/helpers.js @@ -34,7 +34,7 @@ exports.clearPromiseQueue = function(gd) { // before it gets used for anything // backward compatibility and cleanup of nonstandard options exports.cleanLayout = function(layout) { - var i, j; + var i, j, k; if(!layout) layout = {}; @@ -54,6 +54,7 @@ exports.cleanLayout = function(layout) { var axisAttrRegex = (Plots.subplotsRegistry.cartesian || {}).attrRegex; var polarAttrRegex = (Plots.subplotsRegistry.polar || {}).attrRegex; + var ternaryAttrRegex = (Plots.subplotsRegistry.ternary || {}).attrRegex; var sceneAttrRegex = (Plots.subplotsRegistry.gl3d || {}).attrRegex; var keys = Object.keys(layout); @@ -132,6 +133,38 @@ exports.cleanLayout = function(layout) { } } + // modifications for ternary + else if(ternaryAttrRegex && ternaryAttrRegex.test(key)) { + var ternary = layout[key]; + + var ternaryKeys = Object.keys(ternary); + for(k = 0; k < ternaryKeys.length; k++) { + var ternaryKey = ternaryKeys[k]; + if( + ternaryKey === 'aaxis' || + ternaryKey === 'baxis' || + ternaryKey === 'caxis' + ) { + var ternaryAxis = ternary[ternaryKey]; + + // TODO 882 DRY that up with polar, axes and main title + // title -> title.text + if(typeof ternaryAxis.title === 'string') { + ternaryAxis.title = { + text: ternaryAxis.title + }; + } + + // titlefont -> title.font + if(Lib.isPlainObject(ternaryAxis.titlefont) && + !Lib.isPlainObject(ternaryAxis.title.font)) { + ternaryAxis.title.font = ternaryAxis.titlefont; + delete ternaryAxis.titlefont; + } + } + } + } + // modifications for 3D scenes else if(sceneAttrRegex && sceneAttrRegex.test(key)) { var scene = layout[key]; diff --git a/src/plots/ternary/layout_attributes.js b/src/plots/ternary/layout_attributes.js index 72397f0f876..606765e8c94 100644 --- a/src/plots/ternary/layout_attributes.js +++ b/src/plots/ternary/layout_attributes.js @@ -17,7 +17,6 @@ var extendFlat = require('../../lib/extend').extendFlat; var ternaryAxesAttrs = { title: axesAttrs.title, - titlefont: axesAttrs.titlefont, color: axesAttrs.color, // ticks tickmode: axesAttrs.tickmode, diff --git a/src/plots/ternary/layout_defaults.js b/src/plots/ternary/layout_defaults.js index 7952a4559a5..0ccbbd2dfd8 100644 --- a/src/plots/ternary/layout_defaults.js +++ b/src/plots/ternary/layout_defaults.js @@ -83,10 +83,10 @@ function handleAxisDefaults(containerIn, containerOut, options) { letterUpper = axName.charAt(0).toUpperCase(), dfltTitle = 'Component ' + letterUpper; - var title = coerce('title', dfltTitle); + var title = coerce('title.text', dfltTitle); containerOut._hovertitle = title === dfltTitle ? title : letterUpper; - Lib.coerceFont(coerce, 'titlefont', { + Lib.coerceFont(coerce, 'title.font', { family: options.font.family, size: Math.round(options.font.size * 1.2), color: dfltFontColor diff --git a/src/plots/ternary/ternary.js b/src/plots/ternary/ternary.js index 75f3a17c996..f8bc991349c 100644 --- a/src/plots/ternary/ternary.js +++ b/src/plots/ternary/ternary.js @@ -375,7 +375,7 @@ proto.drawAxes = function(doTitles) { placeholder: _(gd, 'Click to enter Component A title'), attributes: { x: _this.x0 + _this.w / 2, - y: _this.y0 - aaxis.titlefont.size / 3 - apad, + y: _this.y0 - aaxis.title.font.size / 3 - apad, 'text-anchor': 'middle' } }); @@ -385,7 +385,7 @@ proto.drawAxes = function(doTitles) { placeholder: _(gd, 'Click to enter Component B title'), attributes: { x: _this.x0 - bpad, - y: _this.y0 + _this.h + baxis.titlefont.size * 0.83 + bpad, + y: _this.y0 + _this.h + baxis.title.font.size * 0.83 + bpad, 'text-anchor': 'middle' } }); @@ -395,7 +395,7 @@ proto.drawAxes = function(doTitles) { placeholder: _(gd, 'Click to enter Component C title'), attributes: { x: _this.x0 + _this.w + bpad, - y: _this.y0 + _this.h + caxis.titlefont.size * 0.83 + bpad, + y: _this.y0 + _this.h + caxis.title.font.size * 0.83 + bpad, 'text-anchor': 'middle' } }); diff --git a/test/jasmine/tests/ternary_test.js b/test/jasmine/tests/ternary_test.js index f202749ed3b..7d209b41f8d 100644 --- a/test/jasmine/tests/ternary_test.js +++ b/test/jasmine/tests/ternary_test.js @@ -1,5 +1,6 @@ var Plotly = require('@lib'); var Lib = require('@src/lib'); +var rgb = require('@src/components/color').rgb; var supplyLayoutDefaults = require('@src/plots/ternary/layout_defaults'); @@ -382,6 +383,55 @@ describe('ternary plots', function() { .then(done); }); + it('should be able to relayout axis title attributes', function(done) { + var gd = createGraphDiv(); + var fig = Lib.extendDeep({}, require('@mocks/ternary_simple.json')); + + function _assert(axisPrefix, title, family, color, size) { + var titleSel = d3.select('.' + axisPrefix + 'title'); + var titleNode = titleSel.node(); + + var msg = 'for ' + axisPrefix + 'axis title'; + expect(titleSel.text()).toBe(title, 'title ' + msg); + expect(titleNode.style['font-family']).toBe(family, 'font family ' + msg); + expect(parseFloat(titleNode.style['font-size'])).toBe(size, 'font size ' + msg); + expect(titleNode.style.fill).toBe(color, 'font color ' + msg); + } + + Plotly.plot(gd, fig).then(function() { + _assert('a', 'Component A', '"Open Sans", verdana, arial, sans-serif', rgb('#ccc'), 14); + _assert('b', 'chocolate', '"Open Sans", verdana, arial, sans-serif', rgb('#0f0'), 14); + _assert('c', 'Component C', '"Open Sans", verdana, arial, sans-serif', rgb('#444'), 14); + + // Note: Different update notations to also test legacy title structures + return Plotly.relayout(gd, { + 'ternary.aaxis.title.text': 'chips', + 'ternary.aaxis.title.font.color': 'yellow', + 'ternary.aaxis.titlefont.family': 'monospace', + 'ternary.aaxis.titlefont.size': 16, + 'ternary.baxis.title': 'white chocolate', + 'ternary.baxis.title.font.color': 'blue', + 'ternary.baxis.titlefont.family': 'sans-serif', + 'ternary.baxis.titlefont.size': 10, + 'ternary.caxis.title': { + text: 'candy', + font: { + color: 'pink', + family: 'serif', + size: 30 + } + } + }); + }) + .then(function() { + _assert('a', 'chips', 'monospace', rgb('yellow'), 16); + _assert('b', 'white chocolate', 'sans-serif', rgb('blue'), 10); + _assert('c', 'candy', 'serif', rgb('pink'), 30); + }) + .catch(failTest) + .then(done); + }); + it('should be able to hide/show ticks and tick labels', function(done) { var gd = createGraphDiv(); var fig = Lib.extendDeep({}, require('@mocks/ternary_simple.json')); @@ -546,9 +596,9 @@ describe('ternary defaults', function() { layoutIn = {}; supplyLayoutDefaults(layoutIn, layoutOut, fullData); - expect(layoutOut.ternary.aaxis.title).toEqual('Component A'); - expect(layoutOut.ternary.baxis.title).toEqual('Component B'); - expect(layoutOut.ternary.caxis.title).toEqual('Component C'); + expect(layoutOut.ternary.aaxis.title.text).toEqual('Component A'); + expect(layoutOut.ternary.baxis.title.text).toEqual('Component B'); + expect(layoutOut.ternary.caxis.title.text).toEqual('Component C'); }); it('should default \'gricolor\' to 60% dark', function() { From 573c90dc674c497ec0aead08af86eda75bdb7f32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=B6stl?= Date: Thu, 15 Nov 2018 12:40:34 +0100 Subject: [PATCH 17/49] Deduplicate transitioning to new title structure in `cleanLayout` [882] --- src/plot_api/helpers.js | 98 +++++++++++------------------------------ 1 file changed, 26 insertions(+), 72 deletions(-) diff --git a/src/plot_api/helpers.js b/src/plot_api/helpers.js index 664d75ec31d..d76c9b5f419 100644 --- a/src/plot_api/helpers.js +++ b/src/plot_api/helpers.js @@ -34,7 +34,7 @@ exports.clearPromiseQueue = function(gd) { // before it gets used for anything // backward compatibility and cleanup of nonstandard options exports.cleanLayout = function(layout) { - var i, j, k; + var i, j; if(!layout) layout = {}; @@ -94,75 +94,21 @@ exports.cleanLayout = function(layout) { delete ax.autotick; } - // TODO Should this be done with typeof as well (see radialaxes)? - // At least code would be more obvious! - - // title -> title.text - if(!Lib.isPlainObject(ax.title)) { - ax.title = { - text: ax.title - }; - } - - // titlefont -> title.font - if(Lib.isPlainObject(ax.titlefont) && !Lib.isPlainObject(ax.title.font)) { - ax.title.font = ax.titlefont; - delete ax.titlefont; - } + cleanTitle(ax); } - // modifications for radial axes + // modifications for polar else if(polarAttrRegex && polarAttrRegex.test(key)) { var polar = layout[key]; - - if(polar.radialaxis) { - - // title -> title.text - if(typeof polar.radialaxis.title === 'string') { - polar.radialaxis.title = { - text: polar.radialaxis.title - }; - } - - // titlefont -> title.font - if(Lib.isPlainObject(polar.radialaxis.titlefont) && - !Lib.isPlainObject(polar.radialaxis.title.font)) { - polar.radialaxis.title.font = polar.radialaxis.titlefont; - delete polar.radialaxis.titlefont; - } - } + cleanTitle(polar.radialaxis); } // modifications for ternary else if(ternaryAttrRegex && ternaryAttrRegex.test(key)) { var ternary = layout[key]; - - var ternaryKeys = Object.keys(ternary); - for(k = 0; k < ternaryKeys.length; k++) { - var ternaryKey = ternaryKeys[k]; - if( - ternaryKey === 'aaxis' || - ternaryKey === 'baxis' || - ternaryKey === 'caxis' - ) { - var ternaryAxis = ternary[ternaryKey]; - - // TODO 882 DRY that up with polar, axes and main title - // title -> title.text - if(typeof ternaryAxis.title === 'string') { - ternaryAxis.title = { - text: ternaryAxis.title - }; - } - - // titlefont -> title.font - if(Lib.isPlainObject(ternaryAxis.titlefont) && - !Lib.isPlainObject(ternaryAxis.title.font)) { - ternaryAxis.title.font = ternaryAxis.titlefont; - delete ternaryAxis.titlefont; - } - } - } + cleanTitle(ternary.aaxis); + cleanTitle(ternary.baxis); + cleanTitle(ternary.caxis); } // modifications for 3D scenes @@ -249,17 +195,7 @@ exports.cleanLayout = function(layout) { } // Check for old-style title definition - if(!Lib.isPlainObject(layout.title)) { - layout.title = { - text: layout.title - }; - } - - // Check for old-style title font definitions - if(Lib.isPlainObject(layout.titlefont) && !Lib.isPlainObject(layout.title.font)) { - layout.title.font = layout.titlefont; - delete layout.titlefont; - } + cleanTitle(layout); /* * Moved from rotate -> orbit for dragmode @@ -281,6 +217,24 @@ function cleanAxRef(container, attr) { } } +function cleanTitle(titleContainer) { + if(titleContainer) { + if(typeof titleContainer.title === 'string') { + titleContainer.title = { + text: titleContainer.title + }; + } + + // titlefont -> title.font + if(titleContainer.title && + Lib.isPlainObject(titleContainer.titlefont) && + !Lib.isPlainObject(titleContainer.title.font)) { + titleContainer.title.font = titleContainer.titlefont; + delete titleContainer.titlefont; + } + } +} + /* * cleanData: Make a few changes to the data for backward compatibility * before it gets used for anything. Modifies the data traces users provide. From ddf9c0bb77d87a93f6606abc4251886e14b6b57b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=B6stl?= Date: Thu, 15 Nov 2018 15:29:28 +0100 Subject: [PATCH 18/49] Transition gl3d plot type to new title attr structure [882] - Adapt layout attributes. - Extend compatibility code in `cleanLayout`. --- src/plot_api/helpers.js | 5 +++++ src/plots/gl3d/layout/axis_attributes.js | 1 - src/plots/gl3d/layout/axis_defaults.js | 10 +++++++++- src/plots/gl3d/layout/convert.js | 10 +++++----- 4 files changed, 19 insertions(+), 7 deletions(-) diff --git a/src/plot_api/helpers.js b/src/plot_api/helpers.js index d76c9b5f419..441e433eb69 100644 --- a/src/plot_api/helpers.js +++ b/src/plot_api/helpers.js @@ -137,6 +137,11 @@ exports.cleanLayout = function(layout) { delete scene.cameraposition; } + + // clean axis titles + cleanTitle(scene.xaxis); + cleanTitle(scene.yaxis); + cleanTitle(scene.zaxis); } } diff --git a/src/plots/gl3d/layout/axis_attributes.js b/src/plots/gl3d/layout/axis_attributes.js index 78309a1deb5..476ba93a82d 100644 --- a/src/plots/gl3d/layout/axis_attributes.js +++ b/src/plots/gl3d/layout/axis_attributes.js @@ -73,7 +73,6 @@ module.exports = overrideAll({ categoryorder: axesAttrs.categoryorder, categoryarray: axesAttrs.categoryarray, title: axesAttrs.title, - titlefont: axesAttrs.titlefont, type: axesAttrs.type, autorange: axesAttrs.autorange, rangemode: axesAttrs.rangemode, diff --git a/src/plots/gl3d/layout/axis_defaults.js b/src/plots/gl3d/layout/axis_defaults.js index e28b1a74de4..3a2a0531c87 100644 --- a/src/plots/gl3d/layout/axis_defaults.js +++ b/src/plots/gl3d/layout/axis_defaults.js @@ -56,7 +56,15 @@ module.exports = function supplyLayoutDefaults(layoutIn, layoutOut, options) { options.fullLayout); coerce('gridcolor', colorMix(containerOut.color, options.bgColor, gridLightness).toRgbString()); - coerce('title', axName[0]); // shouldn't this be on-par with 2D? + coerce('title.text', axName[0]); // shouldn't this be on-par with 2D? + + // TODO 882 coercing old 'titlefont' was missing. Why? Activating the below would break a lot image tests... + // Lib.coerceFont(coerce, 'title.font', { + // family: options.font.family, + // size: Math.round(options.font.size), + // color: options.font.color + // }); + containerOut.setScale = Lib.noop; diff --git a/src/plots/gl3d/layout/convert.js b/src/plots/gl3d/layout/convert.js index 8aded799e13..89ebecdc79d 100644 --- a/src/plots/gl3d/layout/convert.js +++ b/src/plots/gl3d/layout/convert.js @@ -83,11 +83,11 @@ proto.merge = function(sceneLayout) { } // Axes labels - opts.labels[i] = axes.title; - if('titlefont' in axes) { - if(axes.titlefont.color) opts.labelColor[i] = str2RgbaArray(axes.titlefont.color); - if(axes.titlefont.family) opts.labelFont[i] = axes.titlefont.family; - if(axes.titlefont.size) opts.labelSize[i] = axes.titlefont.size; + opts.labels[i] = axes.title.text; + if('title' in axes && 'font' in axes.title) { + if(axes.title.font.color) opts.labelColor[i] = str2RgbaArray(axes.title.font.color); + if(axes.title.font.family) opts.labelFont[i] = axes.title.font.family; + if(axes.title.font.size) opts.labelSize[i] = axes.title.font.size; } // Lines From 0e144b63a486c2a9875b72ca7eacda7f45ae8b3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=B6stl?= Date: Thu, 15 Nov 2018 15:42:52 +0100 Subject: [PATCH 19/49] Transition gl2d plot type to new title attr structure [882] --- src/plots/gl2d/convert.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/plots/gl2d/convert.js b/src/plots/gl2d/convert.js index 43ab172a845..e8ba2054732 100644 --- a/src/plots/gl2d/convert.js +++ b/src/plots/gl2d/convert.js @@ -113,14 +113,14 @@ proto.merge = function(options) { // '_name' is e.g. xaxis, xaxis2, yaxis, yaxis4 ... ax = options[this.scene[axisName]._name]; - axTitle = ax.title === this.scene.fullLayout._dfltTitle[axisLetter] ? '' : ax.title; + axTitle = ax.title.text === this.scene.fullLayout._dfltTitle[axisLetter] ? '' : ax.title.text; for(j = 0; j <= 2; j += 2) { this.labelEnable[i + j] = false; this.labels[i + j] = axTitle; - this.labelColor[i + j] = str2RGBArray(ax.titlefont.color); - this.labelFont[i + j] = ax.titlefont.family; - this.labelSize[i + j] = ax.titlefont.size; + this.labelColor[i + j] = str2RGBArray(ax.title.font.color); + this.labelFont[i + j] = ax.title.font.family; + this.labelSize[i + j] = ax.title.font.size; this.labelPad[i + j] = this.getLabelPad(axisName, ax); this.tickEnable[i + j] = false; @@ -208,7 +208,7 @@ proto.hasAxisInAltrPos = function(axisName, ax) { proto.getLabelPad = function(axisName, ax) { var offsetBase = 1.5, - fontSize = ax.titlefont.size, + fontSize = ax.title.font.size, showticklabels = ax.showticklabels; if(axisName === 'xaxis') { From 984aa46a588dd892ad01b3665326f1998c42c9ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=B6stl?= Date: Thu, 15 Nov 2018 16:57:44 +0100 Subject: [PATCH 20/49] Expect new title syntax in legend, lib and further tests [882] - Further being splom, validate and template tests. --- test/image/mocks/shapes.json | 8 +++--- test/jasmine/tests/legend_test.js | 4 +-- test/jasmine/tests/lib_test.js | 4 +-- test/jasmine/tests/splom_test.js | 44 ++++++++++++++--------------- test/jasmine/tests/template_test.js | 4 ++- test/jasmine/tests/validate_test.js | 8 ++++-- 6 files changed, 39 insertions(+), 33 deletions(-) diff --git a/test/image/mocks/shapes.json b/test/image/mocks/shapes.json index 2aadf3f0020..f98fcc6b843 100644 --- a/test/image/mocks/shapes.json +++ b/test/image/mocks/shapes.json @@ -12,10 +12,10 @@ "yaxis":"y2" }], "layout": { - "xaxis":{"title":"linear","range":[0,1],"domain":[0.35,0.65],"type":"linear","showgrid":false,"zeroline":false,"showticklabels":false}, - "yaxis":{"title":"log","range":[0,1],"domain":[0.6,1],"type":"log","showgrid":false,"zeroline":false,"showticklabels":false}, - "xaxis2":{"title":"date","range":["2000-01-01","2000-01-02"],"domain":[0.7,1],"anchor":"y2","type":"date","showgrid":false,"zeroline":false,"showticklabels":false}, - "yaxis2":{"title":"category","range":[0,1],"domain":[0.6,1],"anchor":"x2","type":"category","showgrid":false,"zeroline":false,"showticklabels":false}, + "xaxis":{"title":{"text":"linear"},"range":[0,1],"domain":[0.35,0.65],"type":"linear","showgrid":false,"zeroline":false,"showticklabels":false}, + "yaxis":{"title":{"text":"log"},"range":[0,1],"domain":[0.6,1],"type":"log","showgrid":false,"zeroline":false,"showticklabels":false}, + "xaxis2":{"title":{"text":"date"},"range":["2000-01-01","2000-01-02"],"domain":[0.7,1],"anchor":"y2","type":"date","showgrid":false,"zeroline":false,"showticklabels":false}, + "yaxis2":{"title":{"text":"category"},"range":[0,1],"domain":[0.6,1],"anchor":"x2","type":"category","showgrid":false,"zeroline":false,"showticklabels":false}, "height":400, "width":800, "margin": {"l":20,"r":20,"pad":0}, diff --git a/test/jasmine/tests/legend_test.js b/test/jasmine/tests/legend_test.js index 930fa8cd236..8f04f0b7918 100644 --- a/test/jasmine/tests/legend_test.js +++ b/test/jasmine/tests/legend_test.js @@ -1431,7 +1431,7 @@ describe('legend interaction', function() { }); gd.on('plotly_relayout', function(d) { expect(typeof d).toBe('object'); - expect(d.title).toBe('just clicked on trace #2'); + expect(d.title.text).toBe('just clicked on trace #2'); done(); }); gd.on('plotly_restyle', function() { @@ -1451,7 +1451,7 @@ describe('legend interaction', function() { }); gd.on('plotly_relayout', function(d) { expect(typeof d).toBe('object'); - expect(d.title).toBe('just double clicked on trace #0'); + expect(d.title.text).toBe('just double clicked on trace #0'); done(); }); gd.on('plotly_restyle', function() { diff --git a/test/jasmine/tests/lib_test.js b/test/jasmine/tests/lib_test.js index c3e2e6ebb6e..6e4387fddf7 100644 --- a/test/jasmine/tests/lib_test.js +++ b/test/jasmine/tests/lib_test.js @@ -2658,7 +2658,7 @@ describe('Queue', function() { .then(function() { expect(gd.undoQueue.index).toEqual(2); expect(gd.undoQueue.queue[1].undo.args[0][1].title).toEqual(null); - expect(gd.undoQueue.queue[1].redo.args[0][1].title).toEqual('A title'); + expect(gd.undoQueue.queue[1].redo.args[0][1].title.text).toEqual('A title'); return Plotly.restyle(gd, 'mode', 'markers'); }) @@ -2670,7 +2670,7 @@ describe('Queue', function() { expect(gd.undoQueue.queue[1].redo.args[0][1].mode).toEqual('markers'); expect(gd.undoQueue.queue[0].undo.args[0][1].title).toEqual(null); - expect(gd.undoQueue.queue[0].redo.args[0][1].title).toEqual('A title'); + expect(gd.undoQueue.queue[0].redo.args[0][1].title.text).toEqual('A title'); return Plotly.restyle(gd, 'transforms[0]', { type: 'filter' }); }) diff --git a/test/jasmine/tests/splom_test.js b/test/jasmine/tests/splom_test.js index f1dfb13523b..3c4d81911b3 100644 --- a/test/jasmine/tests/splom_test.js +++ b/test/jasmine/tests/splom_test.js @@ -325,10 +325,10 @@ describe('Test splom trace defaults:', function() { }); var fullLayout = gd._fullLayout; - expect(fullLayout.xaxis.title).toBe('A'); - expect(fullLayout.yaxis.title).toBe('A'); - expect(fullLayout.xaxis2.title).toBe('B'); - expect(fullLayout.yaxis2.title).toBe('B'); + expect(fullLayout.xaxis.title.text).toBe('A'); + expect(fullLayout.yaxis.title.text).toBe('A'); + expect(fullLayout.xaxis2.title.text).toBe('B'); + expect(fullLayout.yaxis2.title.text).toBe('B'); }); it('should set axis title default using dimensions *label* (even visible false dimensions)', function() { @@ -346,12 +346,12 @@ describe('Test splom trace defaults:', function() { }); var fullLayout = gd._fullLayout; - expect(fullLayout.xaxis.title).toBe('A'); - expect(fullLayout.yaxis.title).toBe('A'); - expect(fullLayout.xaxis2.title).toBe('B'); - expect(fullLayout.yaxis2.title).toBe('B'); - expect(fullLayout.xaxis3.title).toBe('C'); - expect(fullLayout.yaxis3.title).toBe('C'); + expect(fullLayout.xaxis.title.text).toBe('A'); + expect(fullLayout.yaxis.title.text).toBe('A'); + expect(fullLayout.xaxis2.title.text).toBe('B'); + expect(fullLayout.yaxis2.title.text).toBe('B'); + expect(fullLayout.xaxis3.title.text).toBe('C'); + expect(fullLayout.yaxis3.title.text).toBe('C'); }); it('should ignore (x|y)axes values beyond dimensions length', function() { @@ -382,12 +382,12 @@ describe('Test splom trace defaults:', function() { 'x2y', 'x2y2', 'x2y3', 'x3y', 'x3y2', 'x3y3' ]); - expect(fullLayout.xaxis.title).toBe('A'); - expect(fullLayout.yaxis.title).toBe('A'); - expect(fullLayout.xaxis2.title).toBe('B'); - expect(fullLayout.yaxis2.title).toBe('B'); - expect(fullLayout.xaxis3.title).toBe('C'); - expect(fullLayout.yaxis3.title).toBe('C'); + expect(fullLayout.xaxis.title.text).toBe('A'); + expect(fullLayout.yaxis.title.text).toBe('A'); + expect(fullLayout.xaxis2.title.text).toBe('B'); + expect(fullLayout.yaxis2.title.text).toBe('B'); + expect(fullLayout.xaxis3.title.text).toBe('C'); + expect(fullLayout.yaxis3.title.text).toBe('C'); expect(fullLayout.xaxis4).toBe(undefined); expect(fullLayout.yaxis4).toBe(undefined); }); @@ -422,12 +422,12 @@ describe('Test splom trace defaults:', function() { ]); expect(fullLayout.xaxis).toBe(undefined); expect(fullLayout.yaxis).toBe(undefined); - expect(fullLayout.xaxis2.title).toBe('A'); - expect(fullLayout.yaxis2.title).toBe('A'); - expect(fullLayout.xaxis3.title).toBe('B'); - expect(fullLayout.yaxis3.title).toBe('B'); - expect(fullLayout.xaxis4.title).toBe('C'); - expect(fullLayout.yaxis4.title).toBe('C'); + expect(fullLayout.xaxis2.title.text).toBe('A'); + expect(fullLayout.yaxis2.title.text).toBe('A'); + expect(fullLayout.xaxis3.title.text).toBe('B'); + expect(fullLayout.yaxis3.title.text).toBe('B'); + expect(fullLayout.xaxis4.title.text).toBe('C'); + expect(fullLayout.yaxis4.title.text).toBe('C'); expect(fullLayout.xaxis5).toBe(undefined); expect(fullLayout.yaxis5).toBe(undefined); }); diff --git a/test/jasmine/tests/template_test.js b/test/jasmine/tests/template_test.js index 0e4097c6b59..9ebff733c6a 100644 --- a/test/jasmine/tests/template_test.js +++ b/test/jasmine/tests/template_test.js @@ -158,7 +158,9 @@ describe('makeTemplate', function() { {fill: 'toself'} ] }, layout: { - title: 'Fill toself and tonext', + title: { + text: 'Fill toself and tonext' + }, width: 400, height: 400 } diff --git a/test/jasmine/tests/validate_test.js b/test/jasmine/tests/validate_test.js index 0a455f2b4e5..aed5c603233 100644 --- a/test/jasmine/tests/validate_test.js +++ b/test/jasmine/tests/validate_test.js @@ -18,7 +18,9 @@ describe('Plotly.validate', function() { type: 'scatter', x: [1, 2, 3] }], { - title: 'my simple graph' + title: { + text: 'my simple graph' + } }); expect(out).toBeUndefined(); @@ -371,7 +373,9 @@ describe('Plotly.validate', function() { }] }), ], { - title: 'my transformed graph' + title: { + text: 'my transformed graph' + } }); expect(out.length).toEqual(5); From 58f402a3bd9209d628b2689acd867b8ef9119128 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=B6stl?= Date: Thu, 15 Nov 2018 17:09:14 +0100 Subject: [PATCH 21/49] Reactivating editing plot title [882] --- src/components/titles/index.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/components/titles/index.js b/src/components/titles/index.js index 9315343bf60..8b3f9c81fa3 100644 --- a/src/components/titles/index.js +++ b/src/components/titles/index.js @@ -81,8 +81,7 @@ function draw(gd, titleClass, options) { // only make this title editable if we positively identify its property // as one that has editing enabled. var editAttr; - // TODO 882 Probably compare against 'title.text' and may also adapt calling code? - if(prop === 'title') editAttr = 'titleText'; + if(prop === 'title.text') editAttr = 'titleText'; else if(prop.indexOf('axis') !== -1) editAttr = 'axisTitleText'; else if(prop.indexOf('colorbar' !== -1)) editAttr = 'colorbarTitleText'; var editable = gd._context.edits[editAttr]; From 5e16b0248590095c0fef4e359c2922858b49a554 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=B6stl?= Date: Fri, 16 Nov 2018 09:21:22 +0100 Subject: [PATCH 22/49] Also call cleanLayout on `layout.template.layout` [882] --- src/plot_api/helpers.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/plot_api/helpers.js b/src/plot_api/helpers.js index 441e433eb69..4c155409784 100644 --- a/src/plot_api/helpers.js +++ b/src/plot_api/helpers.js @@ -211,6 +211,11 @@ exports.cleanLayout = function(layout) { // supported, but new tinycolor does not because they're not valid css Color.clean(layout); + // also clean the layout container in layout.template + if(layout.template && layout.template.layout) { + exports.cleanLayout(layout.template.layout); + } + return layout; }; From 7a81f27efd467f03763c4d92f898e0009fd1cb5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=B6stl?= Date: Tue, 20 Nov 2018 08:51:19 +0100 Subject: [PATCH 23/49] Adjust hover template test to new title structure [882] --- test/jasmine/tests/hover_label_test.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/jasmine/tests/hover_label_test.js b/test/jasmine/tests/hover_label_test.js index dbb12c7754b..b8b3bc16dad 100644 --- a/test/jasmine/tests/hover_label_test.js +++ b/test/jasmine/tests/hover_label_test.js @@ -1655,7 +1655,8 @@ describe('hover info', function() { it('should contain the axis names', function(done) { var gd = document.getElementById('graph'); - Plotly.restyle(gd, 'hovertemplate', '%{yaxis.title}:%{y:$.2f}
%{xaxis.title}:%{x:0.4f}') + Plotly.restyle(gd, 'hovertemplate', + '%{yaxis.title.text}:%{y:$.2f}
%{xaxis.title.text}:%{x:0.4f}') .then(function() { Fx.hover('graph', evt, 'xy'); From e53e546f22471f3cf29fcc9c60ac5b2a6a8ab9a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=B6stl?= Date: Tue, 20 Nov 2018 09:16:45 +0100 Subject: [PATCH 24/49] Adapt clonePlot function to new title attr structure [882] --- src/snapshot/cloneplot.js | 10 ++++++---- test/jasmine/tests/snapshot_test.js | 13 +++++++------ 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/src/snapshot/cloneplot.js b/src/snapshot/cloneplot.js index 5a9a49b359a..3b8e35f8b4b 100644 --- a/src/snapshot/cloneplot.js +++ b/src/snapshot/cloneplot.js @@ -24,7 +24,7 @@ function cloneLayoutOverride(tileClass) { autosize: true, width: 150, height: 150, - title: '', + title: {text: ''}, showlegend: false, margin: {l: 5, r: 5, t: 5, b: 5, pad: 0}, annotations: [] @@ -33,7 +33,7 @@ function cloneLayoutOverride(tileClass) { case 'thumbnail': override = { - title: '', + title: {text: ''}, hidesources: true, showlegend: false, borderwidth: 0, @@ -81,10 +81,12 @@ module.exports = function clonePlot(graphObj, options) { for(i = 0; i < keys.length; i++) { if(keyIsAxis(keys[i])) { - newLayout[keys[i]].title = ''; + newLayout[keys[i]].title = {text: ''}; } } + // TODO 882 Shouldn't ternary and polar axis titles be thrown away as well? + // kill colorbar and pie labels for(i = 0; i < newData.length; i++) { var trace = newData[i]; @@ -109,7 +111,7 @@ module.exports = function clonePlot(graphObj, options) { var axesImageOverride = {}; if(options.tileClass === 'thumbnail') { axesImageOverride = { - title: '', + title: {text: ''}, showaxeslabels: false, showticklabels: false, linetickenable: false diff --git a/test/jasmine/tests/snapshot_test.js b/test/jasmine/tests/snapshot_test.js index 90c875e535d..d7da2aa9d52 100644 --- a/test/jasmine/tests/snapshot_test.js +++ b/test/jasmine/tests/snapshot_test.js @@ -36,19 +36,19 @@ describe('Plotly.Snapshot', function() { data = [dummyTrace1, dummyTrace2]; layout = { - title: 'Chart Title', + title: {text: 'Chart Title'}, showlegend: true, autosize: true, width: 688, height: 460, xaxis: { - title: 'xaxis title', + title: {text: 'xaxis title'}, range: [-0.323374917925, 5.32337491793], type: 'linear', autorange: true }, yaxis: { - title: 'yaxis title', + title: {text: 'yaxis title'}, range: [1.41922290389, 10.5807770961], type: 'linear', autorange: true @@ -70,7 +70,7 @@ describe('Plotly.Snapshot', function() { autosize: true, width: 150, height: 150, - title: '', + title: {text: ''}, showlegend: false, margin: {'l': 5, 'r': 5, 't': 5, 'b': 5, 'pad': 0}, annotations: [] @@ -100,7 +100,7 @@ describe('Plotly.Snapshot', function() { }; var THUMBNAIL_DEFAULT_LAYOUT = { - 'title': '', + 'title': {text: ''}, 'hidesources': true, 'showlegend': false, 'hovermode': false, @@ -117,6 +117,7 @@ describe('Plotly.Snapshot', function() { expect(thumbTile.layout.showlegend).toEqual(THUMBNAIL_DEFAULT_LAYOUT.showlegend); expect(thumbTile.layout.borderwidth).toEqual(THUMBNAIL_DEFAULT_LAYOUT.borderwidth); expect(thumbTile.layout.annotations).toEqual(THUMBNAIL_DEFAULT_LAYOUT.annotations); + expect(thumbTile.layout.title).toEqual(THUMBNAIL_DEFAULT_LAYOUT.title); }); it('should create a 3D thumbnail with limited attributes', function() { @@ -142,7 +143,7 @@ describe('Plotly.Snapshot', function() { }; var AXIS_OVERRIDE = { - title: '', + title: {text: ''}, showaxeslabels: false, showticklabels: false, linetickenable: false From ca920ab94998f37f34e736b8246ae8018ad8db86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=B6stl?= Date: Tue, 20 Nov 2018 14:35:51 +0100 Subject: [PATCH 25/49] Implement padding for chart title alignment [882] --- src/components/sliders/attributes.js | 2 +- src/components/updatemenus/attributes.js | 2 +- src/plot_api/subroutines.js | 53 ++++-- src/plots/layout_attributes.js | 12 ++ src/plots/pad_attributes.js | 75 ++++---- src/plots/plots.js | 4 + test/jasmine/tests/titles_test.js | 215 +++++++++++++++++++++-- 7 files changed, 301 insertions(+), 62 deletions(-) diff --git a/src/components/sliders/attributes.js b/src/components/sliders/attributes.js index 574dee98e9d..e0188bb0db2 100644 --- a/src/components/sliders/attributes.js +++ b/src/components/sliders/attributes.js @@ -133,7 +133,7 @@ module.exports = overrideAll(templatedArray('slider', { role: 'style', description: 'Sets the x position (in normalized coordinates) of the slider.' }, - pad: extendDeepAll({}, padAttrs, { + pad: extendDeepAll(padAttrs({editType: 'arraydraw'}), { description: 'Set the padding of the slider component along each side.' }, {t: {dflt: 20}}), xanchor: { diff --git a/src/components/updatemenus/attributes.js b/src/components/updatemenus/attributes.js index 6224392ed02..e6cf24e8d5b 100644 --- a/src/components/updatemenus/attributes.js +++ b/src/components/updatemenus/attributes.js @@ -162,7 +162,7 @@ module.exports = overrideAll(templatedArray('updatemenu', { ].join(' ') }, - pad: extendFlat({}, padAttrs, { + pad: extendFlat(padAttrs({editType: 'arraydraw'}), { description: 'Sets the padding around the buttons or dropdown menu.' }), diff --git a/src/plot_api/subroutines.js b/src/plot_api/subroutines.js index cc8738563d3..15b90dd6c56 100644 --- a/src/plot_api/subroutines.js +++ b/src/plot_api/subroutines.js @@ -27,6 +27,10 @@ var enforceAxisConstraints = axisConstraints.enforce; var cleanAxisConstraints = axisConstraints.clean; var doAutoRange = require('../plots/cartesian/autorange').doAutoRange; +var SVG_TEXT_ANCHOR_START = 'start'; +var SVG_TEXT_ANCHOR_MIDDLE = 'middle'; +var SVG_TEXT_ANCHOR_END = 'end'; + exports.layoutStyles = function(gd) { return Lib.syncOrAsync([Plots.doAutoMargin, lsInner], gd); }; @@ -450,45 +454,62 @@ function findCounterAxisLineWidth(ax, side, counterAx, axList) { exports.drawMainTitle = function(gd) { var fullLayout = gd._fullLayout; + var textAnchor = getMainTitleTextAnchor(fullLayout); + var dy = getMainTitleDy(fullLayout); + Titles.draw(gd, 'gtitle', { propContainer: fullLayout, propName: 'title.text', placeholder: fullLayout._dfltTitle.plot, attributes: { - x: getMainTitleX(fullLayout), - y: getMainTitleY(fullLayout), - 'text-anchor': getMainTitleTextAnchor(fullLayout), - dy: getMainTitleDy(fullLayout) + x: getMainTitleX(fullLayout, textAnchor), + y: getMainTitleY(fullLayout, dy), + 'text-anchor': textAnchor, + dy: dy } }); }; -function getMainTitleX(fullLayout) { +function getMainTitleX(fullLayout, textAnchor) { var title = fullLayout.title; var _size = fullLayout._size; + var hPadShift = 0; + + if(textAnchor === SVG_TEXT_ANCHOR_START) { + hPadShift = title.pad.l; + } else if(textAnchor === SVG_TEXT_ANCHOR_END) { + hPadShift = -title.pad.r; + } switch(title.xref) { case 'paper': - return _size.l + _size.w * title.x; + return _size.l + _size.w * title.x + hPadShift; case 'container': default: - return fullLayout.width * title.x; + return fullLayout.width * title.x + hPadShift; } } -function getMainTitleY(fullLayout) { +function getMainTitleY(fullLayout, dy) { var title = fullLayout.title; var _size = fullLayout._size; + var vPadShift = 0; + + if(dy === '0em' || !dy) { + vPadShift = -title.pad.b; + } else if(dy === alignmentConstants.CAP_SHIFT + 'em') { + vPadShift = title.pad.t; + } if(title.y === 'auto') { return _size.t / 2; } else { switch(title.yref) { case 'paper': - return _size.t + _size.h - _size.h * title.y; + return _size.t + _size.h - _size.h * title.y + vPadShift; case 'container': default: - return fullLayout.height - fullLayout.height * title.y; + return fullLayout.height - fullLayout.height * title.y + vPadShift; } } } @@ -500,22 +521,22 @@ function getMainTitleTextAnchor(fullLayout) { case 'auto': return calcTextAnchor(fullLayout.title.x); case 'left': - return 'start'; + return SVG_TEXT_ANCHOR_START; case 'right': - return 'end'; + return SVG_TEXT_ANCHOR_END; case 'center': default: - return 'middle'; + return SVG_TEXT_ANCHOR_MIDDLE; } function calcTextAnchor(x) { if(x < 1 / 3) { - return 'start'; + return SVG_TEXT_ANCHOR_START; } else if(x > 2 / 3) { - return 'end'; + return SVG_TEXT_ANCHOR_END; } - return 'middle'; + return SVG_TEXT_ANCHOR_MIDDLE; } } diff --git a/src/plots/layout_attributes.js b/src/plots/layout_attributes.js index 706c14915f5..c2e019b56ba 100644 --- a/src/plots/layout_attributes.js +++ b/src/plots/layout_attributes.js @@ -10,6 +10,8 @@ var fontAttrs = require('./font_attributes'); var colorAttrs = require('../components/color/attributes'); +var padAttrs = require('./pad_attributes'); +var extendFlat = require('../lib/extend').extendFlat; var globalFont = fontAttrs({ editType: 'calc', @@ -102,6 +104,16 @@ module.exports = { 'docu', // TODO document ].join(' ') }, + pad: extendFlat(padAttrs({editType: 'layoutstyle'}), { + description: [ + 'Sets the padding of the title.', + 'Each padding value only applies when the corresponding', + 'xanchor / yanchor value is set accordingly. E.g. for left', + 'padding to take effect, xanchor must be set to left.', + 'The same rule applies if xanchor/yanchor is determined automatically.', + 'Padding is muted if respective anchor value is middle/center.' + ].join(' ') + }), editType: 'layoutstyle' }, autosize: { diff --git a/src/plots/pad_attributes.js b/src/plots/pad_attributes.js index 539f2daee6e..47264860c50 100644 --- a/src/plots/pad_attributes.js +++ b/src/plots/pad_attributes.js @@ -8,37 +8,46 @@ 'use strict'; -// This is used exclusively by components inside component arrays, -// hence the 'arraydraw' editType. If this ever gets used elsewhere -// we could generalize it as a function ala font_attributes -module.exports = { - t: { - valType: 'number', - dflt: 0, - role: 'style', - editType: 'arraydraw', - description: 'The amount of padding (in px) along the top of the component.' - }, - r: { - valType: 'number', - dflt: 0, - role: 'style', - editType: 'arraydraw', - description: 'The amount of padding (in px) on the right side of the component.' - }, - b: { - valType: 'number', - dflt: 0, - role: 'style', - editType: 'arraydraw', - description: 'The amount of padding (in px) along the bottom of the component.' - }, - l: { - valType: 'number', - dflt: 0, - role: 'style', - editType: 'arraydraw', - description: 'The amount of padding (in px) on the left side of the component.' - }, - editType: 'arraydraw' +/** + * Creates a set of padding attributes. + * + * @param {object} opts + * @param {string} editType: + * the editType for all pieces of this padding definition + * + * @return {object} attributes object containing {t, r, b, l} as specified + */ +module.exports = function(opts) { + var editType = opts.editType; + return { + t: { + valType: 'number', + dflt: 0, + role: 'style', + editType: editType, + description: 'The amount of padding (in px) along the top of the component.' + }, + r: { + valType: 'number', + dflt: 0, + role: 'style', + editType: editType, + description: 'The amount of padding (in px) on the right side of the component.' + }, + b: { + valType: 'number', + dflt: 0, + role: 'style', + editType: editType, + description: 'The amount of padding (in px) along the bottom of the component.' + }, + l: { + valType: 'number', + dflt: 0, + role: 'style', + editType: editType, + description: 'The amount of padding (in px) on the left side of the component.' + }, + editType: editType + }; }; diff --git a/src/plots/plots.js b/src/plots/plots.js index 5b7ec913b8f..b1237ed7e31 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -1338,6 +1338,10 @@ plots.supplyLayoutGlobalDefaults = function(layoutIn, layoutOut, formatObj) { coerce('title.y'); // TODO restrict to [-2, 3]? coerce('title.xanchor'); coerce('title.yanchor'); + coerce('title.pad.t'); + coerce('title.pad.r'); + coerce('title.pad.b'); + coerce('title.pad.l'); // Make sure that autosize is defaulted to *true* // on layouts with no set width and height for backward compatibly, diff --git a/test/jasmine/tests/titles_test.js b/test/jasmine/tests/titles_test.js index 239223b0145..dbe937219b4 100644 --- a/test/jasmine/tests/titles_test.js +++ b/test/jasmine/tests/titles_test.js @@ -14,6 +14,7 @@ describe('Plot title', function() { 'use strict'; var data = [{x: [1, 2, 3], y: [1, 2, 3]}]; + var titlePad = {t: 10, r: 10, b: 10, l: 10}; var gd; beforeEach(function() { @@ -26,7 +27,8 @@ describe('Plot title', function() { desc: 'container', select: function() { return d3.selectAll('.svg-container').node(); - } + }, + ref: 'container' }; var paperElemSelector = { @@ -36,7 +38,8 @@ describe('Plot title', function() { expect(bgLayer.empty()).toBe(false, 'No background layer found representing the size of the plot area'); return bgLayer.node(); - } + }, + ref: 'paper' }; it('is centered horizontally and vertically above the plot by default', function() { @@ -236,22 +239,196 @@ describe('Plot title', function() { expectDefaultCenteredPosition(gd); }); - // TODO padding + // Horizontal padding + [containerElemSelector, paperElemSelector].forEach(function(refSelector) { + it('can be placed x pixels away from left ' + refSelector.desc + ' edge', function() { + Plotly.plot(gd, data, extendLayout({ + xref: refSelector.ref, + xanchor: 'left', + x: 0, + pad: titlePad + })); - it('preserves alignment when text is updated via `Plotly.relayout` using an attribute string', function() { - // TODO implement once alignment is implemented + expect(titleSel().attr('text-anchor')).toBe('start'); + expect(titleX() - 10).toBe(leftOf(refSelector)); + }); }); - it('preserves alignment when text is updated via `Plotly.update` using an attribute string', function() { - // TODO implement once alignment is implemented + [containerElemSelector, paperElemSelector].forEach(function(refSelector) { + it('can be placed x pixels away from right ' + refSelector.desc + ' edge', function() { + Plotly.plot(gd, data, extendLayout({ + xref: refSelector.ref, + xanchor: 'right', + x: 1, + pad: titlePad + })); + + expect(titleSel().attr('text-anchor')).toBe('end'); + expect(titleX() + 10).toBe(rightOf(refSelector)); + }); }); - it('discards alignment when text is updated via `Plotly.relayout` by passing a new title object', function() { - // TODO implement once alignment is implemented + [containerElemSelector, paperElemSelector].forEach(function(refSelector) { + it('figures out for itself which horizontal padding applies when {xanchor: \'auto\'}' + + refSelector.desc + ' edge', function() { + Plotly.plot(gd, data, extendLayout({ + xref: refSelector.ref, + xanchor: 'auto', + x: 1, + pad: titlePad + })); + + expect(titleSel().attr('text-anchor')).toBe('end'); + expect(titleX() + 10).toBe(rightOf(refSelector)); + + Plotly.relayout(gd, 'title.x', 0); + + expect(titleSel().attr('text-anchor')).toBe('start'); + expect(titleX() - 10).toBe(leftOf(refSelector)); + + Plotly.relayout(gd, 'title.x', 0.5); + expectCenteredWithin(refSelector); + }); }); - it('discards alignment when text is updated via `Plotly.update` by passing a new title object', function() { - // TODO implement once alignment is implemented + // Cases when horizontal padding is ignored + // (just testing with paper is sufficient) + [ + {pad: {l: 20}, dir: 'left'}, + {pad: {r: 20}, dir: 'right'} + ].forEach(function(testCase) { + it('mutes ' + testCase.dir + ' padding for {xanchor: \'center\'}', function() { + Plotly.plot(gd, data, extendLayout({ + xref: 'paper', + xanchor: 'middle', + x: 0.5, + pad: testCase.pad + })); + + expectCenteredWithin(paperElemSelector); + }); + }); + + it('mutes left padding when xanchor is right', function() { + Plotly.plot(gd, data, extendLayout({ + xref: 'paper', + x: 1, + xanchor: 'right', + pad: { + l: 1000 + } + })); + + expectRightEdgeAlignedTo(paperElemSelector); + }); + + it('mutes right padding when xanchor is left', function() { + Plotly.plot(gd, data, extendLayout({ + xref: 'paper', + x: 0, + xanchor: 'left', + pad: { + r: 1000 + } + })); + + expectLeftEdgeAlignedTo(paperElemSelector); + }); + + // Vertical padding + [containerElemSelector, paperElemSelector].forEach(function(refSelector) { + it('can be placed x pixels below top ' + refSelector.desc + ' edge', function() { + Plotly.plot(gd, data, extendLayout({ + yref: refSelector.ref, + yanchor: 'top', + y: 1, + pad: titlePad + })); + + var capLineY = calcTextCapLineY(titleSel()); + expect(capLineY).toBe(topOf(refSelector) + 10); + }); + }); + + [containerElemSelector, paperElemSelector].forEach(function(refSelector) { + it('can be placed x pixels above bottom ' + refSelector.desc + ' edge', function() { + Plotly.plot(gd, data, extendLayout({ + yref: refSelector.ref, + yanchor: 'bottom', + y: 0, + pad: titlePad + })); + + var baselineY = calcTextBaselineY(titleSel()); + expect(baselineY).toBe(bottomOf(refSelector) - 10); + }); + }); + + [containerElemSelector, paperElemSelector].forEach(function(refSelector) { + it('figures out for itself which vertical padding applies when {yanchor: \'auto\'}' + + refSelector.desc + ' edge', function() { + Plotly.plot(gd, data, extendLayout({ + yref: refSelector.ref, + yanchor: 'auto', + y: 1, + pad: titlePad + })); + + var capLineY = calcTextCapLineY(titleSel()); + expect(capLineY).toBe(topOf(refSelector) + 10); + + Plotly.relayout(gd, 'title.y', 0); + + var baselineY = calcTextBaselineY(titleSel()); + expect(baselineY).toBe(bottomOf(refSelector) - 10); + + Plotly.relayout(gd, 'title.y', 0.5); + expectCenteredVerticallyWithin(refSelector); + }); + }); + + // Cases when vertical padding is ignored + // (just testing with paper is sufficient) + [ + {pad: {t: 20}, dir: 'top'}, + {pad: {b: 20}, dir: 'bottom'} + ].forEach(function(testCase) { + it('mutes ' + testCase.dir + ' padding for {yanchor: \'middle\'}', function() { + Plotly.plot(gd, data, extendLayout({ + yref: 'paper', + yanchor: 'middle', + y: 0.5, + pad: testCase.pad + })); + + expectCenteredVerticallyWithin(paperElemSelector); + }); + }); + + it('mutes top padding when yanchor is bottom', function() { + Plotly.plot(gd, data, extendLayout({ + yref: 'paper', + y: 0, + yanchor: 'bottom', + pad: { + t: 1000 + } + })); + + expectBaselineAlignsWithBottomEdgeOf(paperElemSelector); + }); + + it('mutes bottom padding when yanchor is top', function() { + Plotly.plot(gd, data, extendLayout({ + yref: 'paper', + y: 1, + yanchor: 'top', + pad: { + b: 1000 + } + })); + + expectCapLineAlignsWithTopEdgeOf(paperElemSelector); }); function extendLayout(titleAttrs) { @@ -264,6 +441,22 @@ describe('Plot title', function() { }; } + function topOf(elemSelector) { + return elemSelector.select().getBoundingClientRect().top; + } + + function rightOf(elemSelector) { + return elemSelector.select().getBoundingClientRect().right; + } + + function bottomOf(elemSelector) { + return elemSelector.select().getBoundingClientRect().bottom; + } + + function leftOf(elemSelector) { + return elemSelector.select().getBoundingClientRect().left; + } + function expectLeftEdgeAlignedTo(elemSelector) { expectHorizontalEdgeAligned(elemSelector, 'left'); } From 37bb68c62fcfc977e4b9b1fa7ac4558720bb3e4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=B6stl?= Date: Tue, 20 Nov 2018 15:35:51 +0100 Subject: [PATCH 26/49] Restrict title.y (and x) to a number between 0 and 1 [882] - Special coerce logic for y because it can be 'auto' also. --- src/plots/layout_attributes.js | 6 ++++-- src/plots/plots.js | 17 +++++++++++++++-- test/jasmine/tests/titles_test.js | 4 +--- 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/src/plots/layout_attributes.js b/src/plots/layout_attributes.js index c2e019b56ba..bdf3fad0c26 100644 --- a/src/plots/layout_attributes.js +++ b/src/plots/layout_attributes.js @@ -64,7 +64,9 @@ module.exports = { }, x: { valType: 'number', - dflt: '0.5', + min: 0, + max: 1, + dflt: 0.5, role: 'style', editType: 'layoutstyle', description: [ @@ -78,7 +80,7 @@ module.exports = { role: 'style', editType: 'layoutstyle', description: [ - 'Some', + 'Some', // TODO document 'docu', ].join(' ') }, diff --git a/src/plots/plots.js b/src/plots/plots.js index b1237ed7e31..9d7cc9f887f 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -1315,6 +1315,19 @@ plots.supplyLayoutGlobalDefaults = function(layoutIn, layoutOut, formatObj) { return Lib.coerce(layoutIn, layoutOut, plots.layoutAttributes, attr, dflt); } + // Special treatment for title.y that can be 'auto' + // or a number between 0 and 1. + function coerceTitleY() { + if(layoutIn.title && isNumeric(layoutIn.title.y)) { + var propOut = Lib.nestedProperty(layoutOut, 'title.y'); + var opts = Lib.nestedProperty(plots.layoutAttributes, 'title.y').get(); + + Lib.valObjectMeta.number.coerceFunction(layoutIn.title.y, propOut, 'auto', opts); + } else { + coerce('title.y'); + } + } + var template = layoutIn.template; if(Lib.isPlainObject(template)) { layoutOut.template = template; @@ -1334,8 +1347,8 @@ plots.supplyLayoutGlobalDefaults = function(layoutIn, layoutOut, formatObj) { coerce('title.xref'); coerce('title.yref'); - coerce('title.x'); // TODO restrict to [-2, 3]? - coerce('title.y'); // TODO restrict to [-2, 3]? + coerce('title.x'); + coerceTitleY(); coerce('title.xanchor'); coerce('title.yanchor'); coerce('title.pad.t'); diff --git a/test/jasmine/tests/titles_test.js b/test/jasmine/tests/titles_test.js index dbe937219b4..2dd4ba2230f 100644 --- a/test/jasmine/tests/titles_test.js +++ b/test/jasmine/tests/titles_test.js @@ -170,15 +170,13 @@ describe('Plot title', function() { // xanchor 'auto' test [ - {x: -0.5, expAlignment: 'start'}, {x: 0, expAlignment: 'start'}, {x: 0.3, expAlignment: 'start'}, {x: 0.4, expAlignment: 'middle'}, {x: 0.5, expAlignment: 'middle'}, {x: 0.6, expAlignment: 'middle'}, {x: 0.7, expAlignment: 'end'}, - {x: 1, expAlignment: 'end'}, - {x: 1.5, expAlignment: 'end'} + {x: 1, expAlignment: 'end'} ].forEach(function(testCase) { runXAnchorAutoTest(testCase, 'container'); runXAnchorAutoTest(testCase, 'paper'); From e438a3f87528e1a8473f2ff8784e213610b1b7f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=B6stl?= Date: Wed, 21 Nov 2018 08:57:11 +0100 Subject: [PATCH 27/49] Edit attribute descriptions for new title attr structure [882] --- src/plots/layout_attributes.js | 44 +++++++++++++++++++++------------- 1 file changed, 28 insertions(+), 16 deletions(-) diff --git a/src/plots/layout_attributes.js b/src/plots/layout_attributes.js index bdf3fad0c26..1cf743b3639 100644 --- a/src/plots/layout_attributes.js +++ b/src/plots/layout_attributes.js @@ -47,8 +47,9 @@ module.exports = { role: 'info', editType: 'layoutstyle', description: [ - 'Some', - 'docu', // TODO document + 'Sets the container `x` refers to.', + '*container* spans the entire `width` of the plot.', + '*paper* refers to the width of the plotting area only.' ].join(' ') }, yref: { @@ -58,8 +59,9 @@ module.exports = { role: 'info', editType: 'layoutstyle', description: [ - 'Some', - 'docu', // TODO document + 'Sets the container `y` refers to.', + '*container* spans the entire `height` of the plot.', + '*paper* refers to the height of the plotting area only.' ].join(' ') }, x: { @@ -70,8 +72,8 @@ module.exports = { role: 'style', editType: 'layoutstyle', description: [ - 'Some', - 'docu', // TODO document + 'Sets the x position with respect to `xref` in normalized', + 'coordinates from *0* (left) to *1* (right).' ].join(' ') }, y: { @@ -80,8 +82,10 @@ module.exports = { role: 'style', editType: 'layoutstyle', description: [ - 'Some', // TODO document - 'docu', + 'Sets the y position with respect to `yref` in normalized', + 'coordinates from *0* (bottom) to *1* (top).', + '*auto* places the baseline of the title onto the', + 'vertical center of the top margin.' ].join(' ') }, xanchor: { @@ -91,8 +95,12 @@ module.exports = { role: 'info', editType: 'layoutstyle', description: [ - 'Some', - 'docu', // TODO document + 'Sets the title\'s horizontal alignment with respect to its x position.', + '*left* means that the title starts at x,', + '*right* means that the title ends at x', + 'and *center* means that the title\'s center is at x.', + '*auto* divides `xref` by three and calculates the `xanchor`', + 'value automatically based on the value of `x`.' ].join(' ') }, yanchor: { @@ -102,18 +110,22 @@ module.exports = { role: 'info', editType: 'layoutstyle', description: [ - 'Some', - 'docu', // TODO document + 'Sets the title\'s vertical alignment with respect to its y position.', + '*top* means that the title\'s cap line is at y,', + '*bottom* means that the title\'s baseline is at y', + 'and *middle* means that the title\'s midline is at y.', + '*auto* divides `yref` by three and calculates the `yanchor`', + 'value automatically based on the value of `y`.' ].join(' ') }, pad: extendFlat(padAttrs({editType: 'layoutstyle'}), { description: [ 'Sets the padding of the title.', 'Each padding value only applies when the corresponding', - 'xanchor / yanchor value is set accordingly. E.g. for left', - 'padding to take effect, xanchor must be set to left.', - 'The same rule applies if xanchor/yanchor is determined automatically.', - 'Padding is muted if respective anchor value is middle/center.' + '`xanchor`/`yanchor` value is set accordingly. E.g. for left', + 'padding to take effect, `xanchor` must be set to *left*.', + 'The same rule applies if `xanchor`/`yanchor` is determined automatically.', + 'Padding is muted if the respective anchor value is *middle*/*center*.' ].join(' ') }), editType: 'layoutstyle' From b37950d79080a92f9baa7f4b0cb33216201df5ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=B6stl?= Date: Wed, 21 Nov 2018 10:39:02 +0100 Subject: [PATCH 28/49] Minor clean up in subroutines.js [882] - Eliminate an unnecessary else block. --- src/plot_api/subroutines.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/plot_api/subroutines.js b/src/plot_api/subroutines.js index 15b90dd6c56..7a4b14dd4a8 100644 --- a/src/plot_api/subroutines.js +++ b/src/plot_api/subroutines.js @@ -564,9 +564,9 @@ function getMainTitleDy(fullLayout) { return '0em'; } else if(y > 2 / 3) { return alignmentConstants.CAP_SHIFT + 'em'; - } else { - return alignmentConstants.MID_SHIFT + 'em'; } + + return alignmentConstants.MID_SHIFT + 'em'; } return 0; From 0c2bfd73af2fee1c361fa84cb9e80fb20e40ccca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=B6stl?= Date: Wed, 21 Nov 2018 10:41:14 +0100 Subject: [PATCH 29/49] Fix cleaning deprecated title structure on relayout/update [882] - Bug: Using an attribute string representing the deprecated title structure to unset a value wasn't supported. --- src/plot_api/plot_api.js | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js index 110c81e5abc..c14e9cebf34 100644 --- a/src/plot_api/plot_api.js +++ b/src/plot_api/plot_api.js @@ -1687,12 +1687,12 @@ function _restyle(gd, aobj, traces) { } /** - * Maps deprecated layout attributes that need to be converted - * to the current API at this stage to ensure backwards compatibility. + * Converts deprecated layout attributes to the current API + * to ensure backwards compatibility. * * @param layoutObj */ -function mapDeprecatedLayoutAttributes(layoutObj) { +function cleanDeprecatedLayoutAttributes(layoutObj) { // Support old-style update of the title's font var isTitlefontSet = layoutObj.titlefont; @@ -1715,14 +1715,14 @@ function mapDeprecatedLayoutAttributes(layoutObj) { } /** - * Maps deprecated layout "string attributes" to + * Converts deprecated layout "string attributes" to * "string attributes" of the current API to ensure backwards compatibility. * * E.g. Maps {'xaxis.title': 'A chart'} to {'xaxis.title.text': 'A chart'} * * @param layoutObj */ -function mapDeprecatedLayoutAttributeStrings(layoutObj) { +function cleanDeprecatedLayoutAttributeStrings(layoutObj) { var oldAxisTitleRegExp = /axis\d{0,2}.title$/; var keys, i, key, value; @@ -1748,11 +1748,9 @@ function mapDeprecatedLayoutAttributeStrings(layoutObj) { } } - function replace(oldKey, newKey) { - if(layoutObj[oldKey] !== undefined) { - layoutObj[newKey] = layoutObj[oldKey]; - delete layoutObj[oldKey]; - } + function replace(oldAttrStr, newAttrStr) { + layoutObj[newAttrStr] = layoutObj[oldAttrStr]; + delete layoutObj[oldAttrStr]; } } @@ -1796,8 +1794,8 @@ exports.relayout = function relayout(gd, astr, val) { if(Object.keys(aobj).length) gd.changed = true; - mapDeprecatedLayoutAttributes(aobj); - mapDeprecatedLayoutAttributeStrings(aobj); + cleanDeprecatedLayoutAttributes(aobj); + cleanDeprecatedLayoutAttributeStrings(aobj); var specs = _relayout(gd, aobj); var flags = specs.flags; @@ -2276,8 +2274,8 @@ exports.update = function update(gd, traceUpdate, layoutUpdate, _traces) { var restyleSpecs = _restyle(gd, Lib.extendFlat({}, traceUpdate), traces); var restyleFlags = restyleSpecs.flags; - mapDeprecatedLayoutAttributes(layoutUpdate); - mapDeprecatedLayoutAttributeStrings(layoutUpdate); + cleanDeprecatedLayoutAttributes(layoutUpdate); + cleanDeprecatedLayoutAttributeStrings(layoutUpdate); var relayoutSpecs = _relayout(gd, Lib.extendFlat({}, layoutUpdate)); var relayoutFlags = relayoutSpecs.flags; From eda4f72abb638cd93f657e12a8c3bbde6c1a59b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=B6stl?= Date: Wed, 21 Nov 2018 10:42:36 +0100 Subject: [PATCH 30/49] Fix `cleanLayout` bug [882] - Bug: if only a `titlefont` but no `title` was set, `cleanTitle` would not create the new title structure. --- src/plot_api/helpers.js | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/plot_api/helpers.js b/src/plot_api/helpers.js index 4c155409784..c854798290f 100644 --- a/src/plot_api/helpers.js +++ b/src/plot_api/helpers.js @@ -100,12 +100,14 @@ exports.cleanLayout = function(layout) { // modifications for polar else if(polarAttrRegex && polarAttrRegex.test(key)) { var polar = layout[key]; + cleanTitle(polar.radialaxis); } // modifications for ternary else if(ternaryAttrRegex && ternaryAttrRegex.test(key)) { var ternary = layout[key]; + cleanTitle(ternary.aaxis); cleanTitle(ternary.baxis); cleanTitle(ternary.caxis); @@ -199,7 +201,7 @@ exports.cleanLayout = function(layout) { } } - // Check for old-style title definition + // clean plot title cleanTitle(layout); /* @@ -211,7 +213,7 @@ exports.cleanLayout = function(layout) { // supported, but new tinycolor does not because they're not valid css Color.clean(layout); - // also clean the layout container in layout.template + // clean the layout container in layout.template if(layout.template && layout.template.layout) { exports.cleanLayout(layout.template.layout); } @@ -229,6 +231,8 @@ function cleanAxRef(container, attr) { function cleanTitle(titleContainer) { if(titleContainer) { + + // title -> title.text if(typeof titleContainer.title === 'string') { titleContainer.title = { text: titleContainer.title @@ -236,9 +240,13 @@ function cleanTitle(titleContainer) { } // titlefont -> title.font - if(titleContainer.title && - Lib.isPlainObject(titleContainer.titlefont) && - !Lib.isPlainObject(titleContainer.title.font)) { + var oldFontAttrSet = Lib.isPlainObject(titleContainer.titlefont); + var newFontAttrSet = titleContainer.title && Lib.isPlainObject(titleContainer.title.font); + if(oldFontAttrSet && !newFontAttrSet) { + if(!titleContainer.title) { + titleContainer.title = {}; + } + titleContainer.title.font = titleContainer.titlefont; delete titleContainer.titlefont; } From 53ea94d881b341068efb52779acbbd6a4dbc2f9b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=B6stl?= Date: Wed, 21 Nov 2018 10:44:06 +0100 Subject: [PATCH 31/49] Remove small piece of obsolete code in gl3d's convert module [882] --- src/plots/gl3d/layout/convert.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plots/gl3d/layout/convert.js b/src/plots/gl3d/layout/convert.js index 89ebecdc79d..d5b7b0fecab 100644 --- a/src/plots/gl3d/layout/convert.js +++ b/src/plots/gl3d/layout/convert.js @@ -84,7 +84,7 @@ proto.merge = function(sceneLayout) { // Axes labels opts.labels[i] = axes.title.text; - if('title' in axes && 'font' in axes.title) { + if('font' in axes.title) { if(axes.title.font.color) opts.labelColor[i] = str2RgbaArray(axes.title.font.color); if(axes.title.font.family) opts.labelFont[i] = axes.title.font.family; if(axes.title.font.size) opts.labelSize[i] = axes.title.font.size; From f8962732cd54aeec3eb49be23a35406c03e67383 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=B6stl?= Date: Thu, 22 Nov 2018 08:36:05 +0100 Subject: [PATCH 32/49] Still accept numbers as titles when defined in deprecated notation [882] - Before introducing the new title attribute structure, a title could have been defined as a number as well - which is a feature of the string attribute coerce function. --- src/plot_api/helpers.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/plot_api/helpers.js b/src/plot_api/helpers.js index c854798290f..7bdde44ae90 100644 --- a/src/plot_api/helpers.js +++ b/src/plot_api/helpers.js @@ -233,7 +233,9 @@ function cleanTitle(titleContainer) { if(titleContainer) { // title -> title.text - if(typeof titleContainer.title === 'string') { + // (although title used to be a string attribute, + // numbers are accepted as well) + if(typeof titleContainer.title === 'string' || typeof titleContainer.title === 'number') { titleContainer.title = { text: titleContainer.title }; From f7b652bd5eada67d541d54a24f52f7d39f6dbf9f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=B6stl?= Date: Thu, 22 Nov 2018 10:49:23 +0100 Subject: [PATCH 33/49] Improve and simplify title compatibility code for relayout [882] - Downsize the cleaning of relayout attributes to one method. - Allow titles to be numbers, not just strings. - Call cleaning method just in one place. - Make comments more clear. --- src/plot_api/plot_api.js | 57 ++++++++++------------------------------ 1 file changed, 14 insertions(+), 43 deletions(-) diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js index c14e9cebf34..c45bd7b016a 100644 --- a/src/plot_api/plot_api.js +++ b/src/plot_api/plot_api.js @@ -1687,53 +1687,26 @@ function _restyle(gd, aobj, traces) { } /** - * Converts deprecated layout attributes to the current API - * to ensure backwards compatibility. + * Converts deprecated layout attributes and "string attributes" alike to + * the current API to ensure backwards compatibility. * - * @param layoutObj - */ -function cleanDeprecatedLayoutAttributes(layoutObj) { - - // Support old-style update of the title's font - var isTitlefontSet = layoutObj.titlefont; - var isFontInTitleSet = layoutObj.title && layoutObj.title.font; - var isFontInTitleNotSet = !isFontInTitleSet; - if(isTitlefontSet && isFontInTitleNotSet) { - - // Use string attribute because initiating a new title object - // to be able to specify a font property on it would require to - // know the potentially existing `title.text` property. - layoutObj['title.font'] = layoutObj.titlefont; - delete layoutObj.titlefont; - } - - // Note, that updating x-axis and y-axis title fonts - // was never supported unless (i) using string - // attributes or (ii) passing `xaxis.title` / `yaxis.title` - // again. And these cases are covered by - // helpers.cleanLayout anyways. -} - -/** - * Converts deprecated layout "string attributes" to - * "string attributes" of the current API to ensure backwards compatibility. + * This is needed for the relayout mechanism to determine which + * subroutines to run based on the actual layout attribute + * definitions (that don't include the deprecated ones). * * E.g. Maps {'xaxis.title': 'A chart'} to {'xaxis.title.text': 'A chart'} + * and {titlefont: {...}} to {'title.font': {...}}. * * @param layoutObj */ -function cleanDeprecatedLayoutAttributeStrings(layoutObj) { - var oldAxisTitleRegExp = /axis\d{0,2}.title$/; +function cleanDeprecatedLayoutAttributes(layoutObj) { + var oldAxisTitleRegExp = /axis\d{0,2}\.title$/; var keys, i, key, value; if(typeof layoutObj.title === 'string') { layoutObj.title = {text: layoutObj.title}; } - // Note: Only "nested" (dot notation) attributes - // need to be converted. For example 'layout.titlefont' - // was a top-level attribute and thus is covered by - // the general compatibility layer. keys = Object.keys(layoutObj); for(i = 0; i < keys.length; i++) { key = keys[i]; @@ -1743,7 +1716,8 @@ function cleanDeprecatedLayoutAttributeStrings(layoutObj) { replace(key, key.replace('titlefont', 'title.font')); } - if(typeof value === 'string' && oldAxisTitleRegExp.test(key)) { + if((typeof value === 'string' || typeof value === 'number') && + oldAxisTitleRegExp.test(key)) { replace(key, key.replace('title', 'title.text')); } } @@ -1794,9 +1768,6 @@ exports.relayout = function relayout(gd, astr, val) { if(Object.keys(aobj).length) gd.changed = true; - cleanDeprecatedLayoutAttributes(aobj); - cleanDeprecatedLayoutAttributeStrings(aobj); - var specs = _relayout(gd, aobj); var flags = specs.flags; @@ -1887,13 +1858,16 @@ var AX_DOMAIN_RE = /^[xyz]axis[0-9]*\.domain(\[[0|1]\])?$/; function _relayout(gd, aobj) { var layout = gd.layout, fullLayout = gd._fullLayout, - keys = Object.keys(aobj), axes = Axes.list(gd), arrayEdits = {}, + keys, arrayStr, i, j; + cleanDeprecatedLayoutAttributes(aobj); + keys = Object.keys(aobj); + // look for 'allaxes', split out into all axes // in case of 3D the axis are nested within a scene which is held in _id for(i = 0; i < keys.length; i++) { @@ -2274,9 +2248,6 @@ exports.update = function update(gd, traceUpdate, layoutUpdate, _traces) { var restyleSpecs = _restyle(gd, Lib.extendFlat({}, traceUpdate), traces); var restyleFlags = restyleSpecs.flags; - cleanDeprecatedLayoutAttributes(layoutUpdate); - cleanDeprecatedLayoutAttributeStrings(layoutUpdate); - var relayoutSpecs = _relayout(gd, Lib.extendFlat({}, layoutUpdate)); var relayoutFlags = relayoutSpecs.flags; From 12881213513e9fc1afc31eb9a1277b6249d33a1a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=B6stl?= Date: Thu, 22 Nov 2018 11:19:19 +0100 Subject: [PATCH 34/49] Clear TODOs in gl3d axis_defaults.js and cloneplot.js [882] - Have been clarified in respective PR. --- src/plots/gl3d/layout/axis_defaults.js | 8 -------- src/snapshot/cloneplot.js | 2 -- 2 files changed, 10 deletions(-) diff --git a/src/plots/gl3d/layout/axis_defaults.js b/src/plots/gl3d/layout/axis_defaults.js index 3a2a0531c87..3c18a06ab69 100644 --- a/src/plots/gl3d/layout/axis_defaults.js +++ b/src/plots/gl3d/layout/axis_defaults.js @@ -58,14 +58,6 @@ module.exports = function supplyLayoutDefaults(layoutIn, layoutOut, options) { coerce('gridcolor', colorMix(containerOut.color, options.bgColor, gridLightness).toRgbString()); coerce('title.text', axName[0]); // shouldn't this be on-par with 2D? - // TODO 882 coercing old 'titlefont' was missing. Why? Activating the below would break a lot image tests... - // Lib.coerceFont(coerce, 'title.font', { - // family: options.font.family, - // size: Math.round(options.font.size), - // color: options.font.color - // }); - - containerOut.setScale = Lib.noop; if(coerce('showspikes')) { diff --git a/src/snapshot/cloneplot.js b/src/snapshot/cloneplot.js index 3b8e35f8b4b..ebda88a12ad 100644 --- a/src/snapshot/cloneplot.js +++ b/src/snapshot/cloneplot.js @@ -85,8 +85,6 @@ module.exports = function clonePlot(graphObj, options) { } } - // TODO 882 Shouldn't ternary and polar axis titles be thrown away as well? - // kill colorbar and pie labels for(i = 0; i < newData.length; i++) { var trace = newData[i]; From ee613bb3c960ff03fd21356e41c40aba8ddb015f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=B6stl?= Date: Fri, 23 Nov 2018 08:45:36 +0100 Subject: [PATCH 35/49] Streamline variable names in subroutines.js [882] - In fact `fullLayout._size` is usually called `gs` for "graph size" when being extracted to a variable. --- src/plot_api/subroutines.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/plot_api/subroutines.js b/src/plot_api/subroutines.js index 7a4b14dd4a8..afc94d4e965 100644 --- a/src/plot_api/subroutines.js +++ b/src/plot_api/subroutines.js @@ -472,7 +472,7 @@ exports.drawMainTitle = function(gd) { function getMainTitleX(fullLayout, textAnchor) { var title = fullLayout.title; - var _size = fullLayout._size; + var gs = fullLayout._size; var hPadShift = 0; if(textAnchor === SVG_TEXT_ANCHOR_START) { @@ -483,7 +483,7 @@ function getMainTitleX(fullLayout, textAnchor) { switch(title.xref) { case 'paper': - return _size.l + _size.w * title.x + hPadShift; + return gs.l + gs.w * title.x + hPadShift; case 'container': default: return fullLayout.width * title.x + hPadShift; @@ -492,7 +492,7 @@ function getMainTitleX(fullLayout, textAnchor) { function getMainTitleY(fullLayout, dy) { var title = fullLayout.title; - var _size = fullLayout._size; + var gs = fullLayout._size; var vPadShift = 0; if(dy === '0em' || !dy) { @@ -502,11 +502,11 @@ function getMainTitleY(fullLayout, dy) { } if(title.y === 'auto') { - return _size.t / 2; + return gs.t / 2; } else { switch(title.yref) { case 'paper': - return _size.t + _size.h - _size.h * title.y + vPadShift; + return gs.t + gs.h - gs.h * title.y + vPadShift; case 'container': default: return fullLayout.height - fullLayout.height * title.y + vPadShift; From 678c2d9053cb82afb7396c8474b1fd9c9ff3c750 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=B6stl?= Date: Fri, 23 Nov 2018 10:21:03 +0100 Subject: [PATCH 36/49] Deprecate old title structure properly [882] - Add _deprecated attributes to attribute definitions. - Mention deprecation in new title attributes. --- src/plots/cartesian/layout_attributes.js | 29 +++++++++++++++++++++--- src/plots/gl3d/layout/axis_attributes.js | 6 ++++- src/plots/layout_attributes.js | 29 ++++++++++++++++++++++-- src/plots/polar/layout_attributes.js | 7 +++++- src/plots/ternary/layout_attributes.js | 4 ++++ 5 files changed, 68 insertions(+), 7 deletions(-) diff --git a/src/plots/cartesian/layout_attributes.js b/src/plots/cartesian/layout_attributes.js index a7bbe0f5727..1f9b064ab7e 100644 --- a/src/plots/cartesian/layout_attributes.js +++ b/src/plots/cartesian/layout_attributes.js @@ -45,12 +45,19 @@ module.exports = { valType: 'string', role: 'info', editType: 'ticks', - description: 'Sets the title of this axis.' + description: [ + 'Sets the title of this axis.', + 'Note that before the existence of `title.text`, the title\'s', + 'contents used to be defined as the `title` attribute itself.', + 'This behavior has been deprecated.' + ].join(' ') }, font: fontAttrs({ editType: 'ticks', description: [ - 'Sets this axis\' title font.' + 'Sets this axis\' title font.', + 'Note that the title\'s font used to be customized', + 'by the now deprecated `titlefont` attribute.' ].join(' ') }), editType: 'ticks' @@ -781,6 +788,22 @@ module.exports = { 'Set `tickmode` to *auto* for old `autotick` *true* behavior.', 'Set `tickmode` to *linear* for `autotick` *false*.' ].join(' ') - } + }, + title: { + valType: 'string', + role: 'info', + editType: 'ticks', + description: [ + 'Value of `title` is no longer a simple *string* but a set of sub-attributes.', + 'To set the axis\' title, please use `title.text` now.' + ].join(' ') + }, + titlefont: fontAttrs({ + editType: 'ticks', + description: [ + 'Former `titlefont` is now the sub-attribute `font` of `title`.', + 'To customize title font properties, please use `title.font` now.' + ].join(' ') + }) } }; diff --git a/src/plots/gl3d/layout/axis_attributes.js b/src/plots/gl3d/layout/axis_attributes.js index 476ba93a82d..e2c2c9482bc 100644 --- a/src/plots/gl3d/layout/axis_attributes.js +++ b/src/plots/gl3d/layout/axis_attributes.js @@ -112,5 +112,9 @@ module.exports = overrideAll({ gridwidth: axesAttrs.gridwidth, zeroline: axesAttrs.zeroline, zerolinecolor: axesAttrs.zerolinecolor, - zerolinewidth: axesAttrs.zerolinewidth + zerolinewidth: axesAttrs.zerolinewidth, + _deprecated: { + title: axesAttrs._deprecated.title, + titlefont: axesAttrs._deprecated.titlefont + } }, 'plot', 'from-root'); diff --git a/src/plots/layout_attributes.js b/src/plots/layout_attributes.js index 1cf743b3639..c6884a2f121 100644 --- a/src/plots/layout_attributes.js +++ b/src/plots/layout_attributes.js @@ -33,12 +33,19 @@ module.exports = { role: 'info', editType: 'layoutstyle', description: [ - 'Sets the plot\'s title.' + 'Sets the plot\'s title.', + 'Note that before the existence of `title.text`, the title\'s', + 'contents used to be defined as the `title` attribute itself.', + 'This behavior has been deprecated.' ].join(' ') }, font: fontAttrs({ editType: 'layoutstyle', - description: 'Sets the title font.' + description: [ + 'Sets the title font.', + 'Note that the title\'s font used to be customized', + 'by the now deprecated `titlefont` attribute.' + ].join(' ') }), xref: { valType: 'enumerated', @@ -346,5 +353,23 @@ module.exports = { description: 'Sets the color of the active or hovered on icons in the modebar.' }, editType: 'modebar' + }, + _deprecated: { + title: { + valType: 'string', + role: 'info', + editType: 'layoutstyle', + description: [ + 'Value of `title` is no longer a simple *string* but a set of sub-attributes.', + 'To set the contents of the title, please use `title.text` now.' + ].join(' ') + }, + titlefont: fontAttrs({ + editType: 'layoutstyle', + description: [ + 'Former `titlefont` is now the sub-attribute `font` of `title`.', + 'To customize title font properties, please use `title.font` now.' + ].join(' ') + }) } }; diff --git a/src/plots/polar/layout_attributes.js b/src/plots/polar/layout_attributes.js index 36340346e50..49a9b47956a 100644 --- a/src/plots/polar/layout_attributes.js +++ b/src/plots/polar/layout_attributes.js @@ -117,7 +117,12 @@ var radialAxisAttrs = { hoverformat: axesAttrs.hoverformat, - editType: 'calc' + editType: 'calc', + + _deprecated: { + title: overrideAll(axesAttrs._deprecated.title, 'plot', 'from-root'), + titlefont: overrideAll(axesAttrs._deprecated.titlefont, 'plot', 'from-root') + } }; extendFlat( diff --git a/src/plots/ternary/layout_attributes.js b/src/plots/ternary/layout_attributes.js index 606765e8c94..86ae1f9d910 100644 --- a/src/plots/ternary/layout_attributes.js +++ b/src/plots/ternary/layout_attributes.js @@ -62,6 +62,10 @@ var ternaryAxesAttrs = { 'values of the other two axes. The full view corresponds to', 'all the minima set to zero.' ].join(' ') + }, + _deprecated: { + title: axesAttrs._deprecated.title, + titlefont: axesAttrs._deprecated.titlefont } }; From 598fc5f52aede69bddba53f1e7f69b0600ff14f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=B6stl?= Date: Fri, 23 Nov 2018 13:33:03 +0100 Subject: [PATCH 37/49] Fix plotschema_test.js to embrace deprecated attribute containers [882] - Reason: deprecating `titlefont` was the first time a container, not only a value object, has been deprecated. --- test/jasmine/bundle_tests/plotschema_test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/jasmine/bundle_tests/plotschema_test.js b/test/jasmine/bundle_tests/plotschema_test.js index 825c537ecc3..2e85825bb65 100644 --- a/test/jasmine/bundle_tests/plotschema_test.js +++ b/test/jasmine/bundle_tests/plotschema_test.js @@ -228,7 +228,7 @@ describe('plot schema', function() { assertPlotSchema( function(attr, attrName, attrs, level, attrString) { - if(attr && isPlainObject(attr[DEPRECATED])) { + if(attr && isPlainObject(attr[DEPRECATED]) && isValObject(attr[DEPRECATED])) { Object.keys(attr[DEPRECATED]).forEach(function(dAttrName) { var dAttr = attr[DEPRECATED][dAttrName]; From 4efb546c97bdc133df732e1aa33d57b06c39cf36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=B6stl?= Date: Fri, 23 Nov 2018 08:47:52 +0100 Subject: [PATCH 38/49] Make explanatory comment in polar.js more clear [882] --- src/plots/polar/polar.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/plots/polar/polar.js b/src/plots/polar/polar.js index 5f501f4c775..bdbf1e82aa6 100644 --- a/src/plots/polar/polar.js +++ b/src/plots/polar/polar.js @@ -485,7 +485,8 @@ proto.updateRadialAxisTitle = function(fullLayout, polarLayout, _angle) { var pad = 0; // Hint: no need to check if there is in fact a title.text set - // because if plot is editable, pad needs to be calculated anyways. + // because if plot is editable, pad needs to be calculated anyways + // to properly show placeholder text when title is empty. if(radialLayout.title) { var h = Drawing.bBox(_this.layers['radial-axis'].node()).height; var ts = radialLayout.title.font.size; From 014548c764bedaa3b9d1d88c51c735c712e02ad9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=B6stl?= Date: Mon, 26 Nov 2018 12:04:10 +0100 Subject: [PATCH 39/49] Skip overriding deprecated title attributes in polar [882] --- src/plots/polar/layout_attributes.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/plots/polar/layout_attributes.js b/src/plots/polar/layout_attributes.js index 49a9b47956a..ec630027526 100644 --- a/src/plots/polar/layout_attributes.js +++ b/src/plots/polar/layout_attributes.js @@ -120,8 +120,8 @@ var radialAxisAttrs = { editType: 'calc', _deprecated: { - title: overrideAll(axesAttrs._deprecated.title, 'plot', 'from-root'), - titlefont: overrideAll(axesAttrs._deprecated.titlefont, 'plot', 'from-root') + title: axesAttrs._deprecated.title, + titlefont: axesAttrs._deprecated.titlefont } }; From 1117113068a80fd2998ccb06578301d7030dd782 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=B6stl?= Date: Mon, 26 Nov 2018 12:30:20 +0100 Subject: [PATCH 40/49] Reuse Lib.counterRegex to match deprecated title string attributes [882] - Add `matchBeginning` parameter to `regex.counter`. --- src/lib/regex.js | 8 +++++--- src/plot_api/plot_api.js | 2 +- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/lib/regex.js b/src/lib/regex.js index acfaff9d41b..ded7c4dfe40 100644 --- a/src/lib/regex.js +++ b/src/lib/regex.js @@ -16,11 +16,13 @@ * @param {Optional(string)} tail: a fixed piece after the id * eg counterRegex('scene', '.annotations') for scene2.annotations etc. * @param {boolean} openEnded: if true, the string may continue past the match. + * @param {boolean} matchBeginning: if false, the string may start before the match. */ -exports.counter = function(head, tail, openEnded) { +exports.counter = function(head, tail, openEnded, matchBeginning) { var fullTail = (tail || '') + (openEnded ? '' : '$'); + var startWithPrefix = matchBeginning === false ? '' : '^'; if(head === 'xy') { - return new RegExp('^x([2-9]|[1-9][0-9]+)?y([2-9]|[1-9][0-9]+)?' + fullTail); + return new RegExp(startWithPrefix + 'x([2-9]|[1-9][0-9]+)?y([2-9]|[1-9][0-9]+)?' + fullTail); } - return new RegExp('^' + head + '([2-9]|[1-9][0-9]+)?' + fullTail); + return new RegExp(startWithPrefix + head + '([2-9]|[1-9][0-9]+)?' + fullTail); }; diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js index c45bd7b016a..b63e57d5a6e 100644 --- a/src/plot_api/plot_api.js +++ b/src/plot_api/plot_api.js @@ -1700,7 +1700,7 @@ function _restyle(gd, aobj, traces) { * @param layoutObj */ function cleanDeprecatedLayoutAttributes(layoutObj) { - var oldAxisTitleRegExp = /axis\d{0,2}\.title$/; + var oldAxisTitleRegExp = Lib.counterRegex('axis', '\.title', false, false); var keys, i, key, value; if(typeof layoutObj.title === 'string') { From c2b4c10ef849bca4afb90c5ab42353fedc15a7f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=B6stl?= Date: Mon, 26 Nov 2018 14:22:31 +0100 Subject: [PATCH 41/49] Pass back update data unmodified to plotly_relayout event handlers [882] - ... even if deprecated layout attributes have been cleaned up internally. - Reason: to stay backwards-compatible with existing event handlers that count on the old titles structure for example. --- src/plot_api/plot_api.js | 3 ++- test/jasmine/tests/legend_test.js | 4 ++-- test/jasmine/tests/plot_api_test.js | 24 ++++++++++++++++++++++++ 3 files changed, 28 insertions(+), 3 deletions(-) diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js index b63e57d5a6e..8e10c48fb19 100644 --- a/src/plot_api/plot_api.js +++ b/src/plot_api/plot_api.js @@ -1859,6 +1859,7 @@ function _relayout(gd, aobj) { var layout = gd.layout, fullLayout = gd._fullLayout, axes = Axes.list(gd), + eventData = Lib.extendDeepAll({}, aobj), arrayEdits = {}, keys, arrayStr, @@ -2194,7 +2195,7 @@ function _relayout(gd, aobj) { rangesAltered: rangesAltered, undoit: undoit, redoit: redoit, - eventData: Lib.extendDeep({}, redoit) + eventData: eventData }; } diff --git a/test/jasmine/tests/legend_test.js b/test/jasmine/tests/legend_test.js index 8f04f0b7918..930fa8cd236 100644 --- a/test/jasmine/tests/legend_test.js +++ b/test/jasmine/tests/legend_test.js @@ -1431,7 +1431,7 @@ describe('legend interaction', function() { }); gd.on('plotly_relayout', function(d) { expect(typeof d).toBe('object'); - expect(d.title.text).toBe('just clicked on trace #2'); + expect(d.title).toBe('just clicked on trace #2'); done(); }); gd.on('plotly_restyle', function() { @@ -1451,7 +1451,7 @@ describe('legend interaction', function() { }); gd.on('plotly_relayout', function(d) { expect(typeof d).toBe('object'); - expect(d.title.text).toBe('just double clicked on trace #0'); + expect(d.title).toBe('just double clicked on trace #0'); done(); }); gd.on('plotly_restyle', function() { diff --git a/test/jasmine/tests/plot_api_test.js b/test/jasmine/tests/plot_api_test.js index 4898c620c41..4d8141cf2e6 100644 --- a/test/jasmine/tests/plot_api_test.js +++ b/test/jasmine/tests/plot_api_test.js @@ -547,6 +547,30 @@ describe('Test plot api', function() { .catch(failTest) .then(done); }); + + it('passes update data back to plotly_relayout unmodified ' + + 'even if deprecated attributes have been used', function(done) { + Plotly.newPlot(gd, [{y: [1, 3, 2]}]); + + gd.on('plotly_relayout', function(eventData) { + expect(eventData).toEqual({ + 'title': 'Plotly chart', + 'xaxis.title': 'X', + 'xaxis.titlefont': {color: 'green'}, + 'yaxis.title': 'Y', + 'polar.radialaxis.title': 'Radial' + }); + done(); + }); + + Plotly.relayout(gd, { + 'title': 'Plotly chart', + 'xaxis.title': 'X', + 'xaxis.titlefont': {color: 'green'}, + 'yaxis.title': 'Y', + 'polar.radialaxis.title': 'Radial' + }); + }); }); describe('Plotly.relayout subroutines switchboard', function() { From 86a88e24db36aedcbc0e5db8a7e6f086a1185c6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=B6stl?= Date: Fri, 23 Nov 2018 08:54:06 +0100 Subject: [PATCH 42/49] Move legend's anchor_utils to Lib and use them in title alignment [882] --- src/components/legend/draw.js | 21 +++++---- src/components/rangeselector/draw.js | 9 ++-- src/components/sliders/draw.js | 9 ++-- src/components/updatemenus/draw.js | 9 ++-- src/lib/anchor_utils.js | 62 ++++++++++++++++++++++++++ src/lib/index.js | 8 ++++ src/plot_api/subroutines.js | 65 +++++++--------------------- test/jasmine/tests/legend_test.js | 9 ++-- 8 files changed, 112 insertions(+), 80 deletions(-) create mode 100644 src/lib/anchor_utils.js diff --git a/src/components/legend/draw.js b/src/components/legend/draw.js index 30d3c66aa7e..391ffd5c9a2 100644 --- a/src/components/legend/draw.js +++ b/src/components/legend/draw.js @@ -30,7 +30,6 @@ var FROM_BR = alignmentConstants.FROM_BR; var getLegendData = require('./get_legend_data'); var style = require('./style'); var helpers = require('./helpers'); -var anchorUtils = require('./anchor_utils'); var DBLCLICKDELAY = interactConstants.DBLCLICKDELAY; @@ -154,17 +153,17 @@ module.exports = function draw(gd) { lx = gs.l + gs.w * opts.x, ly = gs.t + gs.h * (1 - opts.y); - if(anchorUtils.isRightAnchor(opts)) { + if(Lib.isRightAnchor(opts)) { lx -= opts._width; } - else if(anchorUtils.isCenterAnchor(opts)) { + else if(Lib.isCenterAnchor(opts)) { lx -= opts._width / 2; } - if(anchorUtils.isBottomAnchor(opts)) { + if(Lib.isBottomAnchor(opts)) { ly -= opts._height; } - else if(anchorUtils.isMiddleAnchor(opts)) { + else if(Lib.isMiddleAnchor(opts)) { ly -= opts._height / 2; } @@ -699,18 +698,18 @@ function expandMargin(gd) { opts = fullLayout.legend; var xanchor = 'left'; - if(anchorUtils.isRightAnchor(opts)) { + if(Lib.isRightAnchor(opts)) { xanchor = 'right'; } - else if(anchorUtils.isCenterAnchor(opts)) { + else if(Lib.isCenterAnchor(opts)) { xanchor = 'center'; } var yanchor = 'top'; - if(anchorUtils.isBottomAnchor(opts)) { + if(Lib.isBottomAnchor(opts)) { yanchor = 'bottom'; } - else if(anchorUtils.isMiddleAnchor(opts)) { + else if(Lib.isMiddleAnchor(opts)) { yanchor = 'middle'; } @@ -730,10 +729,10 @@ function expandHorizontalMargin(gd) { opts = fullLayout.legend; var xanchor = 'left'; - if(anchorUtils.isRightAnchor(opts)) { + if(Lib.isRightAnchor(opts)) { xanchor = 'right'; } - else if(anchorUtils.isCenterAnchor(opts)) { + else if(Lib.isCenterAnchor(opts)) { xanchor = 'center'; } diff --git a/src/components/rangeselector/draw.js b/src/components/rangeselector/draw.js index aa717d663b5..65209412e70 100644 --- a/src/components/rangeselector/draw.js +++ b/src/components/rangeselector/draw.js @@ -17,7 +17,6 @@ var Drawing = require('../drawing'); var Lib = require('../../lib'); var svgTextUtils = require('../../lib/svg_text_utils'); var axisIds = require('../../plots/cartesian/axis_ids'); -var anchorUtils = require('../legend/anchor_utils'); var alignmentConstants = require('../../constants/alignment'); var LINE_SPACING = alignmentConstants.LINE_SPACING; @@ -218,21 +217,21 @@ function reposition(gd, buttons, opts, axName, selector) { var ly = graphSize.t + graphSize.h * (1 - opts.y); var xanchor = 'left'; - if(anchorUtils.isRightAnchor(opts)) { + if(Lib.isRightAnchor(opts)) { lx -= width; xanchor = 'right'; } - if(anchorUtils.isCenterAnchor(opts)) { + if(Lib.isCenterAnchor(opts)) { lx -= width / 2; xanchor = 'center'; } var yanchor = 'top'; - if(anchorUtils.isBottomAnchor(opts)) { + if(Lib.isBottomAnchor(opts)) { ly -= height; yanchor = 'bottom'; } - if(anchorUtils.isMiddleAnchor(opts)) { + if(Lib.isMiddleAnchor(opts)) { ly -= height / 2; yanchor = 'middle'; } diff --git a/src/components/sliders/draw.js b/src/components/sliders/draw.js index 62557f92ded..b224d940176 100644 --- a/src/components/sliders/draw.js +++ b/src/components/sliders/draw.js @@ -15,7 +15,6 @@ var Color = require('../color'); var Drawing = require('../drawing'); var Lib = require('../../lib'); var svgTextUtils = require('../../lib/svg_text_utils'); -var anchorUtils = require('../legend/anchor_utils'); var arrayEditor = require('../../plot_api/plot_template').arrayEditor; var constants = require('./constants'); @@ -207,21 +206,21 @@ function findDimensions(gd, sliderOpts) { dims.height = dims.currentValueTotalHeight + constants.tickOffset + sliderOpts.ticklen + constants.labelOffset + dims.labelHeight + sliderOpts.pad.t + sliderOpts.pad.b; var xanchor = 'left'; - if(anchorUtils.isRightAnchor(sliderOpts)) { + if(Lib.isRightAnchor(sliderOpts)) { dims.lx -= dims.outerLength; xanchor = 'right'; } - if(anchorUtils.isCenterAnchor(sliderOpts)) { + if(Lib.isCenterAnchor(sliderOpts)) { dims.lx -= dims.outerLength / 2; xanchor = 'center'; } var yanchor = 'top'; - if(anchorUtils.isBottomAnchor(sliderOpts)) { + if(Lib.isBottomAnchor(sliderOpts)) { dims.ly -= dims.height; yanchor = 'bottom'; } - if(anchorUtils.isMiddleAnchor(sliderOpts)) { + if(Lib.isMiddleAnchor(sliderOpts)) { dims.ly -= dims.height / 2; yanchor = 'middle'; } diff --git a/src/components/updatemenus/draw.js b/src/components/updatemenus/draw.js index f1e5a9adaad..778e01b685e 100644 --- a/src/components/updatemenus/draw.js +++ b/src/components/updatemenus/draw.js @@ -16,7 +16,6 @@ var Color = require('../color'); var Drawing = require('../drawing'); var Lib = require('../../lib'); var svgTextUtils = require('../../lib/svg_text_utils'); -var anchorUtils = require('../legend/anchor_utils'); var arrayEditor = require('../../plot_api/plot_template').arrayEditor; var LINE_SPACING = require('../../constants/alignment').LINE_SPACING; @@ -566,21 +565,21 @@ function findDimensions(gd, menuOpts) { dims.ly = graphSize.t + graphSize.h * (1 - menuOpts.y); var xanchor = 'left'; - if(anchorUtils.isRightAnchor(menuOpts)) { + if(Lib.isRightAnchor(menuOpts)) { dims.lx -= paddedWidth; xanchor = 'right'; } - if(anchorUtils.isCenterAnchor(menuOpts)) { + if(Lib.isCenterAnchor(menuOpts)) { dims.lx -= paddedWidth / 2; xanchor = 'center'; } var yanchor = 'top'; - if(anchorUtils.isBottomAnchor(menuOpts)) { + if(Lib.isBottomAnchor(menuOpts)) { dims.ly -= paddedHeight; yanchor = 'bottom'; } - if(anchorUtils.isMiddleAnchor(menuOpts)) { + if(Lib.isMiddleAnchor(menuOpts)) { dims.ly -= paddedHeight / 2; yanchor = 'middle'; } diff --git a/src/lib/anchor_utils.js b/src/lib/anchor_utils.js new file mode 100644 index 00000000000..0c61fc36efc --- /dev/null +++ b/src/lib/anchor_utils.js @@ -0,0 +1,62 @@ +/** +* Copyright 2012-2018, 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'; + + +/** + * Determine the position anchor property of x/y xanchor/yanchor components. + * + * - values < 1/3 align the low side at that fraction, + * - values [1/3, 2/3] align the center at that fraction, + * - values > 2/3 align the right at that fraction. + */ + + +exports.isLeftAnchor = function isLeftAnchor(opts) { + return ( + opts.xanchor === 'left' || + (opts.xanchor === 'auto' && opts.x <= 1 / 3) + ); +}; + +exports.isCenterAnchor = function isCenterAnchor(opts) { + return ( + opts.xanchor === 'center' || + (opts.xanchor === 'auto' && opts.x > 1 / 3 && opts.x < 2 / 3) + ); +}; + +exports.isRightAnchor = function isRightAnchor(opts) { + return ( + opts.xanchor === 'right' || + (opts.xanchor === 'auto' && opts.x >= 2 / 3) + ); +}; + +exports.isTopAnchor = function isTopAnchor(opts) { + return ( + opts.yanchor === 'top' || + (opts.yanchor === 'auto' && opts.y >= 2 / 3) + ); +}; + +exports.isMiddleAnchor = function isMiddleAnchor(opts) { + return ( + opts.yanchor === 'middle' || + (opts.yanchor === 'auto' && opts.y > 1 / 3 && opts.y < 2 / 3) + ); +}; + +exports.isBottomAnchor = function isBottomAnchor(opts) { + return ( + opts.yanchor === 'bottom' || + (opts.yanchor === 'auto' && opts.y <= 1 / 3) + ); +}; diff --git a/src/lib/index.js b/src/lib/index.js index 5b329d5c223..2d9de4b1ac1 100644 --- a/src/lib/index.js +++ b/src/lib/index.js @@ -100,6 +100,14 @@ lib.pathArc = anglesModule.pathArc; lib.pathSector = anglesModule.pathSector; lib.pathAnnulus = anglesModule.pathAnnulus; +var anchorUtils = require('./anchor_utils'); +lib.isLeftAnchor = anchorUtils.isLeftAnchor; +lib.isCenterAnchor = anchorUtils.isCenterAnchor; +lib.isRightAnchor = anchorUtils.isRightAnchor; +lib.isTopAnchor = anchorUtils.isTopAnchor; +lib.isMiddleAnchor = anchorUtils.isMiddleAnchor; +lib.isBottomAnchor = anchorUtils.isBottomAnchor; + var geom2dModule = require('./geometry2d'); lib.segmentsIntersect = geom2dModule.segmentsIntersect; lib.segmentDistance = geom2dModule.segmentDistance; diff --git a/src/plot_api/subroutines.js b/src/plot_api/subroutines.js index afc94d4e965..ebdaa9409d5 100644 --- a/src/plot_api/subroutines.js +++ b/src/plot_api/subroutines.js @@ -515,62 +515,29 @@ function getMainTitleY(fullLayout, dy) { } function getMainTitleTextAnchor(fullLayout) { - var xanchor = fullLayout.title.xanchor; - - switch(xanchor) { - case 'auto': - return calcTextAnchor(fullLayout.title.x); - case 'left': - return SVG_TEXT_ANCHOR_START; - case 'right': - return SVG_TEXT_ANCHOR_END; - case 'center': - default: - return SVG_TEXT_ANCHOR_MIDDLE; - } - - function calcTextAnchor(x) { - if(x < 1 / 3) { - return SVG_TEXT_ANCHOR_START; - } else if(x > 2 / 3) { - return SVG_TEXT_ANCHOR_END; - } + var title = fullLayout.title; - return SVG_TEXT_ANCHOR_MIDDLE; + var textAnchor = SVG_TEXT_ANCHOR_MIDDLE; + if(Lib.isRightAnchor(title)) { + textAnchor = SVG_TEXT_ANCHOR_END; + } else if(Lib.isLeftAnchor(title)) { + textAnchor = SVG_TEXT_ANCHOR_START; } + + return textAnchor; } function getMainTitleDy(fullLayout) { - var yanchor = fullLayout.title.yanchor; - switch(yanchor) { - case 'auto': - return calcDy(fullLayout.title.y); - case 'middle': - return alignmentConstants.MID_SHIFT + 'em'; - case 'top': - return alignmentConstants.CAP_SHIFT + 'em'; - case 'bottom': - default: - return '0em'; - } - - function calcDy(y) { - - // Imitate behavior before "chart title alignment" was introduced - if(y === 'auto') { - return '0em'; - } else if(typeof y === 'number') { - if(y < 1 / 3) { - return '0em'; - } else if(y > 2 / 3) { - return alignmentConstants.CAP_SHIFT + 'em'; - } - - return alignmentConstants.MID_SHIFT + 'em'; - } + var title = fullLayout.title; - return 0; + var dy = '0em'; + if(Lib.isTopAnchor(title)) { + dy = alignmentConstants.CAP_SHIFT + 'em'; + } else if(Lib.isMiddleAnchor(title)) { + dy = alignmentConstants.MID_SHIFT + 'em'; } + + return dy; } exports.doTraceStyle = function(gd) { diff --git a/test/jasmine/tests/legend_test.js b/test/jasmine/tests/legend_test.js index 930fa8cd236..54e367decbd 100644 --- a/test/jasmine/tests/legend_test.js +++ b/test/jasmine/tests/legend_test.js @@ -6,7 +6,6 @@ var DBLCLICKDELAY = require('@src/constants/interactions').DBLCLICKDELAY; var Legend = require('@src/components/legend'); var getLegendData = require('@src/components/legend/get_legend_data'); var helpers = require('@src/components/legend/helpers'); -var anchorUtils = require('@src/components/legend/anchor_utils'); var d3 = require('d3'); var failTest = require('../assets/fail_test'); @@ -501,7 +500,7 @@ describe('legend anchor utils:', function() { 'use strict'; describe('isRightAnchor', function() { - var isRightAnchor = anchorUtils.isRightAnchor; + var isRightAnchor = Lib.isRightAnchor; var threshold = 2 / 3; it('should return true when \'xanchor\' is set to \'right\'', function() { @@ -522,7 +521,7 @@ describe('legend anchor utils:', function() { }); describe('isCenterAnchor', function() { - var isCenterAnchor = anchorUtils.isCenterAnchor; + var isCenterAnchor = Lib.isCenterAnchor; var threshold0 = 1 / 3; var threshold1 = 2 / 3; @@ -544,7 +543,7 @@ describe('legend anchor utils:', function() { }); describe('isBottomAnchor', function() { - var isBottomAnchor = anchorUtils.isBottomAnchor; + var isBottomAnchor = Lib.isBottomAnchor; var threshold = 1 / 3; it('should return true when \'yanchor\' is set to \'right\'', function() { @@ -565,7 +564,7 @@ describe('legend anchor utils:', function() { }); describe('isMiddleAnchor', function() { - var isMiddleAnchor = anchorUtils.isMiddleAnchor; + var isMiddleAnchor = Lib.isMiddleAnchor; var threshold0 = 1 / 3; var threshold1 = 2 / 3; From 5fb7b8f4aa180ee894dce63646510407fb535777 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=B6stl?= Date: Mon, 26 Nov 2018 16:09:35 +0100 Subject: [PATCH 43/49] Redeclare valType of title.y to be a 'number' [882] - Swichting the valType to 'number' spares the custom coerce function. - In addition fixes the missing min and max bounds for y. --- src/plots/layout_attributes.js | 4 +++- src/plots/plots.js | 15 +-------------- 2 files changed, 4 insertions(+), 15 deletions(-) diff --git a/src/plots/layout_attributes.js b/src/plots/layout_attributes.js index c6884a2f121..30a6fadb291 100644 --- a/src/plots/layout_attributes.js +++ b/src/plots/layout_attributes.js @@ -84,7 +84,9 @@ module.exports = { ].join(' ') }, y: { - valType: 'any', + valType: 'number', + min: 0, + max: 1, dflt: 'auto', role: 'style', editType: 'layoutstyle', diff --git a/src/plots/plots.js b/src/plots/plots.js index 9d7cc9f887f..0fd72fd7341 100644 --- a/src/plots/plots.js +++ b/src/plots/plots.js @@ -1315,19 +1315,6 @@ plots.supplyLayoutGlobalDefaults = function(layoutIn, layoutOut, formatObj) { return Lib.coerce(layoutIn, layoutOut, plots.layoutAttributes, attr, dflt); } - // Special treatment for title.y that can be 'auto' - // or a number between 0 and 1. - function coerceTitleY() { - if(layoutIn.title && isNumeric(layoutIn.title.y)) { - var propOut = Lib.nestedProperty(layoutOut, 'title.y'); - var opts = Lib.nestedProperty(plots.layoutAttributes, 'title.y').get(); - - Lib.valObjectMeta.number.coerceFunction(layoutIn.title.y, propOut, 'auto', opts); - } else { - coerce('title.y'); - } - } - var template = layoutIn.template; if(Lib.isPlainObject(template)) { layoutOut.template = template; @@ -1348,7 +1335,7 @@ plots.supplyLayoutGlobalDefaults = function(layoutIn, layoutOut, formatObj) { coerce('title.xref'); coerce('title.yref'); coerce('title.x'); - coerceTitleY(); + coerce('title.y'); coerce('title.xanchor'); coerce('title.yanchor'); coerce('title.pad.t'); From a2f0543c8a64e63d1246e5d49b06a570ec90344d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=B6stl?= Date: Tue, 27 Nov 2018 12:00:09 +0100 Subject: [PATCH 44/49] Transition pie trace type to new title attr structure [882] --- src/plot_api/helpers.js | 25 ++++++++-- src/plot_api/plot_api.js | 43 ++++++++-------- src/traces/pie/attributes.js | 89 +++++++++++++++++++++++++--------- src/traces/pie/defaults.js | 8 +-- src/traces/pie/plot.js | 32 ++++++------ test/jasmine/tests/lib_test.js | 10 ++-- test/jasmine/tests/pie_test.js | 89 ++++++++++++++++++++++++++++++---- 7 files changed, 211 insertions(+), 85 deletions(-) diff --git a/src/plot_api/helpers.js b/src/plot_api/helpers.js index 104da465209..35caf22619e 100644 --- a/src/plot_api/helpers.js +++ b/src/plot_api/helpers.js @@ -245,13 +245,26 @@ function cleanTitle(titleContainer) { var oldFontAttrSet = Lib.isPlainObject(titleContainer.titlefont); var newFontAttrSet = titleContainer.title && Lib.isPlainObject(titleContainer.title.font); if(oldFontAttrSet && !newFontAttrSet) { - if(!titleContainer.title) { - titleContainer.title = {}; - } + nestTitleAttr('titlefont', 'font'); + } + + // titleposition -> title.position + var oldPositionAttrSet = titleContainer.titleposition; + var newPositionAttrSet = titleContainer.title && titleContainer.title.position; + if(oldPositionAttrSet && !newPositionAttrSet) { + nestTitleAttr('titleposition', 'position'); + } + } + + function nestTitleAttr(oldAttrName, newAttrName) { - titleContainer.title.font = titleContainer.titlefont; - delete titleContainer.titlefont; + // Ensure title object exists + if(!titleContainer.title) { + titleContainer.title = {}; } + + titleContainer.title[newAttrName] = titleContainer[oldAttrName]; + delete titleContainer[oldAttrName]; } } @@ -458,6 +471,8 @@ exports.cleanData = function(data) { delete trace.autobiny; delete trace.ybins; } + + cleanTitle(trace); } }; diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js index a31b3694c70..5962ef081bc 100644 --- a/src/plot_api/plot_api.js +++ b/src/plot_api/plot_api.js @@ -1405,8 +1405,11 @@ function _restyle(gd, aobj, traces) { var fullLayout = gd._fullLayout, fullData = gd._fullData, data = gd.data, + eventData = Lib.extendDeepAll({}, aobj), i; + cleanDeprecatedAttributeKeys(aobj); + // initialize flags var flags = editTypes.traceFlags(); @@ -1691,49 +1694,45 @@ function _restyle(gd, aobj, traces) { undoit: undoit, redoit: redoit, traces: traces, - eventData: Lib.extendDeepNoArrays([], [redoit, traces]) + eventData: Lib.extendDeepNoArrays([], [eventData, traces]) }; } /** - * Converts deprecated layout attributes and "string attributes" alike to + * Converts deprecated attribute keys to * the current API to ensure backwards compatibility. * - * This is needed for the relayout mechanism to determine which - * subroutines to run based on the actual layout attribute + * This is needed for the update mechanism to determine which + * subroutines to run based on the actual attribute * definitions (that don't include the deprecated ones). * * E.g. Maps {'xaxis.title': 'A chart'} to {'xaxis.title.text': 'A chart'} * and {titlefont: {...}} to {'title.font': {...}}. * - * @param layoutObj + * @param aobj */ -function cleanDeprecatedLayoutAttributes(layoutObj) { +function cleanDeprecatedAttributeKeys(aobj) { var oldAxisTitleRegExp = Lib.counterRegex('axis', '\.title', false, false); - var keys, i, key, value; - - if(typeof layoutObj.title === 'string') { - layoutObj.title = {text: layoutObj.title}; - } + var keys = Object.keys(aobj); + var i, key, value; - keys = Object.keys(layoutObj); for(i = 0; i < keys.length; i++) { key = keys[i]; - value = layoutObj[key]; + value = aobj[key]; - if(key.indexOf('titlefont') > -1) { - replace(key, key.replace('titlefont', 'title.font')); - } - - if((typeof value === 'string' || typeof value === 'number') && - oldAxisTitleRegExp.test(key)) { + if((key === 'title' || oldAxisTitleRegExp.test(key)) && + (typeof value === 'string' || typeof value === 'number')) { replace(key, key.replace('title', 'title.text')); + } else if(key.indexOf('titlefont') > -1) { + replace(key, key.replace('titlefont', 'title.font')); + } else if(key.indexOf('titleposition') > -1) { + replace(key, key.replace('titleposition', 'title.position')); } } function replace(oldAttrStr, newAttrStr) { - layoutObj[newAttrStr] = layoutObj[oldAttrStr]; - delete layoutObj[oldAttrStr]; + aobj[newAttrStr] = aobj[oldAttrStr]; + delete aobj[oldAttrStr]; } } @@ -1875,7 +1874,7 @@ function _relayout(gd, aobj) { i, j; - cleanDeprecatedLayoutAttributes(aobj); + cleanDeprecatedAttributeKeys(aobj); keys = Object.keys(aobj); // look for 'allaxes', split out into all axes diff --git a/src/traces/pie/attributes.js b/src/traces/pie/attributes.js index 1b42870a83d..a01385a23bd 100644 --- a/src/traces/pie/attributes.js +++ b/src/traces/pie/attributes.js @@ -185,31 +185,44 @@ module.exports = { }), title: { - valType: 'string', - dflt: '', - role: 'info', - editType: 'calc', - description: [ - 'Sets the title of the pie chart.', - 'If it is empty, no title is displayed.' - ].join(' ') - }, - titleposition: { - valType: 'enumerated', - values: [ - 'top left', 'top center', 'top right', - 'middle center', - 'bottom left', 'bottom center', 'bottom right' - ], - role: 'info', - editType: 'calc', - description: [ - 'Specifies the location of the `title`.', - ].join(' ') + text: { + valType: 'string', + dflt: '', + role: 'info', + editType: 'calc', + description: [ + 'Sets the title of the pie chart.', + 'If it is empty, no title is displayed.', + 'Note that before the existence of `title.text`, the title\'s', + 'contents used to be defined as the `title` attribute itself.', + 'This behavior has been deprecated.' + ].join(' ') + }, + font: extendFlat({}, textFontAttrs, { + description: [ + 'Sets the font used for `title`.', + 'Note that the title\'s font used to be set', + 'by the now deprecated `titlefont` attribute.' + ].join(' ') + }), + position: { + valType: 'enumerated', + values: [ + 'top left', 'top center', 'top right', + 'middle center', + 'bottom left', 'bottom center', 'bottom right' + ], + role: 'info', + editType: 'calc', + description: [ + 'Specifies the location of the `title`.', + 'Note that the title\'s position used to be set', + 'by the now deprecated `titleposition` attribute.' + ].join(' ') + }, + + editType: 'calc' }, - titlefont: extendFlat({}, textFontAttrs, { - description: 'Sets the font used for `title`.' - }), // position and shape domain: domainAttrs({name: 'pie', trace: true, editType: 'calc'}), @@ -283,5 +296,33 @@ module.exports = { 'to pull all slices apart from each other equally', 'or an array to highlight one or more slices.' ].join(' ') + }, + + _deprecated: { + title: { + valType: 'string', + dflt: '', + role: 'info', + editType: 'calc', + description: [ + 'Deprecated in favor of `title.text`.', + 'Note that value of `title` is no longer a simple', + '*string* but a set of sub-attributes.' + ].join(' ') + }, + titlefont: extendFlat({}, textFontAttrs, { + description: 'Deprecated in favor of `title.font`.' + }), + titleposition: { + valType: 'enumerated', + values: [ + 'top left', 'top center', 'top right', + 'middle center', + 'bottom left', 'bottom center', 'bottom right' + ], + role: 'info', + editType: 'calc', + description: 'Deprecated in favor of `title.position`.' + } } }; diff --git a/src/traces/pie/defaults.js b/src/traces/pie/defaults.js index 4b3c156140a..2110884688c 100644 --- a/src/traces/pie/defaults.js +++ b/src/traces/pie/defaults.js @@ -77,11 +77,11 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout handleDomainDefaults(traceOut, layout, coerce); var hole = coerce('hole'); - var title = coerce('title'); + var title = coerce('title.text'); if(title) { - var titlePosition = coerce('titleposition', hole ? 'middle center' : 'top center'); - if(!hole && titlePosition === 'middle center') traceOut.titleposition = 'top center'; - coerceFont(coerce, 'titlefont', layout.font); + var titlePosition = coerce('title.position', hole ? 'middle center' : 'top center'); + if(!hole && titlePosition === 'middle center') traceOut.title.position = 'top center'; + coerceFont(coerce, 'title.font', layout.font); } coerce('sort'); diff --git a/src/traces/pie/plot.js b/src/traces/pie/plot.js index eb41487af80..5eff4d6fc07 100644 --- a/src/traces/pie/plot.js +++ b/src/traces/pie/plot.js @@ -322,7 +322,7 @@ module.exports = function plot(gd, cdpie) { // add the title var titleTextGroup = d3.select(this).selectAll('g.titletext') - .data(trace.title ? [0] : []); + .data(trace.title.text ? [0] : []); titleTextGroup.enter().append('g') .classed('titletext', true); @@ -334,18 +334,18 @@ module.exports = function plot(gd, cdpie) { s.attr('data-notex', 1); }); - titleText.text(trace.title) + titleText.text(trace.title.text) .attr({ 'class': 'titletext', transform: '', 'text-anchor': 'middle', }) - .call(Drawing.font, trace.titlefont) + .call(Drawing.font, trace.title.font) .call(svgTextUtils.convertToTspans, gd); var transform; - if(trace.titleposition === 'middle center') { + if(trace.title.position === 'middle center') { transform = positionTitleInside(cd0); } else { transform = positionTitleOutside(cd0, fullLayout._size); @@ -473,11 +473,11 @@ function prerenderTitles(cdpie, gd) { cd0 = cdpie[i][0]; trace = cd0.trace; - if(trace.title) { + if(trace.title.text) { var dummyTitle = Drawing.tester.append('text') .attr('data-notex', 1) - .text(trace.title) - .call(Drawing.font, trace.titlefont) + .text(trace.title.text) + .call(Drawing.font, trace.title.font) .call(svgTextUtils.convertToTspans, gd); var bBox = Drawing.bBox(dummyTitle.node(), true); cd0.titleBox = { @@ -579,7 +579,7 @@ function positionTitleInside(cd0) { y: cd0.cy, scale: cd0.trace.hole * cd0.r * 2 / textDiameter, tx: 0, - ty: - cd0.titleBox.height / 2 + cd0.trace.titlefont.size + ty: - cd0.titleBox.height / 2 + cd0.trace.title.font.size }; } @@ -602,25 +602,25 @@ function positionTitleOutside(cd0, plotSize) { // we reason below as if the baseline is the top middle point of the text box. // so we must add the font size to approximate the y-coord. of the top. // note that this correction must happen after scaling. - translate.ty += trace.titlefont.size; + translate.ty += trace.title.font.size; maxPull = getMaxPull(trace); - if(trace.titleposition.indexOf('top') !== -1) { + if(trace.title.position.indexOf('top') !== -1) { topMiddle.y -= (1 + maxPull) * cd0.r; translate.ty -= cd0.titleBox.height; } - else if(trace.titleposition.indexOf('bottom') !== -1) { + else if(trace.title.position.indexOf('bottom') !== -1) { topMiddle.y += (1 + maxPull) * cd0.r; } - if(trace.titleposition.indexOf('left') !== -1) { + if(trace.title.position.indexOf('left') !== -1) { // we start the text at the left edge of the pie maxWidth = plotSize.w * (trace.domain.x[1] - trace.domain.x[0]) / 2 + cd0.r; topMiddle.x -= (1 + maxPull) * cd0.r; translate.tx += cd0.titleBox.width / 2; - } else if(trace.titleposition.indexOf('center') !== -1) { + } else if(trace.title.position.indexOf('center') !== -1) { maxWidth = plotSize.w * (trace.domain.x[1] - trace.domain.x[0]); - } else if(trace.titleposition.indexOf('right') !== -1) { + } else if(trace.title.position.indexOf('right') !== -1) { maxWidth = plotSize.w * (trace.domain.x[1] - trace.domain.x[0]) / 2 + cd0.r; topMiddle.x += (1 + maxPull) * cd0.r; translate.tx -= cd0.titleBox.width / 2; @@ -774,7 +774,7 @@ function scalePies(cdpie, plotSize) { pieBoxWidth = plotSize.w * (trace.domain.x[1] - trace.domain.x[0]); pieBoxHeight = plotSize.h * (trace.domain.y[1] - trace.domain.y[0]); // leave some space for the title, if it will be displayed outside - if(trace.title && trace.titleposition !== 'middle center') { + if(trace.title.text && trace.title.position !== 'middle center') { pieBoxHeight -= getTitleSpace(cd0, plotSize); } @@ -784,7 +784,7 @@ function scalePies(cdpie, plotSize) { cd0.cx = plotSize.l + plotSize.w * (trace.domain.x[1] + trace.domain.x[0]) / 2; cd0.cy = plotSize.t + plotSize.h * (1 - trace.domain.y[0]) - pieBoxHeight / 2; - if(trace.title && trace.titleposition.indexOf('bottom') !== -1) { + if(trace.title.text && trace.title.position.indexOf('bottom') !== -1) { cd0.cy -= getTitleSpace(cd0, plotSize); } diff --git a/test/jasmine/tests/lib_test.js b/test/jasmine/tests/lib_test.js index 7524a6be641..65c28b9fe1d 100644 --- a/test/jasmine/tests/lib_test.js +++ b/test/jasmine/tests/lib_test.js @@ -2658,12 +2658,12 @@ describe('Queue', function() { expect(gd.undoQueue.queue[0].undo.args[0][1]['marker.color']).toEqual([null]); expect(gd.undoQueue.queue[0].redo.args[0][1]['marker.color']).toEqual('red'); - return Plotly.relayout(gd, 'title', 'A title'); + return Plotly.relayout(gd, 'title.text', 'A title'); }) .then(function() { expect(gd.undoQueue.index).toEqual(2); - expect(gd.undoQueue.queue[1].undo.args[0][1].title).toEqual(null); - expect(gd.undoQueue.queue[1].redo.args[0][1].title.text).toEqual('A title'); + expect(gd.undoQueue.queue[1].undo.args[0][1]['title.text']).toEqual(null); + expect(gd.undoQueue.queue[1].redo.args[0][1]['title.text']).toEqual('A title'); return Plotly.restyle(gd, 'mode', 'markers'); }) @@ -2674,8 +2674,8 @@ describe('Queue', function() { expect(gd.undoQueue.queue[1].undo.args[0][1].mode).toEqual([null]); expect(gd.undoQueue.queue[1].redo.args[0][1].mode).toEqual('markers'); - expect(gd.undoQueue.queue[0].undo.args[0][1].title).toEqual(null); - expect(gd.undoQueue.queue[0].redo.args[0][1].title.text).toEqual('A title'); + expect(gd.undoQueue.queue[0].undo.args[0][1]['title.text']).toEqual(null); + expect(gd.undoQueue.queue[0].redo.args[0][1]['title.text']).toEqual('A title'); return Plotly.restyle(gd, 'transforms[0]', { type: 'filter' }); }) diff --git a/test/jasmine/tests/pie_test.js b/test/jasmine/tests/pie_test.js index 0a9cfd04866..e131248ce48 100644 --- a/test/jasmine/tests/pie_test.js +++ b/test/jasmine/tests/pie_test.js @@ -692,29 +692,100 @@ describe('Pie traces', function() { }); }); - it('should be able to restyle title color', function(done) { - function _assert(msg, exp) { - var title = d3.select('.titletext > text').node(); - expect(title.style.fill).toBe(exp.color, msg); - } + function _assertTitle(msg, expText, expColor) { + var title = d3.select('.titletext > text'); + expect(title.text()).toBe(expText, msg + ' text'); + expect(title.node().style.fill).toBe(expColor, msg + ' color'); + } + it('show a user-defined title with a custom position and font', function(done) { + Plotly.plot(gd, [{ + type: 'pie', + values: [1, 2, 3], + title: { + text: 'yo', + font: {color: 'blue'}, + position: 'top left' + } + }]) + .then(function() { + _assertTitle('base', 'yo', 'rgb(0, 0, 255)'); + _verifyTitle(true, false, true, false, false); + }) + .catch(failTest) + .then(done); + }); + + it('still support the deprecated `title` structure (backwards-compatibility)', function(done) { Plotly.plot(gd, [{ type: 'pie', values: [1, 2, 3], title: 'yo', - titlefont: {color: 'blue'} + titlefont: {color: 'blue'}, + titleposition: 'top left' + }]) + .then(function() { + _assertTitle('base', 'yo', 'rgb(0, 0, 255)'); + _verifyTitle(true, false, true, false, false); + }) + .catch(failTest) + .then(done); + }); + + it('should be able to restyle title', function(done) { + Plotly.plot(gd, [{ + type: 'pie', + values: [1, 2, 3], + title: { + text: 'yo', + font: {color: 'blue'}, + position: 'top left' + } }]) .then(function() { - _assert('base', {color: 'rgb(0, 0, 255)'}); - return Plotly.restyle(gd, 'titlefont.color', 'red'); + _assertTitle('base', 'yo', 'rgb(0, 0, 255)'); + _verifyTitle(true, false, true, false, false); + + return Plotly.restyle(gd, { + 'title.text': 'oy', + 'title.font.color': 'red', + 'title.position': 'bottom right' + }); }) .then(function() { - _assert('base', {color: 'rgb(255, 0, 0)'}); + _assertTitle('base', 'oy', 'rgb(255, 0, 0)'); + _verifyTitle(false, true, false, true, false); }) .catch(failTest) .then(done); }); + it('should be able to restyle title despite using the deprecated attributes', function(done) { + Plotly.plot(gd, [{ + type: 'pie', + values: [1, 2, 3], + title: 'yo', + titlefont: {color: 'blue'}, + titleposition: 'top left' + }]) + .then(function() { + _assertTitle('base', 'yo', 'rgb(0, 0, 255)'); + _verifyTitle(true, false, true, false, false); + + return Plotly.restyle(gd, { + 'title': 'oy', + 'titlefont.color': 'red', + 'titleposition': 'bottom right' + }); + }) + .then(function() { + _assertTitle('base', 'oy', 'rgb(255, 0, 0)'); + _verifyTitle(false, true, false, true, false); + }) + .catch(failTest) + .then(done); + }); + it('should be able to react with new text colors', function(done) { Plotly.plot(gd, [{ type: 'pie', From a4e43a6ab42304b4bb806c1dfcf9e18f7c781120 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=B6stl?= Date: Tue, 27 Nov 2018 15:47:44 +0100 Subject: [PATCH 45/49] Transition color bar to new title attr structure [882] - This transitions all traces depending on `colorbar` as well. --- src/components/colorbar/attributes.js | 65 ++++++++++++++++++++------- src/components/colorbar/defaults.js | 6 +-- src/components/colorbar/draw.js | 34 ++++++-------- src/plot_api/helpers.js | 10 +++++ src/plot_api/plot_api.js | 7 ++- 5 files changed, 81 insertions(+), 41 deletions(-) diff --git a/src/components/colorbar/attributes.js b/src/components/colorbar/attributes.js index eb1efe65552..8a5b68018ea 100644 --- a/src/components/colorbar/attributes.js +++ b/src/components/colorbar/attributes.js @@ -175,21 +175,56 @@ module.exports = overrideAll({ exponentformat: axesAttrs.exponentformat, showexponent: axesAttrs.showexponent, title: { - valType: 'string', - role: 'info', - description: 'Sets the title of the color bar.' + text: { + valType: 'string', + role: 'info', + description: [ + 'Sets the title of the color bar.', + 'Note that before the existence of `title.text`, the title\'s', + 'contents used to be defined as the `title` attribute itself.', + 'This behavior has been deprecated.' + ].join(' ') + }, + font: fontAttrs({ + description: [ + 'Sets this color bar\'s title font.', + 'Note that the title\'s font used to be set', + 'by the now deprecated `titlefont` attribute.' + ].join(' ') + }), + side: { + valType: 'enumerated', + values: ['right', 'top', 'bottom'], + role: 'style', + dflt: 'top', + description: [ + 'Determines the location of color bar\'s title', + 'with respect to the color bar.', + 'Note that the title\'s location used to be set', + 'by the now deprecated `titleside` attribute.' + ].join(' ') + } }, - titlefont: fontAttrs({ - description: 'Sets this color bar\'s title font.' - }), - titleside: { - valType: 'enumerated', - values: ['right', 'top', 'bottom'], - role: 'style', - dflt: 'top', - description: [ - 'Determines the location of the colorbar title', - 'with respect to the color bar.' - ].join(' ') + + _deprecated: { + title: { + valType: 'string', + role: 'info', + description: [ + 'Deprecated in favor of color bar\'s `title.text`.', + 'Note that value of color bar\'s `title` is no longer a simple', + '*string* but a set of sub-attributes.' + ].join(' ') + }, + titlefont: fontAttrs({ + description: 'Deprecated in favor of color bar\'s `title.font`.' + }), + titleside: { + valType: 'enumerated', + values: ['right', 'top', 'bottom'], + role: 'style', + dflt: 'top', + description: 'Deprecated in favor of color bar\'s `title.side`.' + } } }, 'colorbars', 'from-root'); diff --git a/src/components/colorbar/defaults.js b/src/components/colorbar/defaults.js index 6ad1e3d5cbb..8225f2d0801 100644 --- a/src/components/colorbar/defaults.js +++ b/src/components/colorbar/defaults.js @@ -59,7 +59,7 @@ module.exports = function colorbarDefaults(containerIn, containerOut, layout) { handleTickLabelDefaults(colorbarIn, colorbarOut, coerce, 'linear', opts); handleTickMarkDefaults(colorbarIn, colorbarOut, coerce, 'linear', opts); - coerce('title', layout._dfltTitle.colorbar); - Lib.coerceFont(coerce, 'titlefont', layout.font); - coerce('titleside'); + coerce('title.text', layout._dfltTitle.colorbar); + Lib.coerceFont(coerce, 'title.font', layout.font); + coerce('title.side'); }; diff --git a/src/components/colorbar/draw.js b/src/components/colorbar/draw.js index 34e89aa120a..c6ccf28d21f 100644 --- a/src/components/colorbar/draw.js +++ b/src/components/colorbar/draw.js @@ -183,15 +183,7 @@ module.exports = function draw(gd, id) { tickprefix: opts.tickprefix, showticksuffix: opts.showticksuffix, ticksuffix: opts.ticksuffix, - - // Plot and axes titles have a different, nested attribute structure - // for defining title attributes. Since the `titles` component - // assumes that nested structure, let's adapt to it without breaking - // the existing colorbar API. - title: { - text: opts.title, - font: opts.titlefont - }, + title: opts.title, showline: true, anchor: 'free', side: 'right', @@ -225,11 +217,11 @@ module.exports = function draw(gd, id) { // save for other callers to access this axis component.axis = cbAxisOut; - if(['top', 'bottom'].indexOf(opts.titleside) !== -1) { - cbAxisOut.titleside = opts.titleside; + if(['top', 'bottom'].indexOf(opts.title.side) !== -1) { + cbAxisOut.title.side = opts.title.side; cbAxisOut.titlex = opts.x + xpadFrac; cbAxisOut.titley = yBottomFrac + - (opts.titleside === 'top' ? lenFrac - ypadFrac : ypadFrac); + (opts.title.side === 'top' ? lenFrac - ypadFrac : ypadFrac); } if(opts.line.color && opts.tickmode === 'auto') { @@ -292,7 +284,7 @@ module.exports = function draw(gd, id) { var axisLayer = container.select('.cbaxis'); var titleHeight = 0; - if(['top', 'bottom'].indexOf(opts.titleside) !== -1) { + if(['top', 'bottom'].indexOf(opts.title.side) !== -1) { // draw the title so we know how much room it needs // when we squish the axis. This one only applies to // top or bottom titles, not right side. @@ -300,7 +292,7 @@ module.exports = function draw(gd, id) { fontSize = cbAxisOut.title.font.size, y; - if(opts.titleside === 'top') { + if(opts.title.side === 'top') { y = (1 - (yBottomFrac + lenFrac - ypadFrac)) * gs.h + gs.t + 3 + fontSize * 0.75; } @@ -314,7 +306,7 @@ module.exports = function draw(gd, id) { } function drawAxis() { - if(['top', 'bottom'].indexOf(opts.titleside) !== -1) { + if(['top', 'bottom'].indexOf(opts.title.side) !== -1) { // squish the axis top to make room for the title var titleGroup = container.select('.cbtitle'), titleText = titleGroup.select('text'), @@ -345,7 +337,7 @@ module.exports = function draw(gd, id) { // TODO: configurable titleHeight += 5; - if(opts.titleside === 'top') { + if(opts.title.side === 'top') { cbAxisOut.domain[1] -= titleHeight / gs.h; titleTrans[1] *= -1; } @@ -466,7 +458,7 @@ module.exports = function draw(gd, id) { }); }, function() { - if(['top', 'bottom'].indexOf(opts.titleside) === -1) { + if(['top', 'bottom'].indexOf(opts.title.side) === -1) { var fontSize = cbAxisOut.title.font.size, y = cbAxisOut._offset + cbAxisOut._length / 2, x = gs.l + (cbAxisOut.position || 0) * gs.w + ((cbAxisOut.side === 'right') ? @@ -479,7 +471,7 @@ module.exports = function draw(gd, id) { drawTitle('h' + cbAxisOut._id + 'title', { avoid: { selection: d3.select(gd).selectAll('g.' + cbAxisOut._id + 'tick'), - side: opts.titleside, + side: opts.title.side, offsetLeft: gs.l, offsetTop: 0, maxShift: fullLayout.width @@ -532,11 +524,11 @@ module.exports = function draw(gd, id) { .node(), titleWidth; if(mathJaxNode && - ['top', 'bottom'].indexOf(opts.titleside) !== -1) { + ['top', 'bottom'].indexOf(opts.title.side) !== -1) { titleWidth = Drawing.bBox(mathJaxNode).width; } else { - // note: the formula below works for all titlesides, + // note: the formula below works for all title sides, // (except for top/bottom mathjax, above) // but the weird gs.l is because the titleunshift // transform gets removed by Drawing.bBox @@ -565,7 +557,7 @@ module.exports = function draw(gd, id) { container.selectAll('.cboutline').attr({ x: xLeft, y: yTopPx + opts.ypad + - (opts.titleside === 'top' ? titleHeight : 0), + (opts.title.side === 'top' ? titleHeight : 0), width: Math.max(thickPx, 2), height: Math.max(outerheight - 2 * opts.ypad - titleHeight, 2) }) diff --git a/src/plot_api/helpers.js b/src/plot_api/helpers.js index 35caf22619e..904ffc2b343 100644 --- a/src/plot_api/helpers.js +++ b/src/plot_api/helpers.js @@ -254,6 +254,13 @@ function cleanTitle(titleContainer) { if(oldPositionAttrSet && !newPositionAttrSet) { nestTitleAttr('titleposition', 'position'); } + + // titleside -> title.side + var oldSideAttrSet = titleContainer.titleside; + var newSideAttrSet = titleContainer.title && titleContainer.title.side; + if(oldSideAttrSet && !newSideAttrSet) { + nestTitleAttr('titleside', 'side'); + } } function nestTitleAttr(oldAttrName, newAttrName) { @@ -473,6 +480,9 @@ exports.cleanData = function(data) { } cleanTitle(trace); + if(trace.colorbar) cleanTitle(trace.colorbar); + if(trace.marker && trace.marker.colorbar) cleanTitle(trace.marker.colorbar); + if(trace.line && trace.line.colorbar) cleanTitle(trace.line.colorbar); } }; diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js index 5962ef081bc..65d858f164b 100644 --- a/src/plot_api/plot_api.js +++ b/src/plot_api/plot_api.js @@ -1712,7 +1712,8 @@ function _restyle(gd, aobj, traces) { * @param aobj */ function cleanDeprecatedAttributeKeys(aobj) { - var oldAxisTitleRegExp = Lib.counterRegex('axis', '\.title', false, false); + var oldAxisTitleRegex = Lib.counterRegex('axis', '\.title', false, false); + var colorbarRegex = /colorbar\.title$/; var keys = Object.keys(aobj); var i, key, value; @@ -1720,13 +1721,15 @@ function cleanDeprecatedAttributeKeys(aobj) { key = keys[i]; value = aobj[key]; - if((key === 'title' || oldAxisTitleRegExp.test(key)) && + if((key === 'title' || oldAxisTitleRegex.test(key) || colorbarRegex.test(key)) && (typeof value === 'string' || typeof value === 'number')) { replace(key, key.replace('title', 'title.text')); } else if(key.indexOf('titlefont') > -1) { replace(key, key.replace('titlefont', 'title.font')); } else if(key.indexOf('titleposition') > -1) { replace(key, key.replace('titleposition', 'title.position')); + } else if(key.indexOf('titleside') > -1) { + replace(key, key.replace('titleside', 'title.side')); } } From e4dbc5dbfa5d067d40043b0b3938d2539ca2e3a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=B6stl?= Date: Wed, 28 Nov 2018 08:41:59 +0100 Subject: [PATCH 46/49] Transition carpet to new title attr structure [882] - Also remove unnecessary code in carpet implementation. --- src/plot_api/helpers.js | 10 ++++ src/plot_api/plot_api.js | 2 + src/traces/carpet/axis_attributes.js | 75 +++++++++++++++++++++------- src/traces/carpet/axis_defaults.js | 11 ++-- src/traces/carpet/plot.js | 10 ++-- 5 files changed, 76 insertions(+), 32 deletions(-) diff --git a/src/plot_api/helpers.js b/src/plot_api/helpers.js index 904ffc2b343..a11cb9b8fc1 100644 --- a/src/plot_api/helpers.js +++ b/src/plot_api/helpers.js @@ -241,6 +241,7 @@ function cleanTitle(titleContainer) { }; } + // TODO 882 DRY UP? // titlefont -> title.font var oldFontAttrSet = Lib.isPlainObject(titleContainer.titlefont); var newFontAttrSet = titleContainer.title && Lib.isPlainObject(titleContainer.title.font); @@ -261,6 +262,13 @@ function cleanTitle(titleContainer) { if(oldSideAttrSet && !newSideAttrSet) { nestTitleAttr('titleside', 'side'); } + + // titleoffset -> title.offset + var oldOffsetAttrSet = titleContainer.titleoffset; + var newOffsetAttrSet = titleContainer.title && titleContainer.title.offset; + if(oldOffsetAttrSet && !newOffsetAttrSet) { + nestTitleAttr('titleoffset', 'offset'); + } } function nestTitleAttr(oldAttrName, newAttrName) { @@ -483,6 +491,8 @@ exports.cleanData = function(data) { if(trace.colorbar) cleanTitle(trace.colorbar); if(trace.marker && trace.marker.colorbar) cleanTitle(trace.marker.colorbar); if(trace.line && trace.line.colorbar) cleanTitle(trace.line.colorbar); + if(trace.aaxis) cleanTitle(trace.aaxis); + if(trace.baxis) cleanTitle(trace.baxis); } }; diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js index 65d858f164b..15c4c4d0b0d 100644 --- a/src/plot_api/plot_api.js +++ b/src/plot_api/plot_api.js @@ -1730,6 +1730,8 @@ function cleanDeprecatedAttributeKeys(aobj) { replace(key, key.replace('titleposition', 'title.position')); } else if(key.indexOf('titleside') > -1) { replace(key, key.replace('titleside', 'title.side')); + } else if(key.indexOf('titleoffset') > -1) { + replace(key, key.replace('titleoffset', 'title.offset')); } } diff --git a/src/traces/carpet/axis_attributes.js b/src/traces/carpet/axis_attributes.js index 42e6dcc7f13..88837cb7098 100644 --- a/src/traces/carpet/axis_attributes.js +++ b/src/traces/carpet/axis_attributes.js @@ -34,26 +34,38 @@ module.exports = { editType: 'calc' }, title: { - valType: 'string', - role: 'info', + text: { + valType: 'string', + role: 'info', + editType: 'calc', + description: [ + 'Sets the title of this axis.', + 'Note that before the existence of `title.text`, the title\'s', + 'contents used to be defined as the `title` attribute itself.', + 'This behavior has been deprecated.' + ].join(' ') + }, + font: fontAttrs({ + editType: 'calc', + description: [ + 'Sets this axis\' title font.', + 'Note that the title\'s font used to be set', + 'by the now deprecated `titlefont` attribute.' + ].join(' ') + }), + offset: { + valType: 'number', + role: 'info', + dflt: 10, + editType: 'calc', + description: [ + 'An additional amount by which to offset the title from the tick', + 'labels, given in pixels.', + 'Note that this used to be set', + 'by the now deprecated `titleoffset` attribute.' + ].join(' '), + }, editType: 'calc', - description: 'Sets the title of this axis.' - }, - titlefont: fontAttrs({ - editType: 'calc', - description: [ - 'Sets this axis\' title font.' - ].join(' ') - }), - titleoffset: { - valType: 'number', - role: 'info', - dflt: 10, - editType: 'calc', - description: [ - 'An additional amount by which to offset the title from the tick', - 'labels, given in pixels' - ].join(' '), }, type: { valType: 'enumerated', @@ -494,5 +506,30 @@ module.exports = { editType: 'calc', description: 'The stride between grid lines along the axis' }, + + _deprecated: { + title: { + valType: 'string', + role: 'info', + editType: 'calc', + description: [ + 'Deprecated in favor of `title.text`.', + 'Note that value of `title` is no longer a simple', + '*string* but a set of sub-attributes.' + ].join(' ') + }, + titlefont: fontAttrs({ + editType: 'calc', + description: 'Deprecated in favor of `title.font`.' + }), + titleoffset: { + valType: 'number', + role: 'info', + dflt: 10, + editType: 'calc', + description: 'Deprecated in favor of `title.offset`.' + } + }, + editType: 'calc' }; diff --git a/src/traces/carpet/axis_defaults.js b/src/traces/carpet/axis_defaults.js index 511428dc1ba..10e8afabfb8 100644 --- a/src/traces/carpet/axis_defaults.js +++ b/src/traces/carpet/axis_defaults.js @@ -113,14 +113,14 @@ module.exports = function handleAxisDefaults(containerIn, containerOut, options) // inherit from global font color in case that was provided. var dfltFontColor = (dfltColor === containerIn.color) ? dfltColor : font.color; - coerce('title'); - Lib.coerceFont(coerce, 'titlefont', { + coerce('title.text'); + Lib.coerceFont(coerce, 'title.font', { family: font.family, size: Math.round(font.size * 1.2), color: dfltFontColor }); - coerce('titleoffset'); + coerce('title.offset'); coerce('tickangle'); @@ -203,11 +203,6 @@ module.exports = function handleAxisDefaults(containerIn, containerOut, options) // but no, we *actually* want to coerce this. coerce('tickmode'); - if(!containerOut.title || (containerOut.title && containerOut.title.length === 0)) { - delete containerOut.titlefont; - delete containerOut.titleoffset; - } - return containerOut; }; diff --git a/src/traces/carpet/plot.js b/src/traces/carpet/plot.js index c7ad270a55c..2d978d31e0f 100644 --- a/src/traces/carpet/plot.js +++ b/src/traces/carpet/plot.js @@ -197,7 +197,7 @@ var midShift = ((1 - alignmentConstants.MID_SHIFT) / lineSpacing) + 1; function drawAxisTitle(gd, layer, trace, t, xy, dxy, axis, xa, ya, labelOrientation, labelClass) { var data = []; - if(axis.title) data.push(axis.title); + if(axis.title.text) data.push(axis.title.text); var titleJoin = layer.selectAll('text.' + labelClass).data(data); var offset = labelOrientation.maxExtent; @@ -213,8 +213,8 @@ function drawAxisTitle(gd, layer, trace, t, xy, dxy, axis, xa, ya, labelOrientat } // In addition to the size of the labels, add on some extra padding: - var titleSize = axis.titlefont.size; - offset += titleSize + axis.titleoffset; + var titleSize = axis.title.font.size; + offset += titleSize + axis.title.offset; var labelNorm = labelOrientation.angle + (labelOrientation.flip < 0 ? 180 : 0); var angleDiff = (labelNorm - orientation.angle + 450) % 360; @@ -222,7 +222,7 @@ function drawAxisTitle(gd, layer, trace, t, xy, dxy, axis, xa, ya, labelOrientat var el = d3.select(this); - el.text(axis.title || '') + el.text(axis.title.text) .call(svgTextUtils.convertToTspans, gd); if(reverseTitle) { @@ -236,7 +236,7 @@ function drawAxisTitle(gd, layer, trace, t, xy, dxy, axis, xa, ya, labelOrientat ) .classed('user-select-none', true) .attr('text-anchor', 'middle') - .call(Drawing.font, axis.titlefont); + .call(Drawing.font, axis.title.font); }); titleJoin.exit().remove(); From ef9e078c1e6c779456c9f75dca611f754815b09e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=B6stl?= Date: Wed, 28 Nov 2018 09:00:55 +0100 Subject: [PATCH 47/49] Refactor code to clean up deprecated title attr structure [882] --- src/plot_api/helpers.js | 54 +++++++++++++++-------------------------- 1 file changed, 20 insertions(+), 34 deletions(-) diff --git a/src/plot_api/helpers.js b/src/plot_api/helpers.js index a11cb9b8fc1..e2c244cbfd7 100644 --- a/src/plot_api/helpers.js +++ b/src/plot_api/helpers.js @@ -229,6 +229,11 @@ function cleanAxRef(container, attr) { } } +/** + * Cleans up old title attribute structure (flat) in favor of the new one (nested). + * + * @param {Object} titleContainer - an object potentially including deprecated title attributes + */ function cleanTitle(titleContainer) { if(titleContainer) { @@ -241,45 +246,26 @@ function cleanTitle(titleContainer) { }; } - // TODO 882 DRY UP? - // titlefont -> title.font - var oldFontAttrSet = Lib.isPlainObject(titleContainer.titlefont); - var newFontAttrSet = titleContainer.title && Lib.isPlainObject(titleContainer.title.font); - if(oldFontAttrSet && !newFontAttrSet) { - nestTitleAttr('titlefont', 'font'); - } - - // titleposition -> title.position - var oldPositionAttrSet = titleContainer.titleposition; - var newPositionAttrSet = titleContainer.title && titleContainer.title.position; - if(oldPositionAttrSet && !newPositionAttrSet) { - nestTitleAttr('titleposition', 'position'); - } + rewireAttr('titlefont', 'font'); + rewireAttr('titleposition', 'position'); + rewireAttr('titleside', 'side'); + rewireAttr('titleoffset', 'offset'); + } - // titleside -> title.side - var oldSideAttrSet = titleContainer.titleside; - var newSideAttrSet = titleContainer.title && titleContainer.title.side; - if(oldSideAttrSet && !newSideAttrSet) { - nestTitleAttr('titleside', 'side'); - } + function rewireAttr(oldAttrName, newAttrName) { + var oldAttrSet = titleContainer[oldAttrName]; + var newAttrSet = titleContainer.title && titleContainer.title[newAttrName]; - // titleoffset -> title.offset - var oldOffsetAttrSet = titleContainer.titleoffset; - var newOffsetAttrSet = titleContainer.title && titleContainer.title.offset; - if(oldOffsetAttrSet && !newOffsetAttrSet) { - nestTitleAttr('titleoffset', 'offset'); - } - } + if(oldAttrSet && !newAttrSet) { - function nestTitleAttr(oldAttrName, newAttrName) { + // Ensure title object exists + if(!titleContainer.title) { + titleContainer.title = {}; + } - // Ensure title object exists - if(!titleContainer.title) { - titleContainer.title = {}; + titleContainer.title[newAttrName] = titleContainer[oldAttrName]; + delete titleContainer[oldAttrName]; } - - titleContainer.title[newAttrName] = titleContainer[oldAttrName]; - delete titleContainer[oldAttrName]; } } From e3bcf7c45f6795bb11dd10a2810905a75ac40618 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=B6stl?= Date: Wed, 28 Nov 2018 10:44:45 +0100 Subject: [PATCH 48/49] Let '' be the default carpet axes title [882] - Reason: having no default at all for `title.text` would lead to a non-existing `title` container (through a call to `relinkPrivateKeys` deep down the stack) eventually and as consequence would require to safeguard access to `[ab]axis.title.text` by checking `[abaxis.title]` is defined before. --- src/traces/carpet/axis_attributes.js | 1 + src/traces/carpet/axis_defaults.js | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/src/traces/carpet/axis_attributes.js b/src/traces/carpet/axis_attributes.js index 88837cb7098..1bb49be9f42 100644 --- a/src/traces/carpet/axis_attributes.js +++ b/src/traces/carpet/axis_attributes.js @@ -36,6 +36,7 @@ module.exports = { title: { text: { valType: 'string', + dflt: '', role: 'info', editType: 'calc', description: [ diff --git a/src/traces/carpet/axis_defaults.js b/src/traces/carpet/axis_defaults.js index 10e8afabfb8..a096e7efecb 100644 --- a/src/traces/carpet/axis_defaults.js +++ b/src/traces/carpet/axis_defaults.js @@ -203,6 +203,11 @@ module.exports = function handleAxisDefaults(containerIn, containerOut, options) // but no, we *actually* want to coerce this. coerce('tickmode'); + if(!containerOut.title.text) { + delete containerOut.title.font; + delete containerOut.title.offset; + } + return containerOut; }; From 2b267466d4164a02a8948c1914315dbcefc58ac9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=B6stl?= Date: Thu, 29 Nov 2018 17:14:40 +0100 Subject: [PATCH 49/49] Reverse coerce logic for carpet axes titles [882] --- src/traces/carpet/axis_defaults.js | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/src/traces/carpet/axis_defaults.js b/src/traces/carpet/axis_defaults.js index a096e7efecb..59cd9c19948 100644 --- a/src/traces/carpet/axis_defaults.js +++ b/src/traces/carpet/axis_defaults.js @@ -113,14 +113,15 @@ module.exports = function handleAxisDefaults(containerIn, containerOut, options) // inherit from global font color in case that was provided. var dfltFontColor = (dfltColor === containerIn.color) ? dfltColor : font.color; - coerce('title.text'); - Lib.coerceFont(coerce, 'title.font', { - family: font.family, - size: Math.round(font.size * 1.2), - color: dfltFontColor - }); - - coerce('title.offset'); + var title = coerce('title.text'); + if(title) { + Lib.coerceFont(coerce, 'title.font', { + family: font.family, + size: Math.round(font.size * 1.2), + color: dfltFontColor + }); + coerce('title.offset'); + } coerce('tickangle'); @@ -203,11 +204,6 @@ module.exports = function handleAxisDefaults(containerIn, containerOut, options) // but no, we *actually* want to coerce this. coerce('tickmode'); - if(!containerOut.title.text) { - delete containerOut.title.font; - delete containerOut.title.offset; - } - return containerOut; };