Skip to content

Conversation

archmoj
Copy link
Contributor

@archmoj archmoj commented Oct 4, 2022

@plotly/plotly_js

@archmoj archmoj added bug something broken status: reviewable labels Oct 4, 2022
strScale(scale) +
strTranslate(transform.tx, transform.ty));
} else {
titleText.style('display', 'none');
Copy link
Collaborator

Choose a reason for hiding this comment

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

OIC, you prevent even setting the transform attribute when we're going to hide the text. That seems reasonable. After a little investigation it seems we DON'T delete empty slice paths, so what we're seeing from Word must be an extra automatic optimization it does to remove elements with no visible impact. I wish it extended that optimization to scaled-down text 😜

What happens if we later edit this pie, giving this slice nonzero size? Do we reuse the element - and if so do we need to clear the display: none in the if(scale) block?

strTranslate(transform.tx, transform.ty));
var scale = Math.min(1, transform.scale);

if(scale) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wish we could reproduce this ourselves, but some of the data we have from the Word tests suggests that there's some minimum scale that won't break things. There's certainly some size that succeeds but is already useless for practical purposes so could be deleted, somewhere between 0.1 and 0.01? Ideally we'd make this configurable but maybe we can hardcode something in that range to start?

@archmoj archmoj changed the title Hide pie text on empty slices to avoid PDF exports in MS Word from SVG imports Hide text on empty slices to avoid PDF exports in MS Word from SVG imports Oct 6, 2022
Copy link
Collaborator

@alexcjohnson alexcjohnson left a comment

Choose a reason for hiding this comment

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

💃

@archmoj archmoj merged commit e1d94b7 into master Oct 6, 2022
@archmoj archmoj deleted the pie-empty-slices branch October 6, 2022 15:53
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