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

Text template formatting fixes #4380

Merged
merged 10 commits into from
Nov 22, 2019
Merged

Text template formatting fixes #4380

merged 10 commits into from
Nov 22, 2019

Conversation

etpinard
Copy link
Contributor

fixes #4263 - a follow-up of #4071


Expanding on the info from #4263 (comment)

There are four different texttemplate "pathways" used currently:

  • bar, funnel and waterfall traces format labels and call Lib.texttemplateString inside bar/plot.js
  • pie/funnelarea and sunburst/treemap format labels and call Lib.texttemplateString from their formatSliceLabel routine
  • scatter, scattercarpet, scatterpolar, scatterternary and scattergeo call Drawing.textPointStyle via Scatter.styleText
  • scatter3d, scattergl, scatterpolargl and scattermapbox call Lib.texttemplateString in their "convert" step

Fixing this bug here for bar, funnel and waterfall traces is relatively easy. This is done in 7717428. Commit 89422ba fixes a similar bug affecting hovertemplate.


Pie, funnelarea, sunburst and treemap are unaffected by this bug. Their texttemplate formatting appears equivalent to their hovertemplate formatting.


All the scatter* traces are currently affected.

To fix the scatter case, we need to pass xLabel and yLabel to Lib.texttemplateString as formatted by Axes.tickText similarly to how we fill in xLabel and yLabel during hover. To fix the scatterternary case, we'll need to pass aLabel, bLabel and cLabel.
For the scattergeo, lonLabel and latLabel and so on ...

To do so, I added a new trace module method called formatLabels which is meant to be called during hover and when building up the available labels for texttemplate. This is done in 76d8a11 and the new Drawing.textPointStyle is implemented in 04503ea along with new tests.

before: https://codepen.io/etpinard/pen/MWWRQqQ
after: https://codepen.io/etpinard/pen/qBBwxMv


Fixing scattergl, scatterpolargl, scattermapbox and scattergl was a bit more tedious. Please refer to the commit messages in 493ee75, acbd0de and 2278910 for the details.


@plotly/plotly_js I'd like to release this in next week's v1.51.2.

Maybe texttemplate author @antoinerg could review this one?

- note also the fix texttemplate test case
... with appropriate edit in legend trace clone
...  in an effort to bring texttemplate and hover coordinates
     formatting to the same place.
... by calling `_module.formatLabels` during Drawing.textPointStyle.

    This way, prefixes and suffixes render correctly in texttemplate
    text and minus signs now show up correctly.
... using a new formatLabels method. Notice that now the minus
    sign render correctly.
... by adding a formatLabels method. The result is a bit hackier
    than for the SVG trace types as we have to juggle with calcdata
    mocks. Note also that text templates are formatting during the
    calc step for scattergl and scatterpolargl which may cause
    some issues down the road. I didn't notice anything in this
    iteration, but we should probably move the text templates
    formatting down to the plot step down the road.
... w/o using a formatLabels method here, as gl3d hover
    does not use `_module.hoverPoints` and the formatting logic
    for scatter3d is fairly trivial, so this here should suffice
    until we refactor the gl3d hover pipeline.
@etpinard etpinard added bug something broken status: reviewable labels Nov 22, 2019
@antoinerg
Copy link
Contributor

Thanks @etpinard for making texttemplate and hovertemplate equivalent. I thought fixing #4263 would require more changes so this is great 👌.

💃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something broken
Projects
None yet
Development

Successfully merging this pull request may close these issues.

texttemplate vs hovertemplate
2 participants