Skip to content

popup example cleanup / simplification #3096

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

Merged
merged 2 commits into from
Jan 8, 2015

Conversation

fredj
Copy link
Member

@fredj fredj commented Jan 5, 2015

No description provided.

@@ -10,10 +10,8 @@
<link rel="stylesheet" href="../resources/bootstrap/css/bootstrap-responsive.min.css" type="text/css">
<style type="text/css">
.ol-popup {
display: none;
Copy link
Member

Choose a reason for hiding this comment

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

I haven't checked your branch, but do you get a flash of popup content if display is not none on load?

Copy link
Member Author

Choose a reason for hiding this comment

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

The popup is not visible on load because its position is absolute with a bottom value of 15px: the element is shifted to the bottom of the page. This can be improved by storing the div in an invisible container (https://github.com/openlayers/ol3/blob/master/examples/overlay.html#L68-L74)

fredj added 2 commits January 6, 2015 09:26
Gecko 13 (Firefox 13) removed support for -moz-box-shadow. Since then,
only the unprefixed version is supported.
@tschaub
Copy link
Member

tschaub commented Jan 8, 2015

Looks like a good simplification. Please merge.

@elemoine
Copy link
Member

elemoine commented Jan 8, 2015

LGTM too. Thanks @fredj.

fredj added a commit that referenced this pull request Jan 8, 2015
popup example cleanup / simplification
@fredj fredj merged commit 904fac0 into openlayers:master Jan 8, 2015
@fredj fredj deleted the example-cleanup branch January 8, 2015 08:07
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

Successfully merging this pull request may close these issues.

3 participants