Page MenuHomePhabricator

Googlemaps input - (1) use other inputs for geocoding, (2) hide "set marker" button
Closed, ResolvedPublic

Description

We discussed this in the Talk page here: https://www.mediawiki.org/wiki/Extension_talk:Page_Forms#Use_textinput_.22Address.22_field_as_input_for_googlemaps_.22look_up_coordinates.22_button

This proposed change has two effects:

  1. It allows other inputs to be used for geocoding by the googlemaps input instead of its own "enter address here" box. This has the potential benefits of (a) avoiding having to enter an address twice, and/or (b) saving, in Cargo, the address used for geocoding.

To achieve this, select a name for a googlemaps input, set the id of that googlemaps input as that name, and the class of the other associated inputs as that name, e.g. {{{field|Coordinates|input type=googlemaps|id=mapone}}} and {{{field|Location|input type=text|class=mapone}}}

  • If a googlemaps input has no 'id' set it operates the way it used to
  • Multiple googlemaps inputs can appear on a form.
  • Multiple inputs can be associated with each googlemap input. Their values are concatenated, separated by commas, for geocoding.
  • Each input can be associated with more than one googlemaps input, e.g. {{{field|Location|input type=text|class=mapone maptwo}}}
  • Tested for text and textarea fields, but should work with any form field with a value.
  1. It adds an ability to hide the "set marker" button, for when form users will not be expected to type coordinates as figures. Achieved by a "hidesetmarkerbutton" parameter, e.g. {{{field|Coordinates|input type=googlemaps|id=mapone|hidesetmarkerbutton}}}
index b190088..d67d94f 100644
--- a/includes/forminputs/PF_GoogleMapsInput.php
+++ b/includes/forminputs/PF_GoogleMapsInput.php
@@ -55,19 +55,24 @@ class PFGoogleMapsInput extends PFOpenLayersInput {
                $width = self::getWidth( $other_args );
                $mapCanvas = Html::element( 'div', array( 'class' => 'pfMapCanvas', 'style' => "height: $height; width: $width;" ), 'Map goes here...' );

-               $fullInputHTML = <<<END
-<div style="padding-bottom: 10px;">
-$coordsInput
-$mapUpdateButton
-</div>
-<div style="padding-bottom: 10px;">
-$addressLookupInput
-$addressLookupButton
-</div>
-$mapCanvas
+               $fullInputHTML = '<div style="padding-bottom: 10px;">';
+               if ( !array_key_exists( 'id', $other_args ) ) {
+                       $fullInputHTML .= $addressLookupInput;
+               }
+               $fullInputHTML .= $addressLookupButton;
+               $fullInputHTML .= '</div><div style="padding-bottom: 10px;">';
+               $fullInputHTML .= $coordsInput;
+               if ( !array_key_exists( 'hidesetmarkerbutton', $other_args ) ) {
+                       $fullInputHTML .= $mapUpdateButton;
+               }
+               $fullInputHTML .= '</div>';
+               $fullInputHTML .= $mapCanvas;

-END;
-               $text = Html::rawElement( 'div', array( 'class' => 'pfGoogleMapsInput' ), $fullInputHTML );
+               $divAttrs = array ('class' => 'pfGoogleMapsInput');
+               if ( array_key_exists( 'id', $other_args ) ) {
+                       $divAttrs['id'] = $other_args['id'];
+               }
+               $text = Html::rawElement( 'div', $divAttrs, $fullInputHTML );

                return $text;
        }
diff --git a/libs/PF_maps.js b/libs/PF_maps.js
index 1a918e1..fc0f2a0 100644
--- a/libs/PF_maps.js
+++ b/libs/PF_maps.js
@@ -75,7 +75,21 @@ function setupMapFormInput( inputDiv, mapService ) {
        }

        function doLookup() {
-               var addressText = inputDiv.find('.pfAddressInput').val();
+               var currentMapId = inputDiv.attr('id');
+               if (document.getElementsByClassName(currentMapId).length!=0) {
+                       var addressText = '';
+                       var allAddrs = document.getElementsByClassName(currentMapId);
+                       for (var i=0; i<allAddrs.length; i++) {
+                               if (addressText!="") {
+                                       addressText += ", ";
+                               }
+                               addressText += allAddrs[i].value;
+                       }
+               } else {
+                       var addressText = inputDiv.find('.pfAddressInput').val();
+               }
+               alert("Looking up coordinates for: " + addressText); //Remove when happy it works
+
                if ( mapService == "Google Maps" ) {
                        geocoder.geocode( { 'address': addressText }, function(results, status) {
                                if (status == google.maps.GeocoderStatus.OK) {

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptDec 13 2016, 1:03 AM
Jonathan3 updated the task description. (Show Details)Dec 13 2016, 1:06 AM

@Jonathan3 - this is great! Having the address input(s) feed into the map is something that people have been asking about for a long time, and you figured out a way to do it. I can see two ways to improve this code:

  • Instead of setting the "id" and "class" parameters, I think it's better to have a new parameter for just the text input fields, maybe called "feeds to map=", that takes in the map input's template and field name; so the parameter might look like "feeds to map=Restaurant[Coordinates]". This might remove the need for the "hidesetmarkerbutton" as well, since presumably a map being fed to should never have its own text input.
  • This is more minor, but in the doLookup() function, the code would really benefit from using jQuery selectors instead of getElementsByClassName(). If you're planning to do more JS coding, I really would recommend trying out the jQuery stuff - it takes some getting used to, but once you understand how it works it makes a lot of things a lot easier.

The first of these would probably take some work, so I don't expect you to do it - I can take care of this myself at some point. On the other hand, if you're willing to put in the work to get the patch ready for inclusion, that would be fantastic. Conversely, if you think either of these changes would be a bad idea, let me know.

Thanks. I'd be happy to have a go at this, though if I try to spend time on it before Christmas I might not survive to tell the tale.

The "hidesetmarkerbutton" parameter gets rid of the "Set marker" button rather than the "Enter address here" box. Perhaps I should I have created a separate thing here on Phabricator for it.

I'll let you know here before I start doing anything, just in case you have already started.

Sorry for the delay. I've changed it now so as not to use id and class parameters.

Now text and textarea inputs can have a "feedstomap" parameter which (as you suggested) can take the form feedstomap=Restaurant[Coordinates].

If this is done, there is no need for the maps input to have its own "Enter address here" input, so its "hideaddressinput" parameter should be set.

The "hidesetmarkerbutton" parameter is unrelated - it is intended to avoid confusion in situations where the user will not be expected to enter coordinate values.

If you are happy with this so far, you may want to consider adding the "feedstomap" option to other input types.

You may want to add spaces to the parameters.

diff --git a/includes/forminputs/PF_GoogleMapsInput.php b/includes/forminputs/PF_GoogleMapsInput.php
index b190088..d8d8aef 100644
--- a/includes/forminputs/PF_GoogleMapsInput.php
+++ b/includes/forminputs/PF_GoogleMapsInput.php
@@ -55,20 +55,22 @@ class PFGoogleMapsInput extends PFOpenLayersInput {
 		$width = self::getWidth( $other_args );
 		$mapCanvas = Html::element( 'div', array( 'class' => 'pfMapCanvas', 'style' => "height: $height; width: $width;" ), 'Map goes here...' );
 
-		$fullInputHTML = <<<END
-<div style="padding-bottom: 10px;">
-$coordsInput
-$mapUpdateButton
-</div>
-<div style="padding-bottom: 10px;">
-$addressLookupInput
-$addressLookupButton
-</div>
-$mapCanvas
-
+		$fullInputHTML = '<div style="padding-bottom: 10px;">';
+		#The "Enter address here" input box is not necessary if we are using other inputs instead
+		if ( !array_key_exists( 'hideaddressinput', $other_args ) ) {
+			$fullInputHTML .= $addressLookupInput;
+		}
+		$fullInputHTML .= $addressLookupButton;
+		$fullInputHTML .= '</div><div style="padding-bottom: 10px;">';
+		$fullInputHTML .= $coordsInput;
+		#The "Set marker" button is not necessary if users are not expected to type in coordinate values 
+		if ( !array_key_exists( 'hidesetmarkerbutton', $other_args ) ) {
+			$fullInputHTML .= $mapUpdateButton;
+		}
+		$fullInputHTML .= '</div>';
+		$fullInputHTML .= $mapCanvas;
 END;
 		$text = Html::rawElement( 'div', array( 'class' => 'pfGoogleMapsInput' ), $fullInputHTML );
-
 		return $text;
 	}
 
diff --git a/includes/forminputs/PF_TextAreaInput.php b/includes/forminputs/PF_TextAreaInput.php
index 6e7e0a5..65d90d0 100644
--- a/includes/forminputs/PF_TextAreaInput.php
+++ b/includes/forminputs/PF_TextAreaInput.php
@@ -235,7 +235,11 @@ class PFTextAreaInput extends PFFormInput {
 		if ( array_key_exists( 'placeholder', $this->mOtherArgs ) ) {
 			$textarea_attrs['placeholder'] = $this->mOtherArgs['placeholder'];
 		}
-
+		
+		if ( array_key_exists( 'feedstomap', $this->mOtherArgs ) ) {
+			$textarea_attrs['feedstomap'] = $this->mOtherArgs['feedstomap'];
+		}
+		
 		return $textarea_attrs;
 	}
 
diff --git a/includes/forminputs/PF_TextInput.php b/includes/forminputs/PF_TextInput.php
index 5ad9963..ab3ef6b 100644
--- a/includes/forminputs/PF_TextInput.php
+++ b/includes/forminputs/PF_TextInput.php
@@ -273,6 +273,10 @@ class PFTextInput extends PFFormInput {
 		if ( array_key_exists( 'placeholder', $other_args ) ) {
 			$inputAttrs['placeholder'] = $other_args['placeholder'];
 		}
+		if ( array_key_exists( 'feedstomap', $other_args ) ) {
+			$inputAttrs['feedstomap'] = $other_args['feedstomap'];
+		}
+		
 		$text = Html::input( $input_name, $cur_value, 'text', $inputAttrs );
 
 		if ( array_key_exists( 'uploadable', $other_args ) && $other_args['uploadable'] == true ) {
diff --git a/libs/PF_maps.js b/libs/PF_maps.js
index 5af15c8..7cc2051 100644
--- a/libs/PF_maps.js
+++ b/libs/PF_maps.js
@@ -75,7 +75,24 @@ function setupMapFormInput( inputDiv, mapService ) {
 	}
 
 	function doLookup() {
-		var addressText = inputDiv.find('.pfAddressInput').val();
+		//currentMap will be in the format templatename[fieldname] e.g. Address[Coordinates]
+		var currentMap = inputDiv.find('.pfCoordsInput').attr('name');
+		var allInputsForCurrentMap = document.querySelectorAll('[feedstomap="' + currentMap + '"]');
+		if (allInputsForCurrentMap.length!=0) {
+			//Concatenate the values of each input which feeds to this map 
+			var addressText = '';
+			for (var i=0; i<allInputsForCurrentMap.length; i++) {
+				if (addressText!="") {
+					addressText += ", ";
+				}
+				addressText += allInputsForCurrentMap[i].value;
+			}
+		} else {
+			//No other inputs feed to this map so revert to the standard "Enter address here" input
+			var addressText = inputDiv.find('.pfAddressInput').val();
+		}
+		alert("Looking up coordinates for: " + addressText); //Remove when happy it works
+		
 		if ( mapService == "Google Maps" ) {
 			geocoder.geocode( { 'address': addressText }, function(results, status) {
 				if (status == google.maps.GeocoderStatus.OK) {

I wonder whether it would be possible to avoid the need for the "hideaddressinput" parameter.

If the "feedstomap" parameter (in the text input) were paired up with a "mapname" parameter (in the maps input) then this latter parameter could be used to determine whether to display the "Enter address here" input.

Alternatively, could jQuery somehow be used to hide the "Enter address here" input whenever a text input has the "feedstomap" parameter?

As an afterthought, I made a small change so that if "hidesetmarker" is set (i.e. the user is not expected to type in coordinate values) then the Coordinates input box is set to "readonly". See the first four green lines below.

diff --git a/includes/forminputs/PF_GoogleMapsInput.php b/includes/forminputs/PF_GoogleMapsInput.php
index b190088..7dd08f8 100644
--- a/includes/forminputs/PF_GoogleMapsInput.php
+++ b/includes/forminputs/PF_GoogleMapsInput.php
@@ -46,6 +46,10 @@ class PFGoogleMapsInput extends PFOpenLayersInput {
 			'value' => PFOpenLayersInput::parseCoordinatesString( $cur_value ),
 			'size' => 40
 		);
+		if ( array_key_exists( 'hidesetmarkerbutton', $other_args ) ) {
+			#User is not expected to type in coordinates so make this input readonly 
+			$coordsInputAttrs['readonly'] = 'readonly';
+		}
 		$coordsInput = Html::element( 'input', $coordsInputAttrs );
 		$wgPageFormsTabIndex++;
 		$mapUpdateButton = Html::element( 'input', array( 'type' => 'button', 'class' => 'pfUpdateMap', 'value' => wfMessage( 'pf-maps-setmarker' )->parse() ), null );
@@ -55,20 +59,22 @@ class PFGoogleMapsInput extends PFOpenLayersInput {
 		$width = self::getWidth( $other_args );
 		$mapCanvas = Html::element( 'div', array( 'class' => 'pfMapCanvas', 'style' => "height: $height; width: $width;" ), 'Map goes here...' );
 
-		$fullInputHTML = <<<END
-<div style="padding-bottom: 10px;">
-$coordsInput
-$mapUpdateButton
-</div>
-<div style="padding-bottom: 10px;">
-$addressLookupInput
-$addressLookupButton
-</div>
-$mapCanvas
-
+		$fullInputHTML = '<div style="padding-bottom: 10px;">';
+		#The "Enter address here" input box is not necessary if we are using other inputs instead
+		if ( !array_key_exists( 'hideaddressinput', $other_args ) ) {
+			$fullInputHTML .= $addressLookupInput;
+		}
+		$fullInputHTML .= $addressLookupButton;
+		$fullInputHTML .= '</div><div style="padding-bottom: 10px;">';
+		$fullInputHTML .= $coordsInput;
+		#The "Set marker" button is not necessary if users are not expected to type in coordinate values 
+		if ( !array_key_exists( 'hidesetmarkerbutton', $other_args ) ) {
+			$fullInputHTML .= $mapUpdateButton;
+		}
+		$fullInputHTML .= '</div>';
+		$fullInputHTML .= $mapCanvas;
 END;
 		$text = Html::rawElement( 'div', array( 'class' => 'pfGoogleMapsInput' ), $fullInputHTML );
-
 		return $text;
 	}
 
diff --git a/includes/forminputs/PF_TextAreaInput.php b/includes/forminputs/PF_TextAreaInput.php
index 6e7e0a5..65d90d0 100644
--- a/includes/forminputs/PF_TextAreaInput.php
+++ b/includes/forminputs/PF_TextAreaInput.php
@@ -235,7 +235,11 @@ class PFTextAreaInput extends PFFormInput {
 		if ( array_key_exists( 'placeholder', $this->mOtherArgs ) ) {
 			$textarea_attrs['placeholder'] = $this->mOtherArgs['placeholder'];
 		}
-
+		
+		if ( array_key_exists( 'feedstomap', $this->mOtherArgs ) ) {
+			$textarea_attrs['feedstomap'] = $this->mOtherArgs['feedstomap'];
+		}
+		
 		return $textarea_attrs;
 	}
 
diff --git a/includes/forminputs/PF_TextInput.php b/includes/forminputs/PF_TextInput.php
index 5ad9963..ab3ef6b 100644
--- a/includes/forminputs/PF_TextInput.php
+++ b/includes/forminputs/PF_TextInput.php
@@ -273,6 +273,10 @@ class PFTextInput extends PFFormInput {
 		if ( array_key_exists( 'placeholder', $other_args ) ) {
 			$inputAttrs['placeholder'] = $other_args['placeholder'];
 		}
+		if ( array_key_exists( 'feedstomap', $other_args ) ) {
+			$inputAttrs['feedstomap'] = $other_args['feedstomap'];
+		}
+		
 		$text = Html::input( $input_name, $cur_value, 'text', $inputAttrs );
 
 		if ( array_key_exists( 'uploadable', $other_args ) && $other_args['uploadable'] == true ) {
diff --git a/libs/PF_maps.js b/libs/PF_maps.js
index 5af15c8..7cc2051 100644
--- a/libs/PF_maps.js
+++ b/libs/PF_maps.js
@@ -75,7 +75,24 @@ function setupMapFormInput( inputDiv, mapService ) {
 	}
 
 	function doLookup() {
-		var addressText = inputDiv.find('.pfAddressInput').val();
+		//currentMap will be in the format templatename[fieldname] e.g. Address[Coordinates]
+		var currentMap = inputDiv.find('.pfCoordsInput').attr('name');
+		var allInputsForCurrentMap = document.querySelectorAll('[feedstomap="' + currentMap + '"]');
+		if (allInputsForCurrentMap.length!=0) {
+			//Concatenate the values of each input which feeds to this map 
+			var addressText = '';
+			for (var i=0; i<allInputsForCurrentMap.length; i++) {
+				if (addressText!="") {
+					addressText += ", ";
+				}
+				addressText += allInputsForCurrentMap[i].value;
+			}
+		} else {
+			//No other inputs feed to this map so revert to the standard "Enter address here" input
+			var addressText = inputDiv.find('.pfAddressInput').val();
+		}
+		alert("Looking up coordinates for: " + addressText); //Remove when happy it works
+		
 		if ( mapService == "Google Maps" ) {
 			geocoder.geocode( { 'address': addressText }, function(results, status) {
 				if (status == google.maps.GeocoderStatus.OK) {

This is great! I don't think either of the two "hide" parameters are necessary- though I don't really understand the "hidesetmarker" one. Yes, hiding the address input, if map feeding is being done, should be doable via either PHP or JavaScript - probably JavaScript is easier.

Yes, I think calling it "feeds to map" is better than "feedstomap".

I'm pleased you like it so far :-)

Typing in a set of coordinates and clicking "set marker" is one of the three methods of adding coordinates to the Googlemaps input - but there will be many situations in which the website user would not be expected to know how to obtain coordinates and type them in. In these cases, it would make sense, and avoid confusion, to remove the ability to type anything into the coordinates box (make it readonly), and also to remove the "set marker" box entirely. This is what the "hidesetmarker" parameter would achieve.

For the situation where other inputs are being used to feed the address into the Googlemaps input, I'll try to find a way of removing the "Enter address here" box without requiring an extra Cargo parameter to achieve it. Incidentally I think "Calculate coordinates using address" might be better than "Look up coordinates" as the default text for the button here.

It's great that you're going to look into automatically detecting map feeding!

I agree that the map form input interface is not ideal, and I know that it causes at least some confusion to everyone who first sees it. But I'm not convinced that disabling the coordinates "set marker" button is the right approach. I don't know if there's any way for a wiki admin to know for sure that none of their users will benefit from having that button. I would much rather make that part of the interface clearer, so that the people who don't need to use it (the vast majority, to be sure) don't need to waste too much time thinking about it.

I've been thinking today about how to improve this interface - I probably should have thought more about it a long time ago, but better late than never. What do you think about this idea: there's no "Set marker" button; instead, if a user goes in and edits the coordinates value in any way, two buttons appear alongside the coordinates input: a check mark and an "X". Clicking the check mark will update the map, while clicking the X will restore the coordinates value back to what it was before, i.e. back to the value shown on the map.

What do you think of that idea?

I *am* sure that none of my users will have a need for "set marker" :-)

However, I think your idea is really good.

I could then make the input box readonly using jQuery. The problem with using jQuery in Common.js to remove the "set query" button is that it flashes up before it disappears. I had tried the following:

$( function() {
  $('.pfCoordsInput').prop('readonly', true);
  $('.pfUpdateMap').hide();
} );

Here is a way of automatically detecting map feeding with jQuery. The relevant lines in the big diff below are:

-		$addressLookupInput = Html::element( 'input', array( 'type' => 'text', 'tabindex' => $wgPageFormsTabIndex, 'class' => 'pfAddressInput', 'size' => 40, 'placeholder' => wfMessage( 'pf-maps-enteraddress' )->parse() ), null );
+		$addressLookupInput = Html::element( 'input', array( 'type' => 'text', 'tabindex' => $wgPageFormsTabIndex, 'class' => 'pfAddressInput', 'size' => 40, 'placeholder' => wfMessage( 'pf-maps-enteraddress' )->parse(), 'map' => $input_name ), null );


+	$("[feedstomap]").each(function() {
+		var value = $(this).attr("feedstomap");
+		$("[map='" + value + "']").hide();
+	});

It automatically adds a "map" attribute to the "enter address here boxes". For each "feedstomap" attribute found, it uses the "map" attribute to hide the corresponding "enter address here" box.

It "hides" it once per "feedstomap" attribute found, which allows for multiple googlemaps inputs in one form, some with hidden address boxes and some with them visible. I guess it is inefficient as it potentially "hides" the same address box multiple times (if there are multiple "feedstomap" inputs corresponding to that box).

But in any event it leads to the "enter address here" box being displayed until the page, including the Google map, is fully loaded, which can take some time, at which point it disappears.

SO.............

I think that the best way considered so far is to have a "feeds to map" parameter for each relevant text/textarea/etc input, and a corresponding "map" parameter for the googlemaps input. This would work like my initial version (in which classes were equivalent to the "feeds to map" parameter/atttribute and id's were equivalent to the "map" parameter/attribute) and would avoid the "enter address here" box flashing up and disappearing.

What do you think? Would you like to have a go at that?

diff --git a/includes/forminputs/PF_GoogleMapsInput.php b/includes/forminputs/PF_GoogleMapsInput.php
index b190088..bc0cab0 100644
--- a/includes/forminputs/PF_GoogleMapsInput.php
+++ b/includes/forminputs/PF_GoogleMapsInput.php
@@ -46,29 +46,36 @@ class PFGoogleMapsInput extends PFOpenLayersInput {
 			'value' => PFOpenLayersInput::parseCoordinatesString( $cur_value ),
 			'size' => 40
 		);
+		if ( array_key_exists( 'hidesetmarkerbutton', $other_args ) ) {
+			#User is not expected to type in coordinates so make this input readonly 
+			$coordsInputAttrs['readonly'] = 'readonly';
+		}
 		$coordsInput = Html::element( 'input', $coordsInputAttrs );
 		$wgPageFormsTabIndex++;
 		$mapUpdateButton = Html::element( 'input', array( 'type' => 'button', 'class' => 'pfUpdateMap', 'value' => wfMessage( 'pf-maps-setmarker' )->parse() ), null );
-		$addressLookupInput = Html::element( 'input', array( 'type' => 'text', 'tabindex' => $wgPageFormsTabIndex, 'class' => 'pfAddressInput', 'size' => 40, 'placeholder' => wfMessage( 'pf-maps-enteraddress' )->parse() ), null );
+		$addressLookupInput = Html::element( 'input', array( 'type' => 'text', 'tabindex' => $wgPageFormsTabIndex, 'class' => 'pfAddressInput', 'size' => 40, 'placeholder' => wfMessage( 'pf-maps-enteraddress' )->parse(), 'map' => $input_name ), null );
 		$addressLookupButton = Html::element( 'input', array( 'type' => 'button', 'class' => 'pfLookUpAddress', 'value' => wfMessage( 'pf-maps-lookupcoordinates' )->parse() ), null );
 		$height = self::getHeight( $other_args );
 		$width = self::getWidth( $other_args );
 		$mapCanvas = Html::element( 'div', array( 'class' => 'pfMapCanvas', 'style' => "height: $height; width: $width;" ), 'Map goes here...' );
 
-		$fullInputHTML = <<<END
-<div style="padding-bottom: 10px;">
-$coordsInput
-$mapUpdateButton
-</div>
-<div style="padding-bottom: 10px;">
-$addressLookupInput
-$addressLookupButton
-</div>
-$mapCanvas
-
+		$fullInputHTML = '<div style="padding-bottom: 10px;">';
+		#The "Enter address here" input box is not necessary if we are using other inputs instead
+		//if ( !array_key_exists( 'hideaddressinput', $other_args ) ) {
+		//Commenting out the "if" as now trying it with jQuery
+			$fullInputHTML .= $addressLookupInput;
+		//}
+		$fullInputHTML .= $addressLookupButton;
+		$fullInputHTML .= '</div><div style="padding-bottom: 10px;">';
+		$fullInputHTML .= $coordsInput;
+		#The "Set marker" button is not necessary if users are not expected to type in coordinate values 
+		if ( !array_key_exists( 'hidesetmarkerbutton', $other_args ) ) {
+			$fullInputHTML .= $mapUpdateButton;
+		}
+		$fullInputHTML .= '</div>';
+		$fullInputHTML .= $mapCanvas;
 END;
 		$text = Html::rawElement( 'div', array( 'class' => 'pfGoogleMapsInput' ), $fullInputHTML );
-
 		return $text;
 	}
 
diff --git a/includes/forminputs/PF_TextAreaInput.php b/includes/forminputs/PF_TextAreaInput.php
index 6e7e0a5..65d90d0 100644
--- a/includes/forminputs/PF_TextAreaInput.php
+++ b/includes/forminputs/PF_TextAreaInput.php
@@ -235,7 +235,11 @@ class PFTextAreaInput extends PFFormInput {
 		if ( array_key_exists( 'placeholder', $this->mOtherArgs ) ) {
 			$textarea_attrs['placeholder'] = $this->mOtherArgs['placeholder'];
 		}
-
+		
+		if ( array_key_exists( 'feedstomap', $this->mOtherArgs ) ) {
+			$textarea_attrs['feedstomap'] = $this->mOtherArgs['feedstomap'];
+		}
+		
 		return $textarea_attrs;
 	}
 
diff --git a/includes/forminputs/PF_TextInput.php b/includes/forminputs/PF_TextInput.php
index 5ad9963..ab3ef6b 100644
--- a/includes/forminputs/PF_TextInput.php
+++ b/includes/forminputs/PF_TextInput.php
@@ -273,6 +273,10 @@ class PFTextInput extends PFFormInput {
 		if ( array_key_exists( 'placeholder', $other_args ) ) {
 			$inputAttrs['placeholder'] = $other_args['placeholder'];
 		}
+		if ( array_key_exists( 'feedstomap', $other_args ) ) {
+			$inputAttrs['feedstomap'] = $other_args['feedstomap'];
+		}
+		
 		$text = Html::input( $input_name, $cur_value, 'text', $inputAttrs );
 
 		if ( array_key_exists( 'uploadable', $other_args ) && $other_args['uploadable'] == true ) {
diff --git a/libs/PF_maps.js b/libs/PF_maps.js
index 5af15c8..2d5df97 100644
--- a/libs/PF_maps.js
+++ b/libs/PF_maps.js
@@ -75,7 +75,24 @@ function setupMapFormInput( inputDiv, mapService ) {
 	}
 
 	function doLookup() {
-		var addressText = inputDiv.find('.pfAddressInput').val();
+		//currentMap will be in the format templatename[fieldname] e.g. Address[Coordinates]
+		var currentMap = inputDiv.find('.pfCoordsInput').attr('name');
+		var allInputsForCurrentMap = document.querySelectorAll('[feedstomap="' + currentMap + '"]');
+		if (allInputsForCurrentMap.length!=0) {
+			//Concatenate the values of each input which feeds to this map 
+			var addressText = '';
+			for (var i=0; i<allInputsForCurrentMap.length; i++) {
+				if (addressText!="") {
+					addressText += ", ";
+				}
+				addressText += allInputsForCurrentMap[i].value;
+			}
+		} else {
+			//No other inputs feed to this map so revert to the standard "Enter address here" input
+			var addressText = inputDiv.find('.pfAddressInput').val();
+		}
+		alert("Looking up coordinates for: " + addressText); //Remove when happy it works
+		
 		if ( mapService == "Google Maps" ) {
 			geocoder.geocode( { 'address': addressText }, function(results, status) {
 				if (status == google.maps.GeocoderStatus.OK) {
@@ -179,4 +196,13 @@ jQuery(document).ready( function() {
 	jQuery(".pfOpenLayersInput").each( function() {
 		setupMapFormInput( jQuery(this), "OpenLayers" );
 	});
+	
+	
+	$("[feedstomap]").each(function() {
+		var value = $(this).attr("feedstomap");
+		$("[map='" + value + "']").hide();
+	});
+	
+	
+	
 });

Hi,

Great! I think this patch is getting closer to what we need.

I think that the best way considered so far is to have a "feeds to map" parameter for each relevant text/textarea/etc input, and a corresponding "map" parameter for the googlemaps input.

I don't understand - how is that different from the current patch?

I don't understand - how is that different from the current patch?

The current patch has a "feeds to map" parameter for the text inputs which uses the format "Tablename[Fieldname]" - this format allows it to detect which map it relates to, but there is nothing to allow the googlemaps input to know to suppress the "Enter address here" box - and using using jQuery means that the box flashes up and disappears, which looks bad.

What I think is best is to have "feeds to map" using any format chosen by the (e.g. "my map") and a corresponding "map" parameter for the googlemaps input which uses a matching value (e.g. "my map") to (a) work out which text feeds to which map, and ALSO (b) suppress the "Enter address here" box using PHP.

Okay, I get it. It does seem that a PHP-based solution (don't show "Enter address here" in the first place) would be much better than a JS-based solution (show "Enter address here", then hide it).

Why can't this be done if "feeds to map" gets the standard "TemplateName[FieldName]" format, though?

Also, on a mostly-unrelated note, handling for the two "hide..." params should be removed from this patch, based on what we talked about, no?

Why can't this be done if "feeds to map" gets the standard "TemplateName[FieldName]" format, though?

That would work just as well.

Also, on a mostly-unrelated note, handling for the two "hide..." params should be removed from this patch, based on what we talked about, no?

The "hidesetmarker" one can definitely go as it's unrelated, and dealt with by your alternative plan.

The "hideaddressinput" (or a better-named alternative, e.g. "has external address") might still be necessary as the way to let the googlemaps input know not to show its address input box. Though I guess we could use a PHP variable for that - do you you want me to look into that possibility?

Ok, here we go:

  • The text/textarea inputs use a global array $wgPageFormsExternalAddress when they come across "feeds to map" parameters.
  • The googlemaps inputs check the same global array, and do not show the "enter address here" box if that particular googlemaps input is being fed an external address.

The user just needs to use something like this in the form page...

| {{{field|Address|input type=text|feeds to map=Contact[Coordinates]}}}

... and there is no need for any other, corresponding parameter elsewhere in the form.

diff --git a/includes/forminputs/PF_GoogleMapsInput.php b/includes/forminputs/PF_GoogleMapsInput.php
index b190088..5beb86c 100644
--- a/includes/forminputs/PF_GoogleMapsInput.php
+++ b/includes/forminputs/PF_GoogleMapsInput.php
@@ -55,20 +55,20 @@ class PFGoogleMapsInput extends PFOpenLayersInput {
 		$width = self::getWidth( $other_args );
 		$mapCanvas = Html::element( 'div', array( 'class' => 'pfMapCanvas', 'style' => "height: $height; width: $width;" ), 'Map goes here...' );
 
-		$fullInputHTML = <<<END
-<div style="padding-bottom: 10px;">
-$coordsInput
-$mapUpdateButton
-</div>
-<div style="padding-bottom: 10px;">
-$addressLookupInput
-$addressLookupButton
-</div>
-$mapCanvas
+		$fullInputHTML = '<div style="padding-bottom: 10px;">';
+		#The "Enter address here" input box is not necessary if we are using other inputs instead
+		global $wgPageFormsExternalAddress;
+		if ( !$wgPageFormsExternalAddress[$input_name] ) {
+			$fullInputHTML .= $addressLookupInput;
+		} 
+		$fullInputHTML .= $addressLookupButton;
+		$fullInputHTML .= '</div><div style="padding-bottom: 10px;">';
+		$fullInputHTML .= $coordsInput;
+		$fullInputHTML .= $mapUpdateButton;
+		$fullInputHTML .= '</div>';
+		$fullInputHTML .= $mapCanvas;
 
-END;
 		$text = Html::rawElement( 'div', array( 'class' => 'pfGoogleMapsInput' ), $fullInputHTML );
-
 		return $text;
 	}
 
diff --git a/includes/forminputs/PF_TextAreaInput.php b/includes/forminputs/PF_TextAreaInput.php
index 6e7e0a5..86021bd 100644
--- a/includes/forminputs/PF_TextAreaInput.php
+++ b/includes/forminputs/PF_TextAreaInput.php
@@ -235,7 +235,13 @@ class PFTextAreaInput extends PFFormInput {
 		if ( array_key_exists( 'placeholder', $this->mOtherArgs ) ) {
 			$textarea_attrs['placeholder'] = $this->mOtherArgs['placeholder'];
 		}
-
+		
+		if ( array_key_exists( 'feeds to map', $this->mOtherArgs ) ) {
+			global $wgPageFormsExternalAddress;
+			$wgPageFormsExternalAddress[$other_args['feeds to map']] = true;
+			$textarea_attrs['feeds-to-map'] = $this->mOtherArgs['feeds to map'];
+		}
+		
 		return $textarea_attrs;
 	}
 
diff --git a/includes/forminputs/PF_TextInput.php b/includes/forminputs/PF_TextInput.php
index 5ad9963..4f6afa5 100644
--- a/includes/forminputs/PF_TextInput.php
+++ b/includes/forminputs/PF_TextInput.php
@@ -273,6 +273,12 @@ class PFTextInput extends PFFormInput {
 		if ( array_key_exists( 'placeholder', $other_args ) ) {
 			$inputAttrs['placeholder'] = $other_args['placeholder'];
 		}
+		if ( array_key_exists( 'feeds to map', $other_args ) ) {
+			global $wgPageFormsExternalAddress;
+			$wgPageFormsExternalAddress[$other_args['feeds to map']] = true;
+			$inputAttrs['feeds-to-map'] = $other_args['feeds to map'];
+		}
+		
 		$text = Html::input( $input_name, $cur_value, 'text', $inputAttrs );
 
 		if ( array_key_exists( 'uploadable', $other_args ) && $other_args['uploadable'] == true ) {
diff --git a/libs/PF_maps.js b/libs/PF_maps.js
index 5af15c8..cf030f7 100644
--- a/libs/PF_maps.js
+++ b/libs/PF_maps.js
@@ -75,7 +75,24 @@ function setupMapFormInput( inputDiv, mapService ) {
 	}
 
 	function doLookup() {
-		var addressText = inputDiv.find('.pfAddressInput').val();
+		//currentMap will be in the format templatename[fieldname] e.g. Address[Coordinates]
+		var currentMap = inputDiv.find('.pfCoordsInput').attr('name');
+		var allInputsForCurrentMap = document.querySelectorAll('[feeds-to-map="' + currentMap + '"]');
+		if (allInputsForCurrentMap.length!=0) {
+			//Concatenate the values of each input which feeds to this map 
+			var addressText = '';
+			for (var i=0; i<allInputsForCurrentMap.length; i++) {
+				if (addressText!="") {
+					addressText += ", ";
+				}
+				addressText += allInputsForCurrentMap[i].value;
+			}
+		} else {
+			//No other inputs feed to this map so revert to the standard "Enter address here" input
+			var addressText = inputDiv.find('.pfAddressInput').val();
+		}
+		alert("Looking up coordinates for: " + addressText); //Remove when happy it works
+		
 		if ( mapService == "Google Maps" ) {
 			geocoder.geocode( { 'address': addressText }, function(results, status) {
 				if (status == google.maps.GeocoderStatus.OK) {

@Jonathan3 - sorry for the delay. This is fantastic! I haven't tried it out, but this looks like an extremely clean solution, and will provide a very helpful feature. I just have a few small comments:

  • Self-defined HTML attributes are supposed to start with "data-", according to the HTML spec (PF doesn't always follow this, but it should) - so "feeds-to-map" should be something like "data-feeds-to-map" instead.
  • Maybe it would be simpler to have $wgPageFormsExternalAddress hold those field names as its values, instead of its keys?

If you don't want to take care of these, I can do it. If, on the other hand, you object to either of these changes, let me know.

I'm pleased you like it :-)

  • I'd be happy to edit the attribute names as you suggest once I hear from you.
  • I did $wgPageFormsExternalAddress like that as it was the neatest way I could think of to avoid duplication - if a key is set to true multiple times, it's still just there once. It seemed better than checking if a value exists before adding it, or removing duplicates before checking it, or just leaving duplicates there. But maybe there is a good way of doing this.

What other input types (in addition to text and textarea) should be added? I don't know if this is even possible with Page Forms, but sometimes a drop-down list or radio buttons of addresses are presented by websites when you enter a (UK) postcode. If we are to cover this sort of thing then I guess we could add radiobutton and dropdown (probably not checkboxes or listbox).

Sorry for the delay. I'll try to finalise this over the next few days.

Sorry I never responded to this!

The current structure of $wgPageFormsExternalAddress seems like the right approach, then.

"text" and "textarea" seem like the only really important input types to handle - I've never heard of a case where there was a pre-defined set of addresses to choose from. But if you know of such a thing, I suppose it wouldn't hurt to add this functionality to more input types.

I've changed "feeds-to-map" to "data-feeds-to-map".

It's fairly common in the UK (with banks, big shops etc) to enter a postcode and be presented with a list of the addresses in that postcode. I think the data is from the Royal Mail's Postcode Address File service. I'm not sure if Page Forms would be able to do this (present options which are the results of a database query based on another option), PAF is expensive for most organisations, and I've never used PAF in any event. It would probably make more sense to leave our current work at text and textarea, and consider adding other input types later.

One other proposal here is to change "Look up coordinates" to "Calculate coordinates using address" as I think that would make sense to more people.

Finally, the "Remove when happy it works" line will need to be removed.

I hope this all works for you when you check it out!

diff --git a/i18n/en.json b/i18n/en.json
index ebefa68..e22c535 100644
--- a/i18n/en.json
+++ b/i18n/en.json
@@ -239,7 +239,7 @@
 	"pf-select2-selection-too-big": "This field cannot hold more than {{PLURAL:$1|$1 value|$1 values}}.",
 	"pf-maps-setmarker": "Set marker",
 	"pf-maps-enteraddress": "Enter address here",
-	"pf-maps-lookupcoordinates": "Look up coordinates",
+	"pf-maps-lookupcoordinates": "Calculate coordinates using address",
 	"pf-adminlinks-datastructure": "Data structure",
 	"pf-datepicker-close": "Close",
 	"pf-datepicker-prev": "Previous",
diff --git a/includes/forminputs/PF_GoogleMapsInput.php b/includes/forminputs/PF_GoogleMapsInput.php
index b190088..5beb86c 100644
--- a/includes/forminputs/PF_GoogleMapsInput.php
+++ b/includes/forminputs/PF_GoogleMapsInput.php
@@ -55,20 +55,20 @@ class PFGoogleMapsInput extends PFOpenLayersInput {
 		$width = self::getWidth( $other_args );
 		$mapCanvas = Html::element( 'div', array( 'class' => 'pfMapCanvas', 'style' => "height: $height; width: $width;" ), 'Map goes here...' );
 
-		$fullInputHTML = <<<END
-<div style="padding-bottom: 10px;">
-$coordsInput
-$mapUpdateButton
-</div>
-<div style="padding-bottom: 10px;">
-$addressLookupInput
-$addressLookupButton
-</div>
-$mapCanvas
+		$fullInputHTML = '<div style="padding-bottom: 10px;">';
+		#The "Enter address here" input box is not necessary if we are using other inputs instead
+		global $wgPageFormsExternalAddress;
+		if ( !$wgPageFormsExternalAddress[$input_name] ) {
+			$fullInputHTML .= $addressLookupInput;
+		} 
+		$fullInputHTML .= $addressLookupButton;
+		$fullInputHTML .= '</div><div style="padding-bottom: 10px;">';
+		$fullInputHTML .= $coordsInput;
+		$fullInputHTML .= $mapUpdateButton;
+		$fullInputHTML .= '</div>';
+		$fullInputHTML .= $mapCanvas;
 
-END;
 		$text = Html::rawElement( 'div', array( 'class' => 'pfGoogleMapsInput' ), $fullInputHTML );
-
 		return $text;
 	}
 
diff --git a/includes/forminputs/PF_TextAreaInput.php b/includes/forminputs/PF_TextAreaInput.php
index 6e7e0a5..c66b1b4 100644
--- a/includes/forminputs/PF_TextAreaInput.php
+++ b/includes/forminputs/PF_TextAreaInput.php
@@ -235,7 +235,13 @@ class PFTextAreaInput extends PFFormInput {
 		if ( array_key_exists( 'placeholder', $this->mOtherArgs ) ) {
 			$textarea_attrs['placeholder'] = $this->mOtherArgs['placeholder'];
 		}
-
+		
+		if ( array_key_exists( 'feeds to map', $this->mOtherArgs ) ) {
+			global $wgPageFormsExternalAddress;
+			$wgPageFormsExternalAddress[$other_args['feeds to map']] = true;
+			$textarea_attrs['data-feeds-to-map'] = $this->mOtherArgs['feeds to map'];
+		}
+		
 		return $textarea_attrs;
 	}
 
diff --git a/includes/forminputs/PF_TextInput.php b/includes/forminputs/PF_TextInput.php
index 5ad9963..4b68027 100644
--- a/includes/forminputs/PF_TextInput.php
+++ b/includes/forminputs/PF_TextInput.php
@@ -273,6 +273,12 @@ class PFTextInput extends PFFormInput {
 		if ( array_key_exists( 'placeholder', $other_args ) ) {
 			$inputAttrs['placeholder'] = $other_args['placeholder'];
 		}
+		if ( array_key_exists( 'feeds to map', $other_args ) ) {
+			global $wgPageFormsExternalAddress;
+			$wgPageFormsExternalAddress[$other_args['feeds to map']] = true;
+			$inputAttrs['data-feeds-to-map'] = $other_args['feeds to map'];
+		}
+		
 		$text = Html::input( $input_name, $cur_value, 'text', $inputAttrs );
 
 		if ( array_key_exists( 'uploadable', $other_args ) && $other_args['uploadable'] == true ) {
diff --git a/libs/PF_maps.js b/libs/PF_maps.js
index 5af15c8..a2ad40d 100644
--- a/libs/PF_maps.js
+++ b/libs/PF_maps.js
@@ -75,7 +75,24 @@ function setupMapFormInput( inputDiv, mapService ) {
 	}
 
 	function doLookup() {
-		var addressText = inputDiv.find('.pfAddressInput').val();
+		//currentMap will be in the format templatename[fieldname] e.g. Address[Coordinates]
+		var currentMap = inputDiv.find('.pfCoordsInput').attr('name');
+		var allInputsForCurrentMap = document.querySelectorAll('[data-feeds-to-map="' + currentMap + '"]');
+		if (allInputsForCurrentMap.length!=0) {
+			//Concatenate the values of each input which feeds to this map 
+			var addressText = '';
+			for (var i=0; i<allInputsForCurrentMap.length; i++) {
+				if (addressText!="") {
+					addressText += ", ";
+				}
+				addressText += allInputsForCurrentMap[i].value;
+			}
+		} else {
+			//No other inputs feed to this map so revert to the standard "Enter address here" input
+			var addressText = inputDiv.find('.pfAddressInput').val();
+		}
+		alert("Looking up coordinates for: " + addressText); //Remove when happy it works
+		
 		if ( mapService == "Google Maps" ) {
 			geocoder.geocode( { 'address': addressText }, function(results, status) {
 				if (status == google.maps.GeocoderStatus.OK) {

This looks great! I have one question, about something I didn't notice before: why did you switch the address-lookup and coordinate-entry inputs?

Jonathan3 added a comment.EditedJul 6 2017, 7:19 PM

Pleased you like it!

Sorry for not mentioning that change. I think it makes sense in this order:


"Enter address here" input box -------- "Calculate coordinates from address" button

"Coordinates" input box ---------------- "Set marker" button

Google map


The most common scenario is for the user to enter an address, and for the coordinates (and the map marker) to be the output. It just seems to make sense for the input to appear before the output. I think it would be confusing for the coordinates input box to appear at the top, particularly until the "set marker" button is replaced with something more intuitive.

In the more rare case where the user enters coordinates manually, that sort of person is likely to be technically savvy enough to understand that the first line should be ignored.

Yaron_Koren closed this task as Resolved.Jul 28 2017, 3:09 PM

Better late than never! I finally checked in this patch, with some modifications. I did include the text change you made, to "Calculate coordinates using address", since it makes a lot more sense in the context of "feeds to map". I'm looking forward to announcing this great new feature. And I'm also looking forward to implementing that other, related change we discussed - replacing the "Set marker" button for the raw coordinates field with "checkmark" and "X" buttons that only show up if the user edits that field.

Anyway, thanks again!