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] Set default value for maxZoom and limit maxZoom option #242

Closed
wants to merge 2 commits into from

Conversation

d1vyanshu-kumar
Copy link

[fix] Set default value for maxZoom and limit maxZoom option

This commit addresses the issue of limiting the maximum zoom level. The following changes have been made:

  1. Set the default value to 12 if not specified:
    self.config.maxZoom = self.config.maxZoom || 12;
  2. Limit the maxZoom option of Leaflet to the value of self.config.maxZoom:
    options.maxZoom = self.config.maxZoom;

Closes #188

The issue of empty responses from the tile server when zooming in beyond the supported zoom level has been addressed by setting the default maxZoom level to 12. This change ensures that tiles are displayed up to the supported zoom level.

If a different default maxZoom level is desired, it can be modified by adjusting the value of .                                              Closes #188
The issue of empty responses from the tile server when zooming in beyond the supported zoom level has been addressed by setting the default maxZoom level to 12. This change ensures that tiles are displayed up to the supported zoom level.

If a different default maxZoom level is desired, it can be modified by adjusting the value of .                                              Closes #188
Copy link
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

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

I gave it a try and it doesn't seem to work to me, I can still zoom aggressively as shown in the capture below.

netjsongraph js-zoom

Test and documentation are missing.

self.bboxData = {
nodes: [],
links: [],
};
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand what are these lines for?

},
});

self.bboxData = {
Copy link
Member

Choose a reason for hiding this comment

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

what's this for?

// Add the following lines to set the default maxZoom and make it configurable
self.config.maxZoom = self.config.maxZoom || 12;

if (self.type === "netjson") {
Copy link
Member

Choose a reason for hiding this comment

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

there's already some identical if statements below, why are you adding more?
Beware of copy pasta code.

@d1vyanshu-kumar
Copy link
Author

@nemesifier I apologize for any inconvenience caused. I will make an effort to address the issue promptly. There were certain lines that I didn’t fully comprehend, so I copied them. However, I am committed to resolving this matter as soon as possible.

@d1vyanshu-kumar
Copy link
Author

d1vyanshu-kumar commented Feb 27, 2024

#242 (comment)

I initially believed that I needed to replicate the function which is mentioned and simply add the following line. and I resolved the zoom level issue using this formula:
The formula to convert resolution (in meters per pixel) to zoom level
Solving for Zoom Level:
Zoom Level = \log_2(5218.101131)
text{Zoom Level} \approx 12.3
Given that Zoom levels are whole numbers, I should round this to Zoom Level 12.
then I simply copy the function which is below and add this line.
That was my thought process for addressing the problem, which I now realize was incorrect. Now I have identified the actual issue. I will solve this issue soon.
@nemesifier Thank you for your review.

@d1vyanshu-kumar d1vyanshu-kumar closed this by deleting the head repository Feb 28, 2024
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.

[bug] Map default zoom levels seem to allow for more zoom than supported
2 participants