Skip to content

indicator: position delta using drawing.bBox#4007

Merged
antoinerg merged 1 commit into
masterfrom
indicator-fix-delta-pos
Jul 2, 2019
Merged

indicator: position delta using drawing.bBox#4007
antoinerg merged 1 commit into
masterfrom
indicator-fix-delta-pos

Conversation

@antoinerg

Copy link
Copy Markdown
Contributor

Because guessing the size of the text to vertically align it wasn't robust enough.

I introduced a new mock indicator_datacard3 to showcase the 12 possible combinations of delta.position and align. Changing the font size of either number or delta now always yield a correct alignment.

Codepen: https://codepen.io/antoinerg/pen/qzYpeV

@antoinerg antoinerg added this to the v1.49.0 milestone Jul 2, 2019
Comment thread src/traces/indicator/plot.js
Comment thread src/snapshot/tosvg.js
@etpinard etpinard added status: reviewable bug something broken labels Jul 2, 2019
@@ -0,0 +1,140 @@
{
"data": [{

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice mock!

For reference, on master this mock looks like

image

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It looks fine on master but if you change the font-size of delta or number, the alignment might break.

@etpinard

etpinard commented Jul 2, 2019

Copy link
Copy Markdown
Contributor

Looks good @antoinerg - thanks for the quick turnaround

💃

@antoinerg
antoinerg merged commit b6edba5 into master Jul 2, 2019
@antoinerg
antoinerg deleted the indicator-fix-delta-pos branch July 2, 2019 19:41
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.

2 participants