Stop re-creating template parameter pages over and over again

This is what actually happens:
* We call `addParameter()`.
* This triggers an `add` event.
* This calls an `MWTemplateDialog.onAddParameter` event handler.
* This code doesn't check if a parameter already exists (because
  it shouldn't). It detroys the page in the content pane on the
  right and recreates it from scratch.

The only reason we do this is to focus the input field on the
right. This patch introduces a dedicated event to do this.

Bug: T288827
Change-Id: I47effe05427cfabfcf534920edee79521eaa033f

modules/ve-mw/dm/models/ve.dm.MWTemplateModel.js

@@ -256,22 +256,19 @@ ve.dm.MWTemplateModel.prototype.getParameterFromId = function ( id ) {
  * @fires change
  */
 ve.dm.MWTemplateModel.prototype.addParameter = function ( param ) {
-	var name = param.getName(),
-		exists = name in this.params;
-	if ( !exists ) {
-		this.orderedParameterNames = null;
-		this.params[ name ] = param;
-		this.spec.fillFromTemplate();
-		// This forwards cange events from the nested ve.dm.MWParameterModel upwards. The array
-		// syntax is a way to call `this.emit( 'change' )`.
-		param.connect( this, { change: [ 'emit', 'change' ] } );
+	var name = param.getName();
+	if ( name in this.params ) {
+		return;
 	}
-	// FIXME: This should be skipped as well, but is currently needed for the hacks in
-	//  {@see ve.ui.MWTransclusionOutlineTemplateWidget}
+
+	this.orderedParameterNames = null;
+	this.params[ name ] = param;
+	this.spec.fillFromTemplate();
+	// This forwards cange events from the nested ve.dm.MWParameterModel upwards. The array
+	// syntax is a way to call `this.emit( 'change' )`.
+	param.connect( this, { change: [ 'emit', 'change' ] } );
 	this.emit( 'add', param );
-	if ( !exists ) {
-		this.emit( 'change' );
-	}
+	this.emit( 'change' );
 };
 
 /**

modules/ve-mw/ui/widgets/ve.ui.MWTransclusionOutlineContainerWidget.js

@@ -100,14 +100,17 @@ ve.ui.MWTransclusionOutlineContainerWidget.prototype.addPartWidget = function (
 		widget = new ve.ui.MWTransclusionOutlineTemplateWidget( part );
 		// This forwards events from the nested ve.ui.MWTransclusionOutlineTemplateWidget upwards.
 		// The array syntax is a way to call `this.emit( 'filterParameters' )`.
-		widget.connect( this, { filterParameters: [ 'emit', 'filterParameters' ] } );
+		widget.connect( this, {
+			selectParameter: 'focusPart',
+			filterParameters: [ 'emit', 'filterParameters' ]
+		} );
 	} else if ( part instanceof ve.dm.MWTemplatePlaceholderModel ) {
 		widget = new ve.ui.MWTransclusionOutlinePlaceholderWidget( part );
 	} else if ( part instanceof ve.dm.MWTransclusionContentModel ) {
 		widget = new ve.ui.MWTransclusionOutlineWikitextWidget( part );
 	}
 
-	widget.connect( this, { partHeaderClick: 'onPartHeaderClick' } );
+	widget.connect( this, { partHeaderClick: 'focusPart' } );
 
 	this.partWidgets[ part.getId() ] = widget;
 	if ( typeof newPosition === 'number' && newPosition < this.$element.children().length ) {
@@ -118,9 +121,11 @@ ve.ui.MWTransclusionOutlineContainerWidget.prototype.addPartWidget = function (
 };
 
 /**
+ * @private
  * @param {string} partId
  */
-ve.ui.MWTransclusionOutlineContainerWidget.prototype.onPartHeaderClick = function ( partId ) {
+ve.ui.MWTransclusionOutlineContainerWidget.prototype.focusPart = function ( partId ) {
+	this.bookletLayout.focus();
 	this.bookletLayout.setPage( partId );
 };
 
@@ -129,7 +134,9 @@ ve.ui.MWTransclusionOutlineContainerWidget.prototype.onPartHeaderClick = functio
  */
 ve.ui.MWTransclusionOutlineContainerWidget.prototype.clear = function () {
 	for ( var partId in this.partWidgets ) {
-		this.partWidgets[ partId ].$element.remove();
+		var widget = this.partWidgets[ partId ];
+		widget.disconnect( this );
+		widget.$element.remove();
 	}
 	this.partWidgets = {};
 };

modules/ve-mw/ui/widgets/ve.ui.MWTransclusionOutlineTemplateWidget.js

@@ -74,6 +74,11 @@ OO.inheritClass( ve.ui.MWTransclusionOutlineTemplateWidget, ve.ui.MWTransclusion
 
 /* Events */
 
+/**
+ * @event selectParameter
+ * @param {string} paramId Unique id of the parameter, e.g. something like "part_1/param1"
+ */
+
 /**
  * Triggered when the user uses the search widget at the top to filter the list of parameters.
  *
@@ -85,6 +90,7 @@ OO.inheritClass( ve.ui.MWTransclusionOutlineTemplateWidget, ve.ui.MWTransclusion
 /* Methods */
 
 /**
+ * @private
  * @param {string} paramName
  * @return {ve.ui.MWTemplateOutlineParameterCheckboxLayout}
  */
@@ -103,6 +109,7 @@ ve.ui.MWTransclusionOutlineTemplateWidget.prototype.createCheckbox = function (
 };
 
 /**
+ * @private
  * @param {ve.ui.MWTemplateOutlineParameterCheckboxLayout} checkbox
  */
 ve.ui.MWTransclusionOutlineTemplateWidget.prototype.insertCheckboxAtCanonicalPosition = function ( checkbox ) {
@@ -124,6 +131,7 @@ ve.ui.MWTransclusionOutlineTemplateWidget.prototype.insertCheckboxAtCanonicalPos
  * Handles a template model add event {@see ve.dm.MWTemplateModel}.
  * Triggered when a parameter is added to the template model.
  *
+ * @private
  * @param {ve.dm.MWParameterModel} param
  */
 ve.ui.MWTransclusionOutlineTemplateWidget.prototype.onAddParameter = function ( param ) {
@@ -157,6 +165,7 @@ ve.ui.MWTransclusionOutlineTemplateWidget.prototype.onAddParameter = function (
  * Handles a template model remove event {@see ve.dm.MWTemplateModel}.
  * Triggered when a parameter is removed from the template model.
  *
+ * @private
  * @param {ve.dm.MWParameterModel} param
  */
 ve.ui.MWTransclusionOutlineTemplateWidget.prototype.onRemoveParameter = function ( param ) {
@@ -169,6 +178,7 @@ ve.ui.MWTransclusionOutlineTemplateWidget.prototype.onRemoveParameter = function
 /**
  * Handles a parameter checkbox change event {@see ve.ui.MWTemplateOutlineParameterCheckboxLayout}
  *
+ * @private
  * @param {string} paramName
  * @param {boolean} checked New checkbox state
  */
@@ -182,22 +192,33 @@ ve.ui.MWTransclusionOutlineTemplateWidget.prototype.onCheckboxChange = function
 };
 
 /**
+ * @private
  * @param {string} paramName
+ * @fires selectParameter
  */
 ve.ui.MWTransclusionOutlineTemplateWidget.prototype.onCheckboxSelect = function ( paramName ) {
 	var param = this.templateModel.getParameter( paramName );
 	if ( param ) {
-		// FIXME: This triggers a chain of events that (re)does way to much. Replace!
-		this.templateModel.addParameter( param );
+		this.emit( 'selectParameter', param.getId() );
 	}
 };
 
+/**
+ * @private
+ * @fires selectParameter
+ */
 ve.ui.MWTransclusionOutlineTemplateWidget.prototype.addPlaceholderParameter = function () {
-	// FIXME: This triggers a chain of events that (re)does way to much. Replace!
+	var placeholder = this.templateModel.getParameter( '' );
+	if ( placeholder ) {
+		this.emit( 'selectParameter', placeholder.getId() );
+		return;
+	}
+
 	this.templateModel.addParameter( new ve.dm.MWParameterModel( this.templateModel ) );
 };
 
 /**
+ * @private
  * @param {OO.ui.Element[]} items
  */
 ve.ui.MWTransclusionOutlineTemplateWidget.prototype.onCheckboxListChange = function ( items ) {
@@ -209,6 +230,7 @@ ve.ui.MWTransclusionOutlineTemplateWidget.prototype.onCheckboxListChange = funct
  * algorithm is modelled after {@see ve.ui.MWParameterSearchWidget.buildIndex}. We search the
  * parameter's primary name, aliases, label, and description. But not e.g. the example value.
  *
+ * @private
  * @param {string} query user input
  * @fires filterParameters
  */