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

Implementation of textinfo for waterfall traces #3790

Merged
merged 65 commits into from
May 8, 2019

Conversation

archmoj
Copy link
Contributor

@archmoj archmoj commented Apr 18, 2019

Addressing #3777 for waterfall.
A similar code could be useful for bar-like funnel traces.
codepen.

@plotly/plotly_js
cc: @nicolaskruchten

@archmoj archmoj added the feature something new label Apr 18, 2019
@etpinard etpinard added this to the v1.48.0 milestone Apr 18, 2019
if(tx) thisText.push(tx);
}

return thisText.join('<br>'); // TODO: ' | ' may be a better dividor for bar
Copy link
Contributor

Choose a reason for hiding this comment

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

Yep, it might be better to have .join(' | ') for horizontal waterfalls. But since I think most users will set only one textinfo flag, I doubt it's worth adding magic here.

// labels (legend is handled by plots.attributes.showlegend and layout.hiddenlabels)
textinfo: extendFlat({}, pieAtts.textinfo, {
editType: 'plot',
flags: ['text', 'delta', 'initial', 'final'],
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 these flags are 👌

We could maybe add label, but it might awkward as for waterfall it doesn't match a labels data array like for pie and sunburst traces. Maybe adding plain 'x' and 'y' flags would be best.

@archmoj archmoj mentioned this pull request Apr 24, 2019
var final = cdi.v;
var initial = final - delta;

if(hasFlag('initial')) text.push(pieHelpers.formatPieValue(initial, separators));
Copy link
Contributor

Choose a reason for hiding this comment

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

@etpinard etpinard mentioned this pull request May 7, 2019
@archmoj
Copy link
Contributor Author

archmoj commented May 8, 2019

Should we skip a line if text array is not provided or any item in the array is null?
See codepen.
Also please note that for waterfalls there is the final flag instead of value.

@etpinard
Copy link
Contributor

etpinard commented May 8, 2019

Should we skip a line if text array is not provided or any item in the array is null?

What does pie do in this case?

@archmoj
Copy link
Contributor Author

archmoj commented May 8, 2019

What does pie do in this case?

Good question. No new lines when there is empty text.

@etpinard
Copy link
Contributor

etpinard commented May 8, 2019

💃 nice job!

@archmoj archmoj merged commit 09b4972 into master May 8, 2019
@archmoj archmoj deleted the fix3777-textinfo-waterfall-fin branch May 8, 2019 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature something new
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants