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

Test that .getLat() and .getLon() change after map.setView() #5

Closed
jywarren opened this issue May 15, 2017 · 11 comments
Closed

Test that .getLat() and .getLon() change after map.setView() #5

jywarren opened this issue May 15, 2017 · 11 comments
Milestone

Comments

@jywarren
Copy link
Member

jywarren commented May 15, 2017

Let's add a test that ensures that the methods .getLat() and .getLon() change after the Leaflet map is interacted with. We can simulate this in tests with map.setView([lat, lon], zoom):

map.setView([51.505, -0.09], 13);
blurredLocation.getLat() == 51.505;

Later, we should ensure that it returns that value, but truncated to the current precision/zoom level, so:

var object = new basic.BlurredLocation({
  lat: 41.01,
  lon: -85.66
});
blurredLocation.getLat() == 41.01;
map.setView([51.505532, -0.092362]);
expect(blurredLocation.getLat()).toBe(51.50);

In general, testing could use a lot of Leaflet features: http://leafletjs.com/examples/quick-start/

(this can be broken out into a separate issue)

@jywarren jywarren mentioned this issue May 15, 2017
39 tasks
@mridulnagpal
Copy link
Member

@jywarren On it

@mridulnagpal
Copy link
Member

@jywarren What's blurredLocation? Is it an instance of BlurredLocation? If it is shouldn't it be named object? Little confused.

@jywarren
Copy link
Member Author

Hi, yes, it is - and actually, I think it's best to name the instance this way -- it's a convention you'll see in other libraries. I'll leave a comment to this effect on #3 too.

@mridulnagpal
Copy link
Member

@jywarren For this will I have to add an event listener that whenever setView is called any instance of class BlurredLocation will change it values to the same??

@mridulnagpal
Copy link
Member

@jywarren If you can give a green flag I will start working on it. Thanks

@jywarren
Copy link
Member Author

Ah, sorry - yes, that sounds right -- but maybe there's a more general event than setView? See docs: http://leafletjs.com/reference-1.0.3.html

OR, maybe more efficient is that since getLon/getLat are methods, you can just look up the current Leaflet map center when they're called, rather than doing it all the time?

@mridulnagpal
Copy link
Member

The map won't work till there is a map container tag div with id "map". Do I need to create it using javascript only? for tests atleast?

@jywarren
Copy link
Member Author

jywarren commented May 21, 2017 via email

@mridulnagpal
Copy link
Member

I used the fixture template and it works well, thanks. Moreover I added the build button and the tests you have mentioned above. Please have a look #3

@mridulnagpal
Copy link
Member

I think this issue is done, what say @jywarren ?

@jywarren
Copy link
Member Author

jywarren commented Jun 7, 2017

Great!

@jywarren jywarren closed this as completed Jun 7, 2017
@jywarren jywarren modified the milestone: Basic setup Jun 17, 2017
mridulnagpal added a commit that referenced this issue May 9, 2018
Less return statements + code less redundant
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

2 participants