Skip to content

Commit

Permalink
Merge pull request #4307 from plotly/image-scaleanchor
Browse files Browse the repository at this point in the history
image: improve layout defaults
  • Loading branch information
antoinerg authored Oct 29, 2019
2 parents 518fc61 + 408b46b commit fbfadbe
Show file tree
Hide file tree
Showing 16 changed files with 133 additions and 22 deletions.
15 changes: 11 additions & 4 deletions src/plots/cartesian/constraints.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,11 @@ var concatExtremes = require('./autorange').concatExtremes;
var ALMOST_EQUAL = require('../../constants/numerical').ALMOST_EQUAL;
var FROM_BL = require('../../constants/alignment').FROM_BL;

exports.handleConstraintDefaults = function(containerIn, containerOut, coerce, allAxisIds, layoutOut) {
exports.handleConstraintDefaults = function(containerIn, containerOut, coerce, opts) {
var allAxisIds = opts.allAxisIds;
var layoutOut = opts.layoutOut;
var scaleanchorDflt = opts.scaleanchorDflt;
var constrainDflt = opts.constrainDflt;
var constraintGroups = layoutOut._axisConstraintGroups;
var matchGroups = layoutOut._axisMatchGroups;
var axId = containerOut._id;
Expand All @@ -28,7 +32,7 @@ exports.handleConstraintDefaults = function(containerIn, containerOut, coerce, a

// coerce the constraint mechanics even if this axis has no scaleanchor
// because it may be the anchor of another axis.
var constrain = coerce('constrain');
var constrain = coerce('constrain', constrainDflt);
Lib.coerce(containerIn, containerOut, {
constraintoward: {
valType: 'enumerated',
Expand All @@ -53,14 +57,17 @@ exports.handleConstraintDefaults = function(containerIn, containerOut, coerce, a
// 'matches' wins over 'scaleanchor' (for now)
var scaleanchor, scaleOpts;

if(!matches && containerIn.scaleanchor && !(containerOut.fixedrange && constrain !== 'domain')) {
if(!matches &&
!(containerOut.fixedrange && constrain !== 'domain') &&
(containerIn.scaleanchor || scaleanchorDflt)
) {
scaleOpts = getConstraintOpts(constraintGroups, thisID, allAxisIds, layoutOut, constrain);
scaleanchor = Lib.coerce(containerIn, containerOut, {
scaleanchor: {
valType: 'enumerated',
values: scaleOpts.linkableAxes || []
}
}, 'scaleanchor');
}, 'scaleanchor', scaleanchorDflt);
}

if(matches) {
Expand Down
40 changes: 29 additions & 11 deletions src/plots/cartesian/layout_defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,9 @@ module.exports = function supplyLayoutDefaults(layoutIn, layoutOut, fullData) {
var yaMayHide = {};
var xaMustDisplay = {};
var yaMustDisplay = {};
var yaMustForward = {};
var yaMayBackward = {};
var yaMustNotReverse = {};
var yaMayReverse = {};
var axHasImage = {};
var outerTicks = {};
var noGrids = {};
var i, j;
Expand Down Expand Up @@ -74,24 +75,22 @@ module.exports = function supplyLayoutDefaults(layoutIn, layoutOut, fullData) {
if(trace.type === 'funnel') {
if(trace.orientation === 'h') {
if(xaName) xaMayHide[xaName] = true;
if(yaName) yaMayBackward[yaName] = true;
if(yaName) yaMayReverse[yaName] = true;
} else {
if(yaName) yaMayHide[yaName] = true;
}
} else if(trace.type === 'image') {
if(yaName) axHasImage[yaName] = true;
if(xaName) axHasImage[xaName] = true;
} else {
if(yaName) {
yaMustDisplay[yaName] = true;
yaMustForward[yaName] = true;
yaMustNotReverse[yaName] = true;
}

if(!traceIs(trace, 'carpet') || (trace.type === 'carpet' && !trace._cheater)) {
if(xaName) xaMustDisplay[xaName] = true;
}

if(trace.type === 'image') {
if(yaName) yaMustForward[yaName] = false;
if(yaName) yaMayBackward[yaName] = true;
}
}

// Two things trigger axis visibility:
Expand Down Expand Up @@ -197,7 +196,11 @@ module.exports = function supplyLayoutDefaults(layoutIn, layoutOut, fullData) {
(axLetter === 'y' && !yaMustDisplay[axName] && yaMayHide[axName]);

var reverseDflt =
(axLetter === 'y' && !yaMustForward[axName] && yaMayBackward[axName]);
(axLetter === 'y' &&
(
(!yaMustNotReverse[axName] && yaMayReverse[axName]) ||
axHasImage[axName]
));

var defaultOptions = {
letter: axLetter,
Expand Down Expand Up @@ -299,7 +302,22 @@ module.exports = function supplyLayoutDefaults(layoutIn, layoutOut, fullData) {
axLayoutIn = layoutIn[axName];
axLayoutOut = layoutOut[axName];

handleConstraintDefaults(axLayoutIn, axLayoutOut, coerce, allAxisIds, layoutOut);
var scaleanchorDflt;
if(axLetter === 'y' && !axLayoutIn.hasOwnProperty('scaleanchor') && axHasImage[axName]) {
scaleanchorDflt = axLayoutOut.anchor;
} else {scaleanchorDflt = undefined;}

var constrainDflt;
if(!axLayoutIn.hasOwnProperty('constrain') && axHasImage[axName]) {
constrainDflt = 'domain';
} else {constrainDflt = undefined;}

handleConstraintDefaults(axLayoutIn, axLayoutOut, coerce, {
allAxisIds: allAxisIds,
layoutOut: layoutOut,
scaleanchorDflt: scaleanchorDflt,
constrainDflt: constrainDflt
});
}

for(i = 0; i < matchGroups.length; i++) {
Expand Down
7 changes: 6 additions & 1 deletion src/traces/image/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,12 @@ module.exports = {
animatable: false,
meta: {
description: [
'Display an image, i.e. data on a 2D regular raster.'
'Display an image, i.e. data on a 2D regular raster.',
'By default, when an image is displayed in a subplot,',
'its y axis will be reversed (ie. `autorange: \'reversed\'`),',
'constrained to the domain (ie. `constrain: \'domain\'`)',
'and it will have the same scale as its x axis (ie. `scaleanchor: \'x\,`)',
'in order for pixels to be rendered as squares.'
].join(' ')
}
};
Binary file modified test/image/baselines/image_axis_reverse.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/image/baselines/image_axis_type.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/image/baselines/image_cat.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/image/baselines/image_colormodel.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/image/baselines/image_opacity.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/image/baselines/image_with_gaps.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/image/baselines/image_with_heatmap.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/image/baselines/image_zmin_zmax.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
3 changes: 2 additions & 1 deletion test/image/mocks/image_adventurer.json

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion test/image/mocks/image_cat.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"layout": {
"width": 400, "height": 400, "yaxis": {"scaleanchor": "x"}, "margin": {"t":25, "b": 25, "r": 25, "l": 40}
"width": 400, "height": 400, "yaxis": {"scaleanchor": "x", "autorange": "reversed"}, "margin": {"t":25, "b": 25, "r": 25, "l": 40}
},
"data": [
{"x":[50, 150, 350, 50], "y":[350, 23, 100, 350]},
Expand Down
6 changes: 4 additions & 2 deletions test/image/mocks/image_colormodel.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,14 @@
"data": [{
"type": "image",
"colormodel": "rgb",
"dy": 10,
"hoverinfo": "all",
"dy": 1,
"dx": 0.5,
"z": [[[255, 0, 0], [0, 255, 0], [0, 0, 255]]]
}, {
"type": "image",
"colormodel": "hsl",
"hoverinfo": "all",
"z": [
[[0, 33, 50], [0, 66, 50], [0, 100, 50]],
[[90, 33, 50], [90, 66, 50], [90, 100, 50]],
Expand All @@ -18,7 +20,7 @@
"yaxis": "y2"
}],
"layout": {
"grid": {"rows": 2, "columns": 1, "pattern": "independent"},
"grid": {"rows": 1, "columns": 2, "pattern": "independent"},
"width": 600,
"height": 400
}
Expand Down
1 change: 1 addition & 0 deletions test/jasmine/assets/mock_lists.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ var svgMockList = [
['finance_style', require('@mocks/finance_style.json')],
['geo_first', require('@mocks/geo_first.json')],
['indicator_bignumber', require('@mocks/indicator_bignumber.json')],
['image_adventurer', require('@mocks/image_adventurer.json')],
['layout_image', require('@mocks/layout_image.json')],
['layout-colorway', require('@mocks/layout-colorway.json')],
['multicategory', require('@mocks/multicategory.json')],
Expand Down
81 changes: 79 additions & 2 deletions test/jasmine/tests/image_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ var failTest = require('../assets/fail_test');

var customAssertions = require('../assets/custom_assertions');
var assertHoverLabelContent = customAssertions.assertHoverLabelContent;
var supplyAllDefaults = require('../assets/supply_defaults');
var Fx = require('@src/components/fx');

describe('image supplyDefaults', function() {
Expand Down Expand Up @@ -100,6 +101,82 @@ describe('image supplyDefaults', function() {
});
});

describe('image smart layout defaults', function() {
var gd;
beforeEach(function() {
gd = createGraphDiv();
});

afterEach(destroyGraphDiv);

it('should reverse yaxis if images are present', function() {
gd = {};
gd.data = [{type: 'image', z: [[[255, 0, 0]]]}];
supplyAllDefaults(gd);
expect(gd._fullLayout.yaxis.autorange).toBe('reversed');
});

it('should reverse yaxis even if another trace is present', function() {
gd = {};
gd.data = [{type: 'image', z: [[[255, 0, 0]]]}, {type: 'scatter', y: [5, 3, 2]}];
supplyAllDefaults(gd);
expect(gd._fullLayout.yaxis.autorange).toBe('reversed');
});

it('should NOT reverse yaxis if it\'s already defined', function() {
gd = {};
gd.data = [{type: 'image', z: [[[255, 0, 0]]]}];
gd.layout = {yaxis: {autorange: false}};
supplyAllDefaults(gd);
expect(gd._fullLayout.yaxis.autorange).toBe(false);
});

it('should set scaleanchor to make square pixels if images are present', function() {
gd = {};
gd.data = [{type: 'image', z: [[[255, 0, 0]]]}];
supplyAllDefaults(gd);
expect(gd._fullLayout.yaxis.scaleanchor).toBe('x');
});

it('should set scaleanchor even if another trace is present', function() {
gd = {};
gd.data = [{type: 'image', z: [[[255, 0, 0]]]}, {type: 'scatter', y: [5, 3, 2]}];
supplyAllDefaults(gd);
expect(gd._fullLayout.yaxis.scaleanchor).toBe('x');
});

it('should NOT set scaleanchor if it\'s already defined', function() {
gd.data = [{type: 'image', z: [[[255, 0, 0]]]}];
gd.layout = {yaxis: {scaleanchor: 'x3'}};
supplyAllDefaults(gd);
expect(gd._fullLayout.yaxis.scaleanchor).toBe(undefined);
});

it('should constrain axes to domain if images are present', function() {
gd = {};
gd.data = [{type: 'image', z: [[[255, 0, 0]]]}];
supplyAllDefaults(gd);
expect(gd._fullLayout.xaxis.constrain).toBe('domain');
expect(gd._fullLayout.yaxis.constrain).toBe('domain');
});

it('should constrain axes to domain even if another trace is present', function() {
gd = {};
gd.data = [{type: 'image', z: [[[255, 0, 0]]]}, {type: 'scatter', y: [5, 3, 2]}];
supplyAllDefaults(gd);
expect(gd._fullLayout.xaxis.constrain).toBe('domain');
expect(gd._fullLayout.yaxis.constrain).toBe('domain');
});

it('should NOT constrain axes to domain if it\'s already defined', function() {
gd.data = [{type: 'image', z: [[[255, 0, 0]]]}];
gd.layout = {yaxis: {constrain: false}, xaxis: {constrain: false}};
supplyAllDefaults(gd);
expect(gd._fullLayout.xaxis.constrain).toBe('range');
expect(gd._fullLayout.yaxis.constrain).toBe('range');
});
});

describe('image plot', function() {
'use strict';

Expand Down Expand Up @@ -444,10 +521,10 @@ describe('image hover:', function() {
zmax: [1, 1, 1],
text: [['A', 'B', 'C'], ['D', 'E', 'F']],
hovertemplate: '%{text}<extra></extra>'
}], layout: {width: 400, height: 400}};
}], layout: {width: 400, height: 400, yaxis: {constrain: 'range'}}};

Plotly.newPlot(gd, mockCopy)
.then(function() {_hover(140, 200);})
.then(function() {_hover(140, 180);})
.then(function() {
assertHoverLabelContent({
nums: 'E',
Expand Down

0 comments on commit fbfadbe

Please sign in to comment.