Skip to content

Conversation

@ejhaselden
Copy link
Contributor

@ejhaselden ejhaselden commented Oct 8, 2025

Revised in accordance with demo style guidelines and updated loader version to "weekly"

@snippet-bot
Copy link

snippet-bot bot commented Oct 8, 2025

Here is the summary of changes.

You are about to add 2 region tags.
You are about to delete 2 region tags.

This comment is generated by snippet-bot.
If you find problems with this result, please file an issue at:
https://github.com/googleapis/repo-automation-bots/issues.
To update this comment, add snippet-bot:force-run label or use the checkbox below:

  • Refresh this comment

Copy link
Collaborator

@willum070 willum070 left a comment

Choose a reason for hiding this comment

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

Nice work! Don't forget to add show_ts=false in your js_sample macro tag.

<option value="restaurant">Restaurant</option>
<option value="electric_vehicle_charging_station"
>EV charging station</option
>

Choose a reason for hiding this comment

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

minor: formatting of the >

</div>
<div class="list-container">
<gmp-place-search orientation="vertical" selectable>
<gmp-place-all-content> </gmp-place-all-content>

Choose a reason for hiding this comment

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

Suggested change
<gmp-place-all-content> </gmp-place-all-content>
<gmp-place-all-content></gmp-place-all-content>

<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"});

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?

</div>
</gmp-map>
<gmp-map center="-37.813,144.963" zoom="16" map-id="DEMO_MAP_ID"></gmp-map>
<div class="controls">

Choose a reason for hiding this comment

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

I'm seeing a really big change here - the controls are no longer inside the <gmp-map>, no longer using our official slot="control-*" API

Choose a reason for hiding this comment

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

... 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.

<gmp-map center="-37.813,144.963" zoom="16" map-id="DEMO_MAP_ID"></gmp-map>
<div class="controls">
<select name="types" class="type-select">
<option value="">Select a place type</option>

Choose a reason for hiding this comment

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

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.

Comment on lines +147 to +148
gMap.panTo(pos);
gMap.setZoom(16);

Choose a reason for hiding this comment

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

this could be set off the gmp-map directly instead of the .innerMap

);
} else {
console.log("Your browser doesn't support geolocation");
gMap.setZoom(16);

Choose a reason for hiding this comment

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

ditto, direct on gmp-map

Choose a reason for hiding this comment

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

I'll come back to this file later -I suspect a lot of changes here as the other comments are addressed.

placeDetailsPopup.position = place.location;
placeDetailsPopup.map = gMap;

gMap.fitBounds(place.viewport, {top: 0, left: 400});

Choose a reason for hiding this comment

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

fwiw, the IW API will autopen when the IW opens. this line can go away with that change

</gmp-map>
<gmp-map center="-37.813,144.963" zoom="16" map-id="DEMO_MAP_ID"></gmp-map>
<div class="controls">
<select name="types" class="type-select">

Choose a reason for hiding this comment

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

Suggested change
<select name="types" class="type-select">
<select class="type-select">

I don't think name is doing anything in this demo?

@ejhaselden ejhaselden closed this Oct 24, 2025
@ejhaselden ejhaselden deleted the pkit-nearby-update branch October 24, 2025 15:32
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.

3 participants