Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
As detailed here, Google's geocoding service supports specifying a bounding box in which results will be prefered. Could you please support that in this module, too?
Comment | File | Size | Author |
---|---|---|---|
#23 | 1909706-23-support_google_biasing.patch | 2.62 KB | pcambra |
#20 | 1909706-20-support_google_biasing.patch | 3.24 KB | Sam152 |
#19 | 1909706-19-support_google_biasing.patch | 2.61 KB | andrewbelcher |
#15 | 1909706--google_bounds_parameter-15.patch | 1.99 KB | drunken monkey |
#12 | 1909706--google_bounds_parameter-12.patch | 4.23 KB | esclapes |
Comments
Comment #1
drunken monkeyAttached is a (pretty trivial) patch.
Comment #2
attiks CreditAttribution: attiks commentedI needed support for filtering as well
Comment #3
drunken monkeyWhy not just assume that the passed parameter is already in the format used by Google?
Either way, your current code shouldn't be committed, as it uses a closure and PHP 5.3 isn't required for Drupal 7.
Comment #4
attiks CreditAttribution: attiks commented#3 For a user it's much nicer/easier to write
instead of (untested)
Once we agree on the format of the parameter I'll reroll for the closure
Comment #5
drunken monkeyOK, that of course makes sense. In the end, the maintainer(s) will have to decide this, I guess, so you should probably just do a re-roll now and wait for feedback.
Comment #6
attiks CreditAttribution: attiks commentednew patch without closure
Comment #7
drunken monkeyJust realized that plugins have settings forms! Don't know how that escaped me previously.
So, adding
bounds
to the plugin settings, too.Comment #8
drunken monkeyComment #9
attiks CreditAttribution: attiks commentedLooks good to me, do we need a setting for the components as well?
Comment #10
drunken monkeyWould probably be good, yes. I don't know if it will be committed without them.
Comment #11
drunken monkeyOops, didn't mean to change the status.
Comment #12
esclapes CreditAttribution: esclapes commentedI was about to open a new feature request when saw your patch and was really happy to see how easy was to do this. I was looking for the region bias (without knowing it) and thought that it would make sense to implement both Bounding Box and region as different bias options, and let the user decide (with a select).
drunken monkey: very sorry for highjacking your issue, not sure if this is good etiquete or just rude. Let me know if you are happy with the result. I did it with the best intention. I would be happy to move my patch to a different issue if you prefer so.
attiks: I wasn't sure about what to do with filters, because they behave in a different way (filtering vs biasing) and because there are different filters available, options form will also be a bit more complex than what is necessary for biasing. How do you see it fit?
Patch attached. Change to needs review, although changes are not really that big
PS: Looking at the patch it seems my texteditor changed some lines (whitespace or tabs, i suppouse)
Comment #13
attiks CreditAttribution: attiks commented#12 I'm using this from code, so I have no idea what kind of settings form is needed, I think it will be complex and with not that much of benefits. My use case was to reliably search for cities in certain countries, either by postal code or by city name, without filters I sometimes got strange results, so that's why I added them.
I think we should support both options (as in your patch) but without adding too much settings forms
Comment #14
drunken monkeyNo problem, your patch is a valuable addition to this issue and I'm sure we should rather get this right the first time than later come and change the options again. Your changes make sense to me, could be useful.
However, I think it might be a bit unclear to end users that you enter the region into the field named "Bounding box". You should change the name and description of that form element, too, in my opinion.
Comment #15
drunken monkeyRe-roll of #7, as that one doesn't apply anymore.
Would still need a config form for the
components
parameter, in my opinion, so leaving at “needs work”.Comment #16
attiks CreditAttribution: attiks commentedFYI: the components is rather complex, so no idea if a settings form is desirable, I think most people who are going to use this, will use it from code, docs at https://developers.google.com/maps/documentation/geocoding/#ComponentFil...
Comment #17
esclapes CreditAttribution: esclapes commentedI just opened an issue to address this without bloating the interface. #2019419: Adding extra query parameters to google geocoder
May be you can help review it.
Comment #18
attiks CreditAttribution: attiks commented#17 Your approach in #2019419: Adding extra query parameters to google geocoder is a bit different, it means you have to construct the query outside of geocoder, making it harder for people to use.
I still think adding an UI for this is off topic and not really needed, but that's just my 2c
Comment #19
andrewbelcher CreditAttribution: andrewbelcher commentedHope you don't mind me weighing in with another subtly different patch. This one also includes support for region biasing and simplifies the UI by putting it into a collapsed fieldset.
However, I have stripped out some of the processing of the options to keep the form simple, which means that people configuring (either by form or code) have to pass in the correctly formatted parameters as per the linked documentation.
Comment #20
Sam152 CreditAttribution: Sam152 commented#19 no longer applied for me. I have rerolled and attached a new patch.
Comment #21
Sam152 CreditAttribution: Sam152 commentedFor adding region biasing to the geofield views plugin: #2155799: Add support for region biasing in views
Comment #22
Paul Lomax CreditAttribution: Paul Lomax commentedAs a fairly quick addition you can use without patches, this works.
Comment #23
pcambraReroll of the patch, setting as RTBC because solves the issue described
Comment #25
Simon Georges CreditAttribution: Simon Georges at Makina Corpus commentedCommitted. Thanks for the reroll and the review. If you find the time to review other patch, I'd be happy to commit them ;-)
Comment #26
pcambraThanks! :)
Comment #28
fonant CreditAttribution: fonant at Fonant Ltd commentedThanks Paul for #22, this works for geofield proximity searching too.
You do need to manually empty the cache_geocoder table in the database, though, to see the improvement.