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

Merge a lot of PRs from the old repository #8

Open
wants to merge 26 commits into
base: master
Choose a base branch
from

Conversation

george-silva
Copy link
Member

Hello all!

@bhaskarvk merged a lot of Pull Requests from different maintainers into the old repository.

Please, if anyone can help review/test to see if there are any incompatibilities, let me know.

I'll leave this open for a few days, to give everyone the chance to test it out.

@bhaskarvk
Copy link

Thanks @george-silva!

bower.json Outdated
"authors": [
"Lennard Voogdt <[email protected]>"
"Lennard Voogdt <[email protected]>",
"George Silva <[email protected]",
Copy link
Collaborator

@theashyster theashyster Jan 27, 2017

Choose a reason for hiding this comment

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

Missing > at the end of email address

<link rel="stylesheet" href="http://cdn.leafletjs.com/leaflet-0.6.4/leaflet.ie.css" />
<![endif]-->
<link href="https://unpkg.com/[email protected]/css/font-awesome.css" rel="stylesheet"/>
<link rel="stylesheet" href="https://unpkg.com/[email protected]/dist/leaflet.css" />
Copy link
Collaborator

Choose a reason for hiding this comment

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

Leaflet 1.0.3 is out

</head>
<body>
<div id="map" style="width: 800px; height: 600px"></div>

<script src="http://cdn.leafletjs.com/leaflet-0.6.4/leaflet.js"></script>
<script src="https://unpkg.com/leaflet@0.7.7/dist/leaflet.js"></script>
Copy link
Collaborator

@theashyster theashyster Jan 27, 2017

Choose a reason for hiding this comment

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

Leaflet 1.0.3 is out, you should try to use that here in and in other examples

README.md Outdated
- Font Awesome 3.0
- Leaflet 0.5

Use version 2.0 for newer versions of bootstrap and font-awesome!
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use version 2.0.x for compatibility with newer versions of bootstrap, font-awesome and leaflet.
It is tested and works with the latest available versions:

  • Bootstrap 3.3.7
  • Font Awesome 4.7.0
  • Leaflet 1.0.3

@theashyster
Copy link
Collaborator

@george-silva Any progress on this one?

@george-silva
Copy link
Member Author

Hello @theashyster . I haven't been able to correct the pointers you gave us. Will do today.

@george-silva
Copy link
Member Author

@theashyster I've added you to the contributors. What is your email, so I can add it up?

@theashyster
Copy link
Collaborator

Hello @george-silva
Okay, good. Thanks, you can just add my github link there.

README.md Outdated
- Leaflet > 0.7

Use version 2.0 for newer versions of bootstrap and font-awesome!

Copy link
Collaborator

Choose a reason for hiding this comment

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

I have revisited this now, did not see that there is already something written about version 2.0, my bad.
I would suggest that this part is written a bit better all together, like this.

Version 2.0 of Leaflet.awesome-markers is tested and works with the latest available versions:

  • Bootstrap 3 (up to 3.3.7)
  • Font Awesome 4.0 (up to 4.7.0)
  • Ionicons 1.5.2
  • Leaflet 0.5 (up to 1.0.3)

Use version 2.0.x for compatibility with the newer versions of bootstrap, font-awesome and leaflet.

Version 1.0 of Leaflet.awesome-markers is tested and works with:

  • Bootstrap 2
  • Font Awesome 3.0
  • Leaflet 0.5

Use version 1.0.x for compatibility with the older versions of bootstrap, font-awesome and leaflet.

Copy link
Member Author

Choose a reason for hiding this comment

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

I concur :D.

@theashyster
Copy link
Collaborator

@george-silva Also all examples should use Leaflet 1.0.3 both CSS and JS files, I saw you made some changes for the CSS files, but there are still some left and you did not change the JS files to use the 1.0.3 version.

@george-silva
Copy link
Member Author

I saw that now. Updating.

@george-silva
Copy link
Member Author

@theashyster please, check 😄

<script src="http://cdn.leafletjs.com/leaflet-0.6.4/leaflet.js"></script>
<link href="https://unpkg.com/[email protected]/css/font-awesome.css" rel="stylesheet"/>
<link href="https://unpkg.com/[email protected]/dist/css/bootstrap.min.css" rel="stylesheet">
<link rel="stylesheet" href="https://unpkg.com/[email protected]/dist/leaflet.css" />
Copy link
Collaborator

Choose a reason for hiding this comment

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

Still 0.7.7, use 1.0.3

Copy link
Member Author

Choose a reason for hiding this comment

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

darn, missed this one.

@george-silva
Copy link
Member Author

@theashyster I'll leave this another day or two so we can test this out. If all goes well we'll merge it to dev. There is bug #9 to solve before cutting a new release.

@theashyster
Copy link
Collaborator

@george-silva Okay, cool. Yeah, I agree.

@theashyster
Copy link
Collaborator

@george-silva Any plans on merging this and also fixing #11?

@george-silva
Copy link
Member Author

george-silva commented Apr 28, 2017 via email

@george-silva
Copy link
Member Author

I'll give this a run tomorrow. If no issues appear, I'll merge. The other bug might take longer.

@theashyster
Copy link
Collaborator

@george-silva Great to hear that. Yes, merge it. The other bug can be worked on separately.

@theashyster
Copy link
Collaborator

@george-silva As this is still open, maybe you should update and test with the newest version of Leaflet that is out at the moment in 1.2.0?

@AlexGreg
Copy link
Collaborator

@george-silva what is the status of this?

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.

8 participants