cheesegrits / filament-google-maps

Google Maps package for Filament PHP
212 stars 60 forks source link

Geocomplete - Is there a way to get 'establishment' AND 'address' types within the same field? #87

Open CharlieEtienne opened 4 months ago

CharlieEtienne commented 4 months ago

Hi,

In the getTypes() method in Geocomplete class, it defaults to 'geocode', making impossible to get results from Establishments AND Addresses.

public function getTypes(): array
    {
        $types = $this->evaluate($this->types);

        if (count($types) === 0) {
            // $types = ['geocode'];
        }

        return $types;
    }

If I comment out the default value it works like I want, but could you provide a cleaner solution?

Thanks

cheesegrits commented 3 months ago

You should be able to specify the types you want to use in the ->autocomplete() method, using the 'types' argument, which takes an array of types, constrained according to the Google PIA docs:

https://developers.google.com/maps/documentation/javascript/place-autocomplete#constrain-place-types

CharlieEtienne commented 3 months ago

the problem is that it allows to constrain, but I don't want to constrain. I want all types of results, and that's not possible when you specify a type property with any combination (according to my tests) I read all the docs 3 times and tried all sort of combinations, and went to this conclusion. That's why I sent a PR, I don't see the point of specifying a default since your code works well without these two lines

CharlieEtienne commented 3 months ago

From the docs:

If no constraint is specified, all types are returned.

Also, is there any good reason to default with 'geocode'?

mabdelfattah commented 2 months ago

@CharlieEtienne Is there a way to implement your overrides locally until the maintainer accepts the pull request?

CharlieEtienne commented 2 months ago

@CharlieEtienne Is there a way to implement your overrides locally until the maintainer accepts the pull request?

@mabdelfattah yes, in the vendor files, see the changes here, it's very simple, just a few lines to remove/comment out in two files

mabdelfattah commented 2 months ago

@mabdelfattah yes, in the vendor files, see the changes here, it's very simple, just a few lines to remove/comment out in two files

@CharlieEtienne Yeah, but this is almost impossible on production 😊 so probably I need to clone both files to my project and then use them instead of using the packages classes.

kengraversen commented 2 months ago

I have successfully used the package "cweagans/composer-patches" to apply patch files for the two mentioned files. I created the patch files using the git diff function and placed them in a separate directory. This way I separate the original (and otherwise excellent) package and the focused fix (while we wait for the official fix). AND - since I am no expert in this field, I didn't even know the composer-patches package, I got some help from ChatGPT ;-)

CharlieEtienne commented 2 months ago

@kengraversen 🤔 Why don't you just copy paste the Map or Geocomplete class and use yours instead of the plugin's one?

// use Cheesegrits\FilamentGoogleMaps\Fields\Map;
// use Cheesegrits\FilamentGoogleMaps\Fields\Geocomplete;
use App\Overrides\Map;
use App\Overrides\Geocomplete;
kengraversen commented 2 months ago

@kengraversen 🤔 Why don't you just copy paste the Map class and use yours instead of the plugin's one?

use App\Overrides\Map;

Honestly, I wouldn't know how to do that while keeping my code structure clean and controlled. I see your point - and also that it may be apparently easier - but the way I have done it will ensure that I can update the original package without losing my patches (I believe).