Skip to content

Bug: problems when creating a map inside display:none element #135

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

Closed
Indigo744 opened this issue Nov 23, 2015 · 16 comments
Closed

Bug: problems when creating a map inside display:none element #135

Indigo744 opened this issue Nov 23, 2015 · 16 comments

Comments

@Indigo744
Copy link
Collaborator

This has been reported several times by users: #72 and #63

Here is a working example of the problem: http://jsfiddle.net/VqwUZ/361/
We hide the container, draw the map, then show the container.
As a result, no text is visible on the legend, and the map is not set to the screen width (a resize of the window is needed to force redraw).

This is related to Raphael issue DmitryBaranovskiy/raphael#760

If you call getBBox() to get the bounding box of a text element when the whole Raphael element is hidden (e.g. if it is inside a <div> with display: none) then the properties of the returned object are all set to NaN rather than the actual bounding box values of the element.

My solution was to avoid using display:none and prefer using this kind of class:

.map_hide {
    /* display:none is doing some weird stuff on SVG
     * Solution: hide it, then position it far away
     */
    position:absolute !important;
    visibility: hidden !important;
    top:-1000px !important;
    left:-1000px !important; 
}

However, when using a CSS framework such as Bootstrap, you may not be able to change the behavior of certain element. I don't know what we can do.

@Indigo744
Copy link
Collaborator Author

After some research, there doesn't seem to be a way to have the correct boundary box for a text element without actually drawing it.

Here are our options:

  1. do nothing - this is a Raphael (and browser) issue. Mapael shouldn't do anything more
  2. try to circumvent the problem by drawing the map in a div outside viewport (using the class see above) and at the end putting back the div in its original place. Several problems arise, like what to do on update event?
  3. do nothing more, but at least warns the user - we can detect that the user is trying to draw a map that is hidden and throw an error.

I don't like the second option. The code will become really bloated and complicated just for an edge case. Still, the first option bothers me as we will always have some users coming here asking why Mapael is not working.
Hence, I think the third option is the preferred way. That way, the user will know right away that creating a map while hidden is not supported. We could provide a help page on how to circumvent the problem (calling mapael on a div hidden with above said class, and then moving it back to the destination div).

What do you think?

@neveldo
Copy link
Owner

neveldo commented Nov 24, 2015

Hello,

Thank for having investigated on this issue. I agree with you, I don't like the second option and I prefer the third one too. However, I agree with you that we should warn users, as you said, through a new section "known issues" in the documentation page (or maybe in the wiki ?). But I wonder if mapael has to check that and throw an error if a user try to draw a map on a hidden container. Indeed, a fixed size map that contains no text should work fine even if drawn in an hidden div, no ?

@Indigo744
Copy link
Collaborator Author

I fear that a user may discard mapael as "not working" if they encounter this issue while trying it, and go elsewhere. Maybe I'm thinking too much...
Indeed it should work for basic map without any text. So maybe we can add an option "force-hidden" to force the draw on hidden map?
I fear there is no perfect solution for this problem...

@neveldo
Copy link
Owner

neveldo commented Nov 25, 2015

If I had to choose, I would prefer throwing an explicit error than a new option 'force-hidden'. I think that in any cases there is always a way to call mapael only when the related container is shown anyway.

@Indigo744
Copy link
Collaborator Author

Maybe you didn't understood me.
What I suggested was:

  • normal behavior: throw an explicit error when drawing on an hidden element
  • overridden behavior (with map.bypass-hidden-check): no error is thrown (used by advanced users if they want to bypass this check...)

@neveldo
Copy link
Owner

neveldo commented Nov 26, 2015

This is what I understood. However, I'm not fond of throwing an "optionnable" error, I think it will be misleading for the users and it will complicate the code without bringing a real solution for fixing it. Maybe we should do nothing in the code but add clear information in the documentation about this bug : after all, Raphael.js is also affected by the bug and lets its users deal with it. What do you think about it ?

@Indigo744
Copy link
Collaborator Author

I think the main difference is that Raphael is a common low-level library, whereas Mapael is made to "ease the build of pretty [...] map".
Hence, we should avoid situation were the user is left with something utterly broken and no helpful error message. And with something definitely not pretty...
So while I agree to not add the override option, I still think we should throw an error.

Of course, it will be your decision at the end ;-)

Either way, we shall add clear information in the documentation with example and workaround.

@neveldo
Copy link
Owner

neveldo commented Nov 27, 2015

I'm ok with you for throwing an exception at first, and then add the option later, only if one or more users complain about not being able to add a map into an hidden div. But I confess, I'm still hesitating about this option. On the one hand, I think it's a good approach to prevent users from using the plugin in an way that is unstable. On the other, maybe some users will need to create a map in an hidden div while having no workaround to deal with it.

@Indigo744
Copy link
Collaborator Author

I get your point.
However, I wonder why a user would like to create a map that will be broken.
I think we can throw the error, and be attentive to the user feedback. If any issue is raised regarding this problem, it will be interesting to know the use case. We could then propose the bypass option if no choice is possible.
What do you think?

@neveldo
Copy link
Owner

neveldo commented Nov 30, 2015

I think we can throw the error, and be attentive to the user feedback. If any issue is raised regarding this problem, it will be interesting to know the use case. We could then propose the bypass option if no choice is possible.
What do you think?

It's ok for me :)

Indigo744 added a commit to Indigo744/jQuery-Mapael that referenced this issue Dec 10, 2015
@Indigo744
Copy link
Collaborator Author

Good new, we won't even need to provide an option. The user can simply override the check by using:

$.mapael.prototype.isPaperHidden = function() {return false;};

@neveldo
Copy link
Owner

neveldo commented Dec 11, 2015

Well done, good new indeed !

@maxime-kouemo
Copy link

Hi, I've been using Mapael for two days now. I have the same error, because I have three maps in different tabs. Only the map in the first displayed tab is drawn. The two others give me the same error as described on this thread.
I read that @Indigo744 has a fix. Can you guys please tell me where to apply the fix?

@Indigo744
Copy link
Collaborator Author

Hello

Instead of using display: none as usual to hide a tab, you should use the tab_hidden class below. Essentially, it moves the element far away so it is not seen by the user.

You just need to toggle the tab_hidden class to the tab element.

/* Default tab style, may not be needed */
.tab {
    position: static;
    visibility: visible;
    top: 0;
    left: 0; 
}
/* Hidden tab style, to be toggled to show/hide tab */
.tab_hidden {
    /* because display:none not compatible with Mapael */
    position:absolute !important;
    visibility: hidden !important;
    top:-1000px !important;
    left:-1000px !important; 
}

@amanjain-aj
Copy link

isRaphaelBBoxBugPresent: function() {
return false;
},
Using the above solves the issue

@inqdev03
Copy link

Well, I post this here if anyone will face this issue :
Under Bootstrap :

$('a[href="#idTab"]').on('shown.bs.tab', function (e) {
$(".worldMap").mapael(...)};
}

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

No branches or pull requests

5 participants