Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

trace type indicator #3978

Merged
merged 15 commits into from
Jun 28, 2019
Merged

trace type indicator #3978

merged 15 commits into from
Jun 28, 2019

Conversation

antoinerg
Copy link
Contributor

@antoinerg antoinerg commented Jun 21, 2019

This PR introduces a new trace type indicator and closes #3659

Codepen to animate mocks: https://codepen.io/antoinerg/full/dBZdpY (using bundle built off 1a991d5)

  • Improve test coverage of mocks (gauge.axis.visible)
  • Provide codepen to animate aforementionned mocks

@etpinard etpinard added this to the v1.49.0 milestone Jun 21, 2019
number: {
valueformat: {
valType: 'string',
dflt: '.3s',
Copy link
Contributor

Choose a reason for hiding this comment

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

It might be nice to be consistent with other *format attributes in use currently e.g. tickformat for axis where the dflt is left blank and we let Axes.tickText handle things.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't have a strong opinion for this one but having a fixed number of digits with SI suffixes as a default does make sense if we want the number to always fit within the angular gauge regardless of how big or small the value is 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

for this one but having a fixed number of digits with SI suffixes as a default does make sense if we want the number to always fit within the angular gauge regardless of how big or small the value is

Good point here. I don't have a strong opinion either.

'Sets the position of delta with respect to the number.'
].join(' ')
},
relative: {
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to document something me and @antoinerg discussed in-person last week.


One potential addition to indicator would be to add a way to show both "absolute" and "relative" delta widgets on the trace same indicator trace.

One way to achieve this would be to add 'delta2' flag in mode and have something like:

{
  mode: 'number+delta+delta2'
  delta: {},
  delta2: {relative: true}
}

OR, replace delta with deltarel and deltaabs

{
  mode: 'number+deltarel+deltaabs'
  deltarel: {},
  deltaabs: {}
}

making this relative attribute obsolete.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's definitely a possibility!

I think I prefer having delta2 as it would imply it is to be displayed to the right of delta. If we have deltarel and deltaabs, we'd have to add an attribute to set their position relative to one another. Basically, we'd be trading attribute relative for relativeposition...

},
align: {
valType: 'enumerated',
values: ['left', 'center', 'right'],
Copy link
Contributor

Choose a reason for hiding this comment

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

I think title.align corresponds to what most users will find useful for indicator traces, but note that align somewhat conflicts with textposition in pie traces

position: {
valType: 'enumerated',
values: [
'top left', 'top center', 'top right',
'middle center',
'bottom left', 'bottom center', 'bottom right'
],
role: 'info',
editType: 'plot',
description: [
'Specifies the location of the `title`.',
'Note that the title\'s position used to be set',
'by the now deprecated `titleposition` attribute.'
].join(' ')
},

and the (x|y)ref / (x|y)anchor attributes for layout.title:

xref: {
valType: 'enumerated',
dflt: 'container',
values: ['container', 'paper'],
role: 'info',
editType: 'layoutstyle',
description: [
'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: {
valType: 'enumerated',
dflt: 'container',
values: ['container', 'paper'],
role: 'info',
editType: 'layoutstyle',
description: [
'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: {
valType: 'number',
min: 0,
max: 1,
dflt: 0.5,
role: 'style',
editType: 'layoutstyle',
description: [
'Sets the x position with respect to `xref` in normalized',
'coordinates from *0* (left) to *1* (right).'
].join(' ')
},
y: {
valType: 'number',
min: 0,
max: 1,
dflt: 'auto',
role: 'style',
editType: 'layoutstyle',
description: [
'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: {
valType: 'enumerated',
dflt: 'auto',
values: ['auto', 'left', 'center', 'right'],
role: 'info',
editType: 'layoutstyle',
description: [
'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: {
valType: 'enumerated',
dflt: 'auto',
values: ['auto', 'top', 'middle', 'bottom'],
role: 'info',
editType: 'layoutstyle',
description: [
'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(' ')
},

and there's side in colorbars:

xref: {
valType: 'enumerated',
dflt: 'container',
values: ['container', 'paper'],
role: 'info',
editType: 'layoutstyle',
description: [
'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: {
valType: 'enumerated',
dflt: 'container',
values: ['container', 'paper'],
role: 'info',
editType: 'layoutstyle',
description: [
'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: {
valType: 'number',
min: 0,
max: 1,
dflt: 0.5,
role: 'style',
editType: 'layoutstyle',
description: [
'Sets the x position with respect to `xref` in normalized',
'coordinates from *0* (left) to *1* (right).'
].join(' ')
},
y: {
valType: 'number',
min: 0,
max: 1,
dflt: 'auto',
role: 'style',
editType: 'layoutstyle',
description: [
'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: {
valType: 'enumerated',
dflt: 'auto',
values: ['auto', 'left', 'center', 'right'],
role: 'info',
editType: 'layoutstyle',
description: [
'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: {
valType: 'enumerated',
dflt: 'auto',
values: ['auto', 'top', 'middle', 'bottom'],
role: 'info',
editType: 'layoutstyle',
description: [
'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(' ')
},

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm willing to either rename the attribute to position as in pie or change the API altogether.

FYI, the (title|number).align attributes were added at the last minute in order to mimic DDK's "data cards" in which the text is left-aligned and position to the left of the domain.

coerceGauge('shape');
var isBullet = traceOut._isBullet = traceOut.gauge.shape === 'bullet';
if(!isBullet) {
coerce('title.align', 'center');
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't the current behaviour for bullets have essentially title.align: 'right'?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Doesn't the current behaviour for bullets have essentially title.align: 'right'?

They do yes. However, it cannot be changed. That's why I thought it shouldn't be coerced. Should I coerce it?

Copy link
Contributor

Choose a reason for hiding this comment

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

Could it support align: 'center' and align: 'left' eventually (i.e. not in this PR)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could it support align: 'center' and align: 'left' eventually (i.e. not in this PR)?

My thought was that because the title is outside the domain in bullet, the notion of alignment is not well-defined: it's unclear how much whitespace we have on the left (ie. maybe there's another plot there).

@etpinard
Copy link
Contributor

etpinard commented Jun 25, 2019

Awesome work @antoinerg !

I'm a big fan of your numbersScales abstractions and your clever use of Drawing.bBox to place the indicator title.

Your baselines looks great and they appear to 🔒 down a lot of the behaviour. Maybe you could add one more baseline to lock down indicator together with template and layout.grid ?

@etpinard
Copy link
Contributor

... oh and @antoinerg maybe you could share a codepen example of indicator used Plotly.react transition or Plotly.animate?

@etpinard
Copy link
Contributor

Oh one more thing,

Peek 2019-06-25 16-29

the "Toggle show closest data on hover" modebar button shouldn't not appear when only indicator traces are present on the graph.

@antoinerg
Copy link
Contributor Author

antoinerg commented Jun 26, 2019

the "Toggle show closest data on hover" modebar button shouldn't not appear when only indicator traces are present on the graph.

Traces that are part of category noHover do not have hover buttons following commits c796e57 + aa6ba8f
Screenshot_2019-06-26_17-30-37

@antoinerg
Copy link
Contributor Author

antoinerg commented Jun 27, 2019

The default for gauge.bgcolor is black. We should probably change it otherwise we get a not so pretty:

Plotly.newPlot(gd,[{type:'indicator', mode:'gauge', value:10}])

newplot - 2019-06-27T132131 367

Any suggestion?

@etpinard
Copy link
Contributor

etpinard commented Jun 27, 2019

Any suggestion?

Oh right. Good eye! We should be using coerce('bgcolor', layoutOut.paper_bgcolor);

@antoinerg
Copy link
Contributor Author

... oh and @antoinerg maybe you could share a codepen example of indicator used Plotly.react transition or Plotly.animate?

Here's the requested Codepen: https://codepen.io/antoinerg/full/dBZdpY

moduleType: 'trace',
name: 'indicator',
basePlotModule: require('./base_plot'),
categories: ['svg', 'noOpacity', 'noHover'],
Copy link
Contributor

Choose a reason for hiding this comment

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

It could be useful to if this trace had an adjustable opacity.

Copy link
Contributor

Choose a reason for hiding this comment

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

Mojtaba "Mr. Opacity" Samimi

@etpinard
Copy link
Contributor

All right. Time to merge this thing!

https://codepen.io/antoinerg/full/dBZdpY is one of the coolest plotly.js codepen in recent memory 🥇

Not bad for @antoinerg 's first new trace type.

💃 💃 💃


@antoinerg feel free to open a new issue about the array-container / coerce2 + template bugs you noticed. They should be tackled in (a) separate PR(s).

@antoinerg antoinerg merged commit c0cebfd into master Jun 28, 2019
@antoinerg antoinerg deleted the indicator-pr branch June 28, 2019 20:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Indicator trace type (gauge+bignum+bullet)
3 participants