DmitryBaranovskiy / raphael

JavaScript Vector Library
https://dmitrybaranovskiy.github.io/raphael/
MIT License
11.26k stars 1.67k forks source link

rendered Paper.text() incorrectly y-positioned on hidden papers #491

Open rse opened 12 years ago

rse commented 12 years ago

I've a web app which provides two calendar illustrations, each one on its own tab and rendered with Raphael. The problem is the following: text rendered with Paper.text() is incorrectly y-offset in the tab which is initially hidden (not visible because the other tab is selected). I debugged the situation and the root cause is the following:

On the last line of tuneText() Raphael 2.0.1 tries post-adjust the "dy" value of the first "tspan" element of the "text" element. For this it uses _getBBox() to find out the bounding box of the element. It even is clever enough to first make it temporarily visible (not hidden). But this doesn't make any difference if the outmost container (in my case the tab containing the illustration) is hidden. In WebKit browsers one always gets an empty bounding box with "undefined" "y" and "height" values, in Gecko based browsers one gets "y" and "height" set to "0" . As a result, _getBBox() returns a SVGRect with "y" and "height" set to 0 or undefined. Raphael then for "undefined" does nothing (and as a result has an incorrect y offset) and for "0" incorrectly calculates the new "dy" for the first "tspan" as: "dif = a.y - (bb.y + bb.height / 2);" which results in an effective "dif = ay" and hence if you draw text on say position (100,100) the "text" is at (100,100) and its "tspan" will be on (100,200) because the "tspan" becomes a "dy" = 100 as a result of the empty bounding box. In WekKit browsers one can see this incorrect placement most easily as the "dy" is too large, in Gecko browsers one still sees the problem as texts are offset also a small amount.

I've no real solution at hand, except for never drawing a Raphael illustration on a still hidden paper if the app has to function correctly ;-) Perhaps someone else finds a solution for this...

pragmaticdave commented 12 years ago

Our web app which draws circumplex graphs also suffers from the same bug. Firefox & Opera display fine whether rendered hidden or not, but IE9, Chrome, and Safari all render fine while NOT hidden, but hidden doubles the Y coordinate. Any outlook on a fix for this? It's been two months. Thanks. :o)

pragmaticdave commented 12 years ago

Ok so I wrote a temporary fix in our own code until Dmitry is able to attend to this issue.

Add this to your stylesheet:

.hidden {
    height: 1px;
    width: 1px;
    overflow: hidden;
    position: absolute;
    z-index: -100;
}

Then rather than applying display:none to your tab pages, use this class you just made instead.

Finally, assuming all your other hidden tabs are set to display:none, have a js check in your "toggleTab" [to selected part] function:

    if (tab == "the_raphaeled_one") {
        // Fix for bug in Raphael.js -- https://github.com/DmitryBaranovskiy/raphael/issues/491
        $('#page_' + tab).removeClass("hidden");
    } else {
        $('#page_' + tab).css('display', "");
    }

or

    if (tab == "the_raphaeled_one") {
        $('#page_' + tab).addClass("hidden");
    } else {
        $('#page_' + tab).css('display', "none");
    }
gravitypersists commented 12 years ago

I've found a hack-ish solution that might work for people until this is fixed, wrap your code in a setTimeout() function.

setTimeout(function(){
   paper.text(x, y, "text");
});

edit:

Coming back after learning how to javascript, I've learned why this works (kinda, I mean, I don't understand Raphael's code too much).

When you use setTimeout, it defers execution until the current call stack is complete. Read about timers here.

You probably don't want to litter your code with setTimeouts, as it will screw up the flow of your logic, since some code now executes much later than other code following it, which you'd naively presume is executed in reverse order if you didn't understand how setTimeout works.

I'd suggest only using this if you need a quick fix and you have no code dependent on the position of your text. I'd try and fix the problem for real but it doesn't look like the repository is getting any love. :-1:

kuroda commented 12 years ago

@gitpullgravity

Thanks a lot. Your hack solved my problem!

Frobitz commented 12 years ago

I've hit this same issue. Can't really use @pragmaticdave's fix as the items I'm hiding using display:none need to be faded in using jQuery, and @gitpullgravity's hack doesn't seem to want to work with my code either.

I'm curious as to whether this is likely to be fixed in the near future or is simply a limitation that can't be avoided?

oyatek commented 12 years ago

I had the same problem and I found a solution similar to @pragmaticdave. I'm using Tabs Plugin from Twitters's Bootsrap, so this is an extention to it, just add 2 CSS rules. First add class "tab-raphael" to a tab DIV with Raphael. Then add these 2 CSS rules:

<style>
.tab-raphael {
    display: block !important;
    position: absolute;
    top: -9999px;
}
.tab-raphael.active {
    position: relative;
    top: 0;
}
</style>
simonbarker commented 12 years ago

+1 for @pragmaticdave 's solution, I did something similar and it works well. Just remember to block out all of Bootstrap's tab CSS and Javascript or it might not work still

mdecastilho commented 11 years ago

@oyatek 's solution works fine with Twitter Bootstrap. I wonder if this will be fixed (it's been 9 months already)...

MarcinCieslak commented 11 years ago

For me such solution worked, using jQuery:

var label = paper.text(x, y, content);
$('tspan', label.node).attr('dy', 0);
Phoscur commented 11 years ago

The solution above won't work for me, neither changing CSS helps. I used setTimeout before, but it's not an option anymore, as I don't want to carry a callback for this. Also, this occurs without the paper being hidden. I can't position any text correctly in Chrome. Any chance for this getting fixed?

altrome commented 11 years ago

@marcincieslak's solution worked geat for me! Tnx a lot!

Phoscur commented 11 years ago

Nevermind my problem, just make sure the container element is already in the DOM, and thereby not hidden. @marchincieslak's solution changes text positioning on non hidden papers by some pixels (noticed this when removing this bugfix)

MrSwitch commented 11 years ago

@marcincieslak works great, i additionally filtered by ":first-child" otherwise multiple lines would overlap in the same space, i.e. $('tspan:first-child', txt.node).attr('dy', 0);

mklement0 commented 11 years ago

+1 on getting this fixed. Thanks, everyone, for reporting and posting workarounds.

zgohr commented 11 years ago

+1

MaddieM4 commented 11 years ago

I'm working on a fix for this, and I'm struggling to understand the rationale and intentions behind the post-adjustment code.

dif = a.y - (bb.y + bb.height / 2);

For my own extremely narrow set of uses, I can just replace this line with dif = fontSize / 4; and it makes a perfectly amicable approximation of existing behavior. But for obvious reasons, this feels like it's missing some important subtleties and considerations, which aren't explained in the source. I also have a sense that this might break backwards compatibility with how multiline text renders.

Soares commented 11 years ago

+1, I'm also hitting this.

shantanubhadoria commented 11 years ago

+1 I resolved this with setTimeout Function . I was setting attributes after I created the element like this

var element = paper.text();

... few lines later

element.attr(attrObject);

In this case I had to put the timeout around the part where I set attributes so this worked for me

var element = paper.text();

setTimeout(function(){ element.attr(attrObject); });

jenstitterness commented 11 years ago

+1. I'm also running into this issue.

lukaszbachman commented 11 years ago

+1 me too

zackyang000 commented 11 years ago

+1 , and use @gitpullgravity 's solution worked good.

gutierfe commented 11 years ago

@marcincieslak's solution worked geat for me! Tnx a lot!

rlebrette commented 10 years ago

Any news on this subject?

joshuabambrick commented 10 years ago

A run-once, vanilla javascript hack:

var oldText = Raphael.prototype.text;

Raphael.prototype.text = function () {
    var textElement = oldText.apply(this, arguments);

    setTimeout(function () {
        var tspanElement = textElement.node.getElementsByTagName('tspan')[0];
        if (tspanElement) tspanElement.setAttribute('dy', 0);
    }, 1);

    return textElement;
};

Simply run the above code and then you can just use text as normal.

KSoto commented 9 years ago

+1, using the suggestions provided here

KSoto commented 9 years ago

@pragmaticdave's solution didn't work for me @gitpullgravity's solution gave me errors since it's using variables and by the time it ran, it didn't know what the variables were :( @oyatek's solution didn't work, however I'm not using twitter bootstrap @marcincieslak's solution worked!! However it doubled my rendering time :( @joshbambrick's solution worked great! It increased by rendering time by about 15% but that's not that bad

Looking for this to get fixed by Raphael of course, though

MarcinCieslak commented 9 years ago

I do not think Raphael is being developed anymore. If you have a chance, consider switching to Snap http://snapsvg.io/about/

awcab commented 9 years ago

Snap hasn't seen much activity in the last year. I wouldn't be so sure that it's being actively developed either.

tomasAlabes commented 9 years ago

Any PR for @joshbambrick's fix? Because that code is more like an extension and setTimeout seems hacky...

gravitypersists commented 9 years ago

@tomasAlabes joshbambrick's fix also uses a setTimeout. It just abstracts it by monkey-patching Raphael's text method.

aleph1 commented 9 years ago

The dy attribute seems to be set on all tspans earlier in the function. I've had luck showing and hiding text elements with the following patch. It does not require a setTimeout, but I don't think it addresses the issue of a container object being hidden. It does however prevent the dy value being rewritten based on bounding box height of 0 — occurs when the text element is hidden.

Replace this:

dif && R.is(dif, "finite") && $(tspans[0], {dy: dif});

With this:

if( bb.height ) dif && R.is(dif, "finite") && $(tspans[0], {dy: dif});

panxinmiao commented 9 years ago

@aleph1 perfect!

holgerl commented 8 years ago

Why isn't this fixed yet? I ran into the same problem. My canvas is hidden when I draw it, and when it is shown the text has too much dy. I fixed it with by setting the dy manually, but that is a very awkward solution, and I have to figure out the correct value by trying and failing:

var label = window.paper.text(start[0], start[1], content);
$('tspan', label.node).attr('dy', 16)
zhaoruda commented 7 years ago

Maybe we can use 'visibility' instead of 'display' to control the element hidden or show.

holgerl commented 7 years ago

@zhaoruda Maybe you should try that and see if it works