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.

Sign up for GitHub

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

Jump to bottom

feat: error on missing zoom or center #308

Merged
merged 15 commits into from
Apr 18, 2024
Merged

feat: error on missing zoom or center #308

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 Apr 10, 2024
901254f
feat: type require either zoom or initZoom, same for center
maciej-ka Apr 10, 2024
379ccfa
fix: remove duplicate definition
maciej-ka Apr 10, 2024
b35e75f
test: add madeup zoom and center to couple of test cases
maciej-ka Apr 10, 2024
31d4d85
feat: skip ts check failing for strange reason
maciej-ka Apr 10, 2024
56928ef
feat: revert changes in types
maciej-ka Apr 11, 2024
af61261
feat: error on undefined center or zoom prop
maciej-ka Apr 11, 2024
9925d7e
refactor: rename to MapCameraState
maciej-ka Apr 11, 2024
15ded50
refactor: move check to use-map-instance
maciej-ka Apr 11, 2024
933f154
Merge branch 'main' into 283-error-on-missing-center-or-zoom
maciej-ka Apr 11, 2024
a28747d
test: check that error is present when info is missing
maciej-ka Apr 11, 2024
17cf95f
Merge branch 'main' into 283-error-on-missing-center-or-zoom
maciej-ka Apr 13, 2024
0753f71
revert renaming MapCameraProps to MapCameraState
usefulthink Apr 18, 2024
02a9cdc
fix: log a warning instead of throwing an error, make sure the map re…
usefulthink Apr 18, 2024
000afc9
fix: avoid typescript equals import
usefulthink Apr 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions src/components/map/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,17 +37,20 @@ export type {
} from './use-map-events';

export type MapCameraProps = {
center: google.maps.LatLngLiteral;
zoom: number;
heading?: number;
tilt?: number;
};
} & (
| {center: google.maps.LatLngLiteral}
| {defaultCenter: google.maps.LatLngLiteral}
) &
({zoom: number} | {defaultZoom: number});

/**
* Props for the Google Maps Map Component
*/
export type MapProps = google.maps.MapOptions &
MapEventProps &
MapCameraProps &
Copy link
Contributor Author

@maciej-ka maciej-ka Apr 10, 2024 •
edited
Loading

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 it zoom and center are not a Map property.

Copy link
Collaborator

Choose a reason for hiding this comment

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

zoom and center are defined in google.maps.MapOptions so they haven't been repeated here.

MapCameraProps isn't included since it is intended to be used together with the MapCameraChangedEvent (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

const [cameraState, setCameraState] =
useState<MapCameraProps>(INITIAL_CAMERA_STATE);
// we only want to receive cameraChanged events from the map the
// user is interacting with:
const [activeMap, setActiveMap] = useState(1);
const handleCameraChange = useCallback((ev: MapCameraChangedEvent) => {
setCameraState(ev.detail);
}, []);

Maybe it should be renamed to MapCameraState to avoid confusion with the props?

Copy link
Contributor Author

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.

DeckGlCompatProps & {
/**
* An id for the map, this is required when multiple maps are present
Expand All @@ -67,8 +70,6 @@ export type MapProps = google.maps.MapOptions &
*/
controlled?: boolean;

defaultCenter?: google.maps.LatLngLiteral;
defaultZoom?: number;
defaultHeading?: number;
defaultTilt?: number;
/**
Expand Down