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

Popup Module does not work with SVG elements (but could?) #3043

Closed
siderealdata opened this issue Sep 16, 2015 · 8 comments
Closed

Popup Module does not work with SVG elements (but could?) #3043

siderealdata opened this issue Sep 16, 2015 · 8 comments
Milestone

Comments

@siderealdata
Copy link

I am trying to use the popup module with SVG elements produced by D3.js. I didn't expect this to be a supported feature but tried anyway since it would be great to have the consistent style of the popup across all elements on the site. It did not work, but it was not because of incompatible event binding, as I had assumed, since I could see the popup being added to the DOM. And it was not because jquery.position() was returning zero or null data. I added the debug option to the popup settings and confirmed that the correct series of events was firing, but that it was ending with "Popup target is hidden, no action taken". I found this previous issue: #2518 about an earlier bug where this same error message was given when an element was not relatively positioned. The conditional, which used to be based on position (hence the earlier bug), now appears to be based on width and height:

            if(target.width === 0 && target.height === 0) {
              module.debug('Popup target is hidden, no action taken');
              return false;
            }

SVG elements (I have tried path and circle elements) appear to be recognized as 0 width and 0 height by the DOM. If I disable this if statement in the javascript, my popups do appear (well offset, but still relative to the target).

The ability to use Semantic UI to add interactive features to a d3 chart would be a big boon, and it seems to be very close to full integration. There is still an offset problem that I will fix separately, but the workaround for the popup conditional that is now working for me is:

            if(target.width === 0 && target.height === 0 && !(target.element instanceof SVGGraphicsElement)) {
              module.debug('Popup target is hidden, no action taken');
              return false;
            }

I am not dealing with any hidden SVG elements, so this is not actually checking for hidden-ness with SVG elements, just overriding the normal width/height check in the case where it is an SVG element, and it works.

@siderealdata
Copy link
Author

Actually, my offset problem was only because I had given set the popup's context as the parent container. If I let it default to body, there is no problem whatsoever and the modified conditional above works 100% successfully.

@airbugg
Copy link

airbugg commented Sep 22, 2015

@siderealdata, I was facing the exact same problem and your solution works like a charm. Cheers!

@jlukic jlukic added this to the 2.1.5 milestone Sep 25, 2015
@jlukic
Copy link
Member

jlukic commented Sep 25, 2015

I'll add an SVG check as proposed. Thanks for the thorough post.

jlukic added a commit that referenced this issue Nov 1, 2015
@jlukic
Copy link
Member

jlukic commented Nov 1, 2015

Thanks for waiting

@jlukic jlukic closed this as completed Nov 1, 2015
jlukic added a commit that referenced this issue Nov 1, 2015
@thiphariel
Copy link

Hi.

This line

if(target.width === 0 && target.height === 0 && !(target.element instanceof SVGGraphicsElement)) {
    module.debug('Popup target is hidden, no action taken');
    return false;
}

throws a « SVGGraphicsElement » is undefined on IE 11. Any patch for this ?
My popup works on Chrome Firefox or Safari but not on IE (popup on an SVG Element)

Thanks.

EDIT : Even if I change SVGGraphicsElement to SVGElement, the error does not throw anymore but the popup still won't show itself...

@jlukic jlukic reopened this Dec 28, 2015
@jlukic
Copy link
Member

jlukic commented Dec 28, 2015

I'll have to check this out. I'm not familiar

@jlukic jlukic modified the milestones: 2.1.8, 2.1.5 Dec 28, 2015
@thiphariel
Copy link

No problem. For the moment, I changed my popup system with a modal, so it works now. But I think this "bug" should be fixed btw :)

The problem is that "SVGGraphicsElement" does not exist on IE. So, with the actual code, it throw an error. May be you should take a look with basic "SVGElement" type ? I'm not quite good with SVG aswell sorry :/

@jlukic jlukic modified the milestones: 2.1.8, 2.1.9 Jan 8, 2016
@jlukic
Copy link
Member

jlukic commented Feb 16, 2016

Fixed again in caad06c

We now type check that SVGGraphicsElement is defined

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

No branches or pull requests

4 participants