Update Places UI Kit Nearby Search demo #824
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -7,39 +7,55 @@ | |||||
| <!DOCTYPE html> | ||||||
| <html> | ||||||
| <head> | ||||||
| <title>Place Search Nearby with Google Maps</title> | ||||||
| <meta charset="utf-8" /> | ||||||
| <link rel="stylesheet" type="text/css" href="style.css" /> | ||||||
| <script type="module" src="./index.js"></script> | ||||||
| <script> | ||||||
| (g=>{var h,a,k,p="The Google Maps JavaScript API",c="google",l="importLibrary",q="__ib__",m=document,b=window;b=b[c]||(b[c]={});var d=b.maps||(b.maps={}),r=new Set,e=new URLSearchParams,u=()=>h||(h=new Promise(async(f,n)=>{await (a=m.createElement("script"));e.set("libraries",[...r]+"");for(k in g)e.set(k.replace(/[A-Z]/g,t=>"_"+t[0].toLowerCase()),g[k]);e.set("callback",c+".maps."+q);a.src=`https://maps.${c}apis.com/maps/api/js?`+e;d[q]=f;a.onerror=()=>h=n(Error(p+" could not load."));a.nonce=m.querySelector("script[nonce]")?.nonce||"";m.head.append(a)}));d[l]?console.warn(p+" only loads once. Ignoring:",g):d[l]=(f,...n)=>r.add(f)&&u().then(()=>d[l](f,...n))}) | ||||||
| ({key: "AIzaSyA6myHzS10YXdcazAFalmXvDkrYCp5cLc8", v: "weekly"}); | ||||||
| </script> | ||||||
| </head> | ||||||
| <body> | ||||||
| <!--[START maps_ui_kit_place_search_nearby_map] --> | ||||||
| <gmp-map center="-37.813,144.963" zoom="16" map-id="DEMO_MAP_ID"></gmp-map> | ||||||
| <div class="controls"> | ||||||
|
There was a problem hiding this comment. I'm seeing a really big change here - the controls are no longer inside the There was a problem hiding this comment. ... that said, looking more at this demo I'm thinking it would actually be better for this to move outside the map (both structurally, and visually). see later comment on this. |
||||||
| <select name="types" class="type-select"> | ||||||
|
There was a problem hiding this comment.
Suggested change
I don't think |
||||||
| <option value="">Select a place type</option> | ||||||
|
There was a problem hiding this comment. I would highly recommend not having this blank option, and instead default to a real option - that way the demo shows the API without user interaction. |
||||||
| <option value="cafe">Cafe</option> | ||||||
| <option value="restaurant">Restaurant</option> | ||||||
| <option value="electric_vehicle_charging_station" | ||||||
| >EV charging station</option | ||||||
| > | ||||||
|
|
||||||
| </select> | ||||||
| </div> | ||||||
| <div class="list-container"> | ||||||
| <gmp-place-search orientation="vertical" selectable> | ||||||
|
There was a problem hiding this comment. it's not good for this gmp-place-search to be on top of the map. it looks pretty at first glance, but there's actually a big problem doing it this way. See the map doesn't have way of being told it's obscured, so that it's effective center and bounds are less than what it thinks. This causes content you're trying to draw (markers, info windows, etc) to render underneath the big gmp-place-search ... or a lot of extra code is needed to workaround this. I would very mush recommend moving the gmp-place-search next to the map, not on top of it. There was a problem hiding this comment.
|
||||||
| <gmp-place-all-content> </gmp-place-all-content> | ||||||
|
There was a problem hiding this comment.
Suggested change
|
||||||
| <gmp-place-nearby-search-request | ||||||
| location-restriction="[email protected], 144.963"> | ||||||
| </gmp-place-nearby-search-request> | ||||||
| </gmp-place-search> | ||||||
| </div> | ||||||
|
|
||||||
|
There was a problem hiding this comment. why switch away from the simpler |
||||||
| <div id="details-container"> | ||||||
| <gmp-place-details-compact orientation="horizontal" truncation-preferred> | ||||||
| <gmp-place-details-place-request | ||||||
| place="places/ChIJ3S-JXmauEmsRUcIaWtf4MzE"></gmp-place-details-place-request> | ||||||
|
There was a problem hiding this comment. curious: why pre-set a place here in HTML? (it's really more set in JS later, right?) |
||||||
| <gmp-place-content-config> | ||||||
| <gmp-place-media></gmp-place-media> | ||||||
| <gmp-place-rating></gmp-place-rating> | ||||||
| <gmp-place-price></gmp-place-price> | ||||||
| <gmp-place-accessible-entrance-icon></gmp-place-accessible-entrance-icon> | ||||||
| <gmp-place-open-now-status></gmp-place-open-now-status> | ||||||
| <gmp-place-attribution | ||||||
| light-scheme-color="gray" | ||||||
| dark-scheme-color="white"></gmp-place-attribution> | ||||||
| </gmp-place-content-config> | ||||||
| </gmp-place-details-compact> | ||||||
| </div> | ||||||
|
|
||||||
| <!--[END maps_ui_kit_place_search_nearby_map] --> | ||||||
| </body> | ||||||
| </html> | ||||||
| <!--[END maps_ui_kit_place_search_nearby] --> | ||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -6,132 +6,165 @@ | |||||
| /* [START maps_ui_kit_place_search_nearby] */ | ||||||
|
|
||||||
| /* [START maps_ui_kit_place_search_nearby_query_selectors] */ | ||||||
| const map = document.querySelector('gmp-map') as google.maps.MapElement; | ||||||
| const placeSearch = document.querySelector('gmp-place-search') as HTMLElement; | ||||||
| const placeSearchQuery = document.querySelector( | ||||||
| 'gmp-place-nearby-search-request', | ||||||
| ) as HTMLElement; | ||||||
| const placeDetails = document.querySelector( | ||||||
| 'gmp-place-details-compact', | ||||||
| ) as HTMLElement; | ||||||
| const placeRequest = document.querySelector( | ||||||
| 'gmp-place-details-place-request', | ||||||
| ) as HTMLElement; | ||||||
| const typeSelect = document.querySelector('.type-select') as HTMLSelectElement; | ||||||
|
|
||||||
| /* [END maps_ui_kit_place_search_nearby_query_selectors] */ | ||||||
|
|
||||||
| let markers = {}; | ||||||
| let gMap; | ||||||
| let placeDetailsPopup; | ||||||
| let spherical; | ||||||
| let AdvancedMarkerElement; | ||||||
| let LatLngBounds; | ||||||
|
There was a problem hiding this comment. do we need all these new file-globals? |
||||||
|
|
||||||
| async function init(): Promise<void> { | ||||||
| const geometry = (await google.maps.importLibrary( | ||||||
| 'geometry', | ||||||
|
There was a problem hiding this comment.
Suggested change
|
||||||
| )) as google.maps.GeometryLibrary; | ||||||
| await google.maps.importLibrary('maps'); | ||||||
| await google.maps.importLibrary('places'); | ||||||
|
Comment on lines
+34
to
+35
There was a problem hiding this comment. might be good to have a comment why these two importLibrary calls are needed, since the return value isn't used. |
||||||
| const marker = (await google.maps.importLibrary( | ||||||
| 'marker', | ||||||
| )) as google.maps.MarkerLibrary; | ||||||
| const core = (await google.maps.importLibrary( | ||||||
| 'core', | ||||||
| )) as google.maps.CoreLibrary; | ||||||
|
There was a problem hiding this comment. FWIW, I slightly prefer showcasing the other format e.g.
over
|
||||||
|
|
||||||
| spherical = geometry.spherical; | ||||||
| AdvancedMarkerElement = marker.AdvancedMarkerElement; | ||||||
| LatLngBounds = core.LatLngBounds; | ||||||
|
There was a problem hiding this comment. ... because then you get to avoid this |
||||||
|
|
||||||
| gMap = map.innerMap; | ||||||
|
There was a problem hiding this comment. instead of storing a copy of innerMap as gMap, I might suggest using |
||||||
|
|
||||||
| placeDetailsPopup = new AdvancedMarkerElement({ | ||||||
|
There was a problem hiding this comment. super minor, but we probably should never use the term "popup" given the extreme negative connotation that has on the web |
||||||
| map: null, | ||||||
| content: placeDetails, | ||||||
| zIndex: 100, | ||||||
| }); | ||||||
|
There was a problem hiding this comment. use InfoWindows, not markers styled like an infowindow (very important, esp. for a11y reasons) There was a problem hiding this comment. (side note: currently I see the marker-marker replaced with the "popover"-marker, as opposed to both being visible at the same time. this would also get fixed if using proper IWs) |
||||||
|
|
||||||
| findCurrentLocation(); | ||||||
|
There was a problem hiding this comment. how much do we actually need this? I'm wondering if the added code distracts from the true purpose of this sample. (See also "Do not demonstrate APIs that aren’t in Maps JS, unless the sample’s intent is to show off intercompatibility") (It's also a little aggressive for us to ask for your location just because you are looking at our devsite) |
||||||
|
|
||||||
| gMap.addListener('click', () => { | ||||||
| hidePlaceDetailsPopup(); | ||||||
| }); | ||||||
|
|
||||||
| /* [START maps_ui_kit_place_search_nearby_event] */ | ||||||
| typeSelect.addEventListener('change', (event) => { | ||||||
| event.preventDefault(); | ||||||
|
|
||||||
| searchPlaces(); | ||||||
| }); | ||||||
|
|
||||||
| (placeSearch as any).addEventListener('gmp-select', ({place}) => { | ||||||
| if (markers[place.id]) { | ||||||
| (markers[place.id] as any).click(); | ||||||
|
There was a problem hiding this comment. generally should avoid casting to "any". this could probably be addressed by properly typing |
||||||
| } | ||||||
| }); | ||||||
| } | ||||||
| /* [END maps_ui_kit_place_search_nearby_event] */ | ||||||
| function searchPlaces() { | ||||||
| const bounds = gMap.getBounds(); | ||||||
| const cent = gMap.getCenter(); | ||||||
| const ne = bounds.getNorthEast(); | ||||||
| const sw = bounds.getSouthWest(); | ||||||
| const diameter = spherical.computeDistanceBetween(ne, sw); | ||||||
| const cappedRadius = Math.min(diameter / 2, 50000); // Radius cannot be more than 50000. | ||||||
|
|
||||||
| placeDetailsPopup.map = null; | ||||||
|
|
||||||
| for (const markerId in markers) { | ||||||
| if (Object.prototype.hasOwnProperty.call(markers, markerId)) { | ||||||
|
There was a problem hiding this comment. This line could be avoided by using a proper dict/Map data type (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map) instead of a POJO. |
||||||
| markers[markerId].map = null; | ||||||
|
There was a problem hiding this comment. try instead:
Suggested change
IMO, a bit clearer. |
||||||
| } | ||||||
| } | ||||||
| markers = {}; | ||||||
| if (typeSelect.value) { | ||||||
| map.style.height = '75vh'; | ||||||
|
There was a problem hiding this comment. why do we change to map height here? |
||||||
| placeSearch.style.visibility = 'visible'; | ||||||
| placeSearch.style.opacity = '1'; | ||||||
|
Comment on lines
+91
to
+92
There was a problem hiding this comment. overkill - only one way to hide/show needed |
||||||
| (placeSearchQuery as any).maxResultCount = 10; | ||||||
| (placeSearchQuery as any).locationRestriction = { | ||||||
| center: {lat: cent.lat(), lng: cent.lng()}, | ||||||
| radius: cappedRadius, | ||||||
| }; | ||||||
| (placeSearchQuery as any).includedTypes = [typeSelect.value]; | ||||||
| placeSearch.addEventListener('gmp-load', addMarkers, {once: true}); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| async function addMarkers() { | ||||||
| const bounds = new LatLngBounds(); | ||||||
| placeSearch.style.visibility = 'visible'; | ||||||
| placeSearch.style.opacity = '1'; | ||||||
|
There was a problem hiding this comment. ditto, overkill - only one way to hide/show needed |
||||||
|
|
||||||
| if ((placeSearch as any).places.length > 0) { | ||||||
| (placeSearch as any).places.forEach((place) => { | ||||||
| const marker = new AdvancedMarkerElement({ | ||||||
| map: gMap, | ||||||
| position: place.location, | ||||||
| collisionBehavior: | ||||||
| google.maps.CollisionBehavior.REQUIRED_AND_HIDES_OPTIONAL, | ||||||
| }); | ||||||
|
|
||||||
| (marker as any).metadata = {id: place.id}; | ||||||
|
There was a problem hiding this comment.
Suggested change
|
||||||
| markers[place.id] = marker; | ||||||
| bounds.extend(place.location); | ||||||
|
|
||||||
| /* [START maps_ui_kit_place_search_nearby_click_event] */ | ||||||
| marker.addListener('click', () => { | ||||||
| (placeRequest as any).place = place; | ||||||
| placeDetails.style.visibility = 'visible'; | ||||||
| placeDetails.style.opacity = '1'; | ||||||
|
Comment on lines
+124
to
+125
There was a problem hiding this comment.
Suggested change
|
||||||
|
|
||||||
| placeDetailsPopup.position = place.location; | ||||||
| placeDetailsPopup.map = gMap; | ||||||
|
|
||||||
| gMap.fitBounds(place.viewport, {top: 0, left: 400}); | ||||||
|
There was a problem hiding this comment. fwiw, the IW API will autopen when the IW opens. this line can go away with that change |
||||||
| }); | ||||||
| gMap.setCenter(bounds.getCenter()); | ||||||
| gMap.fitBounds(bounds); | ||||||
|
Comment on lines
+132
to
+133
There was a problem hiding this comment. I'd suggest doing this only once, after the loop is done |
||||||
| /* [END maps_ui_kit_place_search_nearby_click_event] */ | ||||||
| }); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| async function findCurrentLocation() { | ||||||
| if (navigator.geolocation) { | ||||||
| navigator.geolocation.getCurrentPosition( | ||||||
| (position) => { | ||||||
| const pos = { | ||||||
| lat: position.coords.latitude, | ||||||
| lng: position.coords.longitude, | ||||||
| }; | ||||||
| gMap.panTo(pos); | ||||||
| gMap.setZoom(16); | ||||||
|
Comment on lines
+147
to
+148
There was a problem hiding this comment. this could be set off the gmp-map directly instead of the .innerMap |
||||||
| }, | ||||||
| () => { | ||||||
| console.log('The Geolocation service failed.'); | ||||||
| gMap.setZoom(16); | ||||||
| }, | ||||||
| ); | ||||||
| } else { | ||||||
| console.log("Your browser doesn't support geolocation"); | ||||||
| gMap.setZoom(16); | ||||||
|
|
||||||
| } | ||||||
| } | ||||||
|
|
||||||
| function hidePlaceDetailsPopup() { | ||||||
| if (placeDetailsPopup.map) { | ||||||
| placeDetailsPopup.map = null; | ||||||
| placeDetails.style.visibility = 'hidden'; | ||||||
| placeDetails.style.opacity = '0'; | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| init(); | ||||||
| /* [END maps_ui_kit_place_search_nearby] */ | ||||||
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.
fwiw v=weekly is the default. it's not a required apart. though I do notice all our demos seem to include it. I'm not sure if that's an intentional choice or not?