Skip to content

Loading…

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

Open
rse opened this Issue · 34 comments
@rse
rse commented

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

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

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

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

@gitpullgravity

Thanks a lot. Your hack solved my problem!

@Frobitz

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

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

+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

@marcelcastilho

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

@marcincieslak

For me such solution worked, using jQuery:

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

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

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

@Phoscur

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

@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

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

@zgohr

+1

@campadrenalin

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

+1, I'm also hitting this.

@shantanubhadoria

+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

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

@lukaszbachman

+1 me too

@TossShinHwa

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

@gutierfe

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

@rlebrette

Any news on this subject?

@joshbambrick

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

+1, using the suggestions provided here

@KSoto

@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

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

@awcab

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

@tomasAlabes
Collaborator

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

@gravitypersists

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

@aleph1

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});

@buma buma referenced this issue in conveyal/otp.js
Open

Bike mode triangle doesn't work #44

@panxinmiao

@aleph1 perfect!

@holgerl

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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.