twostraws / Ignite

A static site generator for Swift developers.
MIT License
1.71k stars 94 forks source link

Add Map Element #101

Closed dl-alexandre closed 1 month ago

dl-alexandre commented 3 months ago

Here's a basic structure showing how a Map Elements work. Greater customization included soon. (eg. Annotations and Reverse Geocoding, Annotations with a Custom Callout, Overlays of Population by US State, Callout Accessory Views, Region and Zoom Limits, Animated Polyline Overlays, Displaying Place Data by PlaceID, and Search Filtering)

A few questions...

Additional Opinions Welcome

twostraws commented 1 month ago

Sorry for taking so long to reply, and thank you for this contribution! We can look to improve the code as appropriate once it's merged in. In the meantime, I have two questions:

  1. What is that huge token string?
  2. If someone places two maps, do we run that JS twice? I wonder if it could be included only once, similarly to how the icons work.
dl-alexandre commented 1 month ago

MapKit JS typically has two scripts.

The first is standard and reusable containing authorizations required by Apple. It includes JS libraries, also required.

<script src="https://cdn.apple-mapkit.com/mk/5.x.x/mapkit.core.js"
    crossorigin async
    data-callback="initMapKit"
    data-libraries="map,annotations,services"
    data-token="IMPORTANT: ADD YOUR TOKEN HERE">
</script>

The second script, a module, includes parameters necessary for Maps which may include interactive elements or external file calls.

twostraws commented 1 month ago

Sorry, I should have been clearer: there's already a huge token in the code. What is that exact token? I'm wary about putting actual tokens into source control, although at this point it's in GitHub's history regardless. Shouldn't we perhaps make it a required thing, or perhaps ask users to enter it in at the site level to avoid having them repeat themselves?

Based on what you've said, it sounds like when we have any map, we need to include one piece of JavaScript to download the actual Maps code. We then include the second piece of JavaScript on a per-map basis, so that second one could be included three, four, five times, etc. Does that sound about right?

dl-alexandre commented 1 month ago

Yes, that token I shouldn't have submitted was non-domain specific. Being temporary, it expires after seven days. MapKit JS supports domain specific tokens which IgniteSamples can use. You may create a separate token via your developer account, due to restrictions on domains, it's not worrisome having it as public knowledge.

///  Domain Restricted Token - Solely works with https://ignitesamples.hackingwithswift.com
public let exampleSiteMapKitJSToken: String = "eyJraWQiOiI0UkRRVVRIMjJOIiwidHlwIjoiSldUIiwiYWxnIjoiRVMyNTYifQ.eyJpc3MiOiIyTVA4UVdLN1I2IiwiaWF0IjoxNzI1NTk4NDM2LCJvcmlnaW4iOiJpZ25pdGVzYW1wbGVzLmhhY2tpbmd3aXRoc3dpZnQuY29tIn0.UgaNnmSfiQsD6D23LC7-_5mEY2p_hvtM_nPOfwtV48UWl1YPX5o2YgvAJpTdjMzEQZ-IPapuIXJ7DJ4N4kdo8w"

Regarding the JavaScript, what's being figured now answers the question whether that initial code can call multiple libraries which then are divvied throughout the site or at least page.

dl-alexandre commented 1 month ago

Currently any token can be added within the Map call site. Would it be preferable (it may be easier with implementation) that that token be added within the Main Site struct via the Site protocol as an optional variable?

twostraws commented 1 month ago

Yes, I definitely think adding a token each time is a bit much. If we can centralize that it would be great!

Where did you get to with loading the JavaScript a bit less? Could you also run SwiftLint on this commit? It seems there are various whitespace changes again.

Thank you!

dl-alexandre commented 1 month ago

Implementing JavaScript will mirror how PR 108 solves it. At least map initialization will take place on a site level with libraries + variables happening within specific pages. 🗺️

dl-alexandre commented 1 month ago

Forgive me, I didn't mean for this PR's closure. I'm not sure if I can restore this or PR 108