-
Notifications
You must be signed in to change notification settings - Fork 95
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
feat: error on missing zoom or center #308
Merged
usefulthink
merged 15 commits into
visgl:main
from
maciej-ka:283-error-on-missing-center-or-zoom
Apr 18, 2024
Merged
Changes from 3 commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
64fd5b5
feat: handle missing center or zoom prop
maciej-ka 901254f
feat: type require either zoom or initZoom, same for center
maciej-ka 379ccfa
fix: remove duplicate definition
maciej-ka b35e75f
test: add madeup zoom and center to couple of test cases
maciej-ka 31d4d85
feat: skip ts check failing for strange reason
maciej-ka 56928ef
feat: revert changes in types
maciej-ka af61261
feat: error on undefined center or zoom prop
maciej-ka 9925d7e
refactor: rename to MapCameraState
maciej-ka 15ded50
refactor: move check to use-map-instance
maciej-ka 933f154
Merge branch 'main' into 283-error-on-missing-center-or-zoom
maciej-ka a28747d
test: check that error is present when info is missing
maciej-ka 17cf95f
Merge branch 'main' into 283-error-on-missing-center-or-zoom
maciej-ka 0753f71
revert renaming MapCameraProps to MapCameraState
usefulthink 02a9cdc
fix: log a warning instead of throwing an error, make sure the map re…
usefulthink 000afc9
fix: avoid typescript equals import
usefulthink File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like inclusion of
MapCameraProps
is missing? Without itzoom
andcenter
are not a Map property.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
zoom
andcenter
are defined ingoogle.maps.MapOptions
so they haven't been repeated here.MapCameraProps
isn't included since it is intended to be used together with theMapCameraChangedEvent
(it's a type you can use for the details of that event), see here for example:react-google-maps/examples/multiple-maps/src/app.tsx
Lines 33 to 41 in 9618024
Maybe it should be renamed to
MapCameraState
to avoid confusion with the props?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. I would add that rename.