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

fix for values < 1 #1

Merged
merged 3 commits into from Nov 20, 2017
Merged

fix for values < 1 #1

merged 3 commits into from Nov 20, 2017

Conversation

obama
Copy link
Contributor

@obama obama commented Oct 14, 2017

now works with values < 1 too

now works with values < 1 too
scale.style.width = Math.round(this.options.maxWidth * (nauticalMiles / maxNauticalMiles)) - 10 + 'px';
scale.innerHTML = nauticalMiles + ' nm';
scale.style.width = Math.round(this.options.maxWidth * (nauticalMiles / maxNauticalMiles)) + 'px';
scale.innerHTML = nauticalMiles + ' nmi';
Copy link
Owner

Choose a reason for hiding this comment

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

Hmmm... this is now an interesting point.
There are some abbreviation for the Nautical Mile.

Maybe we should make the configurable depending of the needs of the user.

Wikipedia show us the Problem: https://en.wikipedia.org/wiki/Nautical_mile#Unit_symbol

We should use a Default and if someone want to have something else, it should be possible to do this over an option:

map.addControl(new L.Control.ScaleNautic({
metric: true,
imperial: true,
nautic: true,
nauticUnit: "nmi"
}));

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, good idea. i didn't even consider this.

Copy link
Owner

Choose a reason for hiding this comment

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

hey @obama
can we split this ?
remove this and the I will accept the pull request

after that we can add the nauticUnit as an Option ;)

@obama
Copy link
Contributor Author

obama commented Nov 19, 2017

the Travis CI fails because some phantomjs thing or so?

@wattnpapa
Copy link
Owner

yes there was a problem with travis
I just updated the travis file no it should work

@wattnpapa wattnpapa merged commit 11186ea into wattnpapa:master Nov 20, 2017
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.

None yet

2 participants