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.

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

Replace mapbox geocoding control with maptile geocoding control #1003

Merged

Conversation

HeyZoos
Copy link
Contributor

@HeyZoos HeyZoos commented Nov 16, 2024

Removed Mapbox dependencies by replacing Mapbox Geocoder with MapTiler GeocodingControl, restoring the Geocoding Control functionality Updated package-lock.json dependencies to remove mapbox-gl dependencies and pass precommit hook.

https://docs.maptiler.com/react/maplibre-gl-js/geocoding-control/

Fixes #864

This commit updates several nodejs dependencies to their latest versions with `npm audit fix` in order to pass the precommit hook.
Included @maptiler/geocoding-control in dependencies as a replacement for @mapbox/mapbox-gl-geocoder.
Replaced Mapbox Geocoder with MapTiler GeocodingControl for address lookup and map navigation.
The original filter condition yields seems to yield no results.
Copy link

vercel bot commented Nov 16, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
vacant-lots-proj ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 16, 2024 8:54pm

Comment on lines +50 to +57
"@typescript-eslint/eslint-plugin": "^7.16.1",
"@typescript-eslint/parser": "^7.16.1",
"eslint": "^8.56.0",
"eslint-config-next": "^14.2.5",
"eslint-config-prettier": "^9.1.0",
"eslint-plugin-custom-rules": "file:./eslint-plugin-custom-rules",
"eslint-plugin-prettier": "^5.0.0",
"eslint-plugin-react": "^7.34.4",
Copy link
Contributor Author

@HeyZoos HeyZoos Nov 16, 2024

Choose a reason for hiding this comment

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

ESLint is alphabetizing the imports

Comment on lines -28 to +27
switch (name as ExpressionName) {
switch (name as string) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is an Expression type with a match operator. But I couldn't figure out how to use it the way we did originally. Given the this is just for TypeScript, I didn't think it was a blocker.

https://maplibre.org/maplibre-style-spec/expressions/

Comment on lines 423 to 425
filter={(feature: any) => {
return feature.context.some((i: any) => {
return i.text.includes("Philadelphia");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is a Feature type, but it's missing the context in the TypeScript definition for some reason. It's confusing because it's present in the docs. So maybe the TS definitions are just out of date.

https://docs.maptiler.com/cloud/api/geocoding/#Feature

Comment on lines 423 to 425
filter={(feature: any) => {
return feature.context.some((i: any) => {
return i.text.includes("Philadelphia");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The original filter was not returning any results:

return (
  (i.id.split('.').shift() === 'place' &&
    i.text === 'Philadelphia') ||
  (i.id.split('.').shift() === 'district' &&
    i.text === 'Philadelphia County')
);

Maybe the API response body changed, but it seemed too specific to match any of the feature context objects. Here's one for example:

    "context": [
        {
            "ref": "oa:../oa-us/us/pa/philadelphia-county/eac8fa1be5800983",
            "country_code": "us",
            "id": "postal_code.3238707",
            "text": "19145-2906",
            "text_en": "19145-2906"
        },
        {
            "ref": "osm:n158288891",
            "country_code": "us",
            "id": "place.4391290",
            "text": "Girard Estates",
            "wikidata": "Q5564154",
            "kind": "place",
            "osm:place_type": "neighbourhood",
            "text_en": "Girard Estates"
        },
        {
            "ref": "osm:r7962835",
            "country_code": "us",
            "id": "neighbourhood.50999",
            "text": "South Philadelphia",
            "wikidata": "Q7568214",
            "kind": "admin_area",
            "language": "en",
            "text_en": "South Philadelphia",
            "language_en": "en"
        },
        {
            "ref": "osm:r188022",
            "country_code": "us",
            "id": "municipality.196053",
            "text": "Philadelphia",
            "wikidata": "Q1345",
            "kind": "admin_area",
            "language": "en",
            "text_en": "Philadelphia",
            "language_en": "en"
        },
        {
            "ref": "osm:r417845",
            "country_code": "us",
            "id": "county.16952",
            "text": "Philadelphia",
            "wikidata": "Q496900",
            "kind": "admin_area",
            "text_en": "Philadelphia"
        },
        {
            "ref": "osm:r162109",
            "country_code": "us",
            "id": "region.3014",
            "text": "Pennsylvania",
            "wikidata": "Q1400",
            "kind": "admin_area",
            "language": "en",
            "text_en": "Pennsylvania",
            "language_en": "en"
        },
        {
            "ref": "osm:r148838",
            "country_code": "us",
            "id": "country.1223",
            "text": "United States",
            "wikidata": "Q30",
            "kind": "admin_area",
            "language": "en",
            "text_en": "United States",
            "language_en": "en"
        },
        {
            "ref": "osm:n36966063",
            "country_code": "",
            "id": "continental_marine.4",
            "text": "North America",
            "wikidata": "Q49",
            "categories": [
                "continent"
            ],
            "language": "en",
            "osm:tags": {
                "population": "528720588",
                "wikipedia": "en:North America",
                "place": "continent",
                "sqkm": "24709000"
            },
            "text_en": "North America",
            "language_en": "en"
        }
    ]

@HeyZoos HeyZoos changed the base branch from main to staging November 16, 2024 20:45
@nlebovits
Copy link
Collaborator

Looks like addresses a both issue #864 and #877! Looks good to me. I'm going to approve and merge.

@nlebovits nlebovits merged commit 46d2612 into CodeForPhilly:staging Nov 18, 2024
4 checks passed
@HeyZoos HeyZoos deleted the issue-864-clean-up-mapbox-references branch November 18, 2024 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants