Overview of changes

[studio4]

---

Move google_api_key to helper_config class - it should not IMO be in helper_base.php
Add $disable_indicia_gmaps_inclusion static variable for use elsewhere

data_entry_helper.php

---

"google_places_api" (typo) should read "google_places"
Suggest minimum of 'label' and 'driver' for map($options) to allow control over georeference_lookup
Add static functions for new email_input and number_input

helper_base.php

---

Add email_input and number_input to $indicia_templates
Remove size="30" from date_picker (should be controlled by css)
Add propsed native_date_picker to $indicia_templates
Remove static variable $google_api_key: now in _helper_config.php
Remove static variable $google_maps_api_key: not used anywhere/deprecated

[JimBacon]

Refer to https://forums.nbn.org.uk/viewtopic.php?id=6805

[johnvanbreda]

Thanks @studio4 for the proposals - much appreciated. I've now had a good look through the changes and have the following comments:

1. If we move $google_api_key to _helper_config.php, then code which assumes $google_api_key exists (even if empty) will break on sites that have the old format - since _helper_config.php is copied to helper_config.php on install. I agree this needs refactoring but the solution needs to not break existing sites.
2. Not sure why $google_map_api_key was dropped?
3. Rather than creating new controls for email and number inputs, would it make more sense to support a type parameter which allows you to choose one of the html5 input types? See https://www.w3schools.com/html/html_form_input_types.asp.
4. There is a new template for a native_date_picker but I could not see why this was needed?
5. Rather than have a flag $disable_indicia_gmaps_inclusion, could you use the $dumped_resources property of helper_base as this already exists and is more flexible?

[studio4]

Hello @johnvanbreda,

1. If we don't move it then any sites that have google maps defined outside of client helpers break as google maps is included twice, once externally once internally. This really needs to be optional IMO given google and related services are so ubiquitous and that client_helpers is intended to be platform agnostic. I'm not sure what the best option is for older sites but I don't think it's sustainable to keep $google_api_key in helper_base.php
2. I might have mixed up my variables because there are two google api key variables (I'm not sure why there are two?). IMO anywhere that $google_maps_api_key is used should be replaced with $google_api_key and then $google_maps_api_key dropped: as far as google is concerned you only get one api key and you associate the services related to that key at Google rather than having two api keys.
3. Yes, that sounds reasonable, I wasn't thinking long term for my suggestions, more of a quick-fix but yes I agree a type parameter would work well.
4. Again this could be controlled by a parameter but I felt it would be useful to use https://www.w3schools.com/html/tryit.asp?filename=tryhtml_input_date for date input types to avoid loading additional javascript - albeit this native datepicker is not yet supported in FireFox.
5. Looking again at my initialisation script and I've not used (or defined) $disable_indicia_gmaps_inclusion in my working copy, only the branch I submitted here so it was just a suggestion, perhaps instead you could change the get_resources function for 'googlemaps' from

'googlemaps' => array('javascript' => array("$protocol://maps.google.com/maps/api/js?v=3" . (empty(self::$google_api_key) ? '' : '&key=' . self::$google_api_key))),
to
'googlemaps' => (empty(self::$google_api_key) ? '' : array('javascript' => array("$protocol://maps.google.com/maps/api/js?v=3&key=" . self::$google_api_key))),

However, this only works if you move google_api_key up to client_helpers which I think is necessary.

Hope this helps?