Define the new optional properties "centerToReset" and "centerToZoom"#7
Open
dtrucs wants to merge 3 commits into20tab:mainfrom
Open
Define the new optional properties "centerToReset" and "centerToZoom"#7dtrucs wants to merge 3 commits into20tab:mainfrom
dtrucs wants to merge 3 commits into20tab:mainfrom
Conversation
Member
|
@dtrucs thanks for this PR, I'll ask for a few changes! |
| * "zoom" and "center" properties are useful when the map is mounted in a particular position | ||
| * and you need this button to reset the view to another position. | ||
| */ | ||
| zoomToReset?: number; |
Member
There was a problem hiding this comment.
Can you document these with a comment per property, like title and icon?
| * "zoom" and "center" properties are useful when the map is mounted in a particular position | ||
| * and you need this button to reset the view to another position. | ||
| */ | ||
| zoomToReset?: number; |
Member
There was a problem hiding this comment.
What do you think of naming these defaultZoom and defaultCenter?
Author
There was a problem hiding this comment.
At the start of this contribution, I also thought of these names.
But once I saw the whole thing, I wonder if it wouldn't be ambiguous with the mapContainer's center/zoom definition.
<MapContainer zoom={defaultMapZoom} center={defaultMapCenter}>
<ResetViewControl
defaultZoom={defaultZoom}
defaultCenter={defaultCenter}
/>
</MapContainer>Maybe the couple targetZoom / targetCenter?
But if you think defaultZoom / defaultCenter is better, i'm totally okay to change it!
__tests__/ResetViewControl.spec.tsx
Outdated
| describe("ResetViewControl", () => { | ||
| const mockHandleViewReset = jest.fn(); | ||
| let mapInstance = null as Map | null | ||
| const defaultMapCenter = [-96.8716348, 32.8205866] as LatLngExpression; |
Member
There was a problem hiding this comment.
let's use new LatLng
dtrucs
commented
Oct 30, 2024
dtrucs
force-pushed
the
Define-centerToReset-and-centerToZoom-properties
branch
from
88190ca to
8cfb1bc
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Thanks for this plugin!
This PR is a small enhancement to explicitly set the view to reset on a center/zoom other than those mounted by the map.