UI: cleanup magic for signal-mechanism

src/UI/Component/JavaScriptBindable.php

@@ -37,10 +37,21 @@ interface JavaScriptBindable {
 	 *		});
 	 *
 	 * @param	\Closure	$binder
-	 * @param	self
+	 * @return	self
 	 */
 	public function withOnLoadCode(\Closure $binder);
 
+	/**
+	 * Add some onload-code to the component instead of replacing the existing one.
+	 *
+	 * Must work like getOnLoadCode was called and the result of the existing binder
+	 * (if any) and the result of the new binder are concatenated in a new binder.
+	 *
+	 * @param	\Closure	$binder
+	 * @return	self
+	 */
+	public function withAdditionalOnLoadCode(\Closure $binder);
+
 	/**
 	 * Get the currently bound on load code.
 	 *

src/UI/Component/Triggerable.php

@@ -12,7 +12,7 @@
  *
  * @package ILIAS\UI\Component
  */
-interface Triggerable {
+interface Triggerable extends JavaScriptBindable {
 
 	/**
 	 * Get a component like this but reset (regenerate) its signals.
@@ -21,4 +21,4 @@ interface Triggerable {
 	 */
 	public function withResetSignals();
 
-}
+}

src/UI/Component/Triggerer.php

@@ -11,7 +11,7 @@
  *
  * @package ILIAS\UI\Component
  */
-interface Triggerer {
+interface Triggerer extends JavaScriptBindable {
 
 	/**
 	 * Get a component like this but reset any triggered signals of other components
@@ -26,5 +26,4 @@ public function withResetTriggeredSignals();
 	 * @return TriggeredSignalInterface[]
 	 */
 	public function getTriggeredSignals();
-
-}
+}

src/UI/Implementation/Component/Button/Renderer.php

@@ -101,11 +101,6 @@ protected function renderClose($component) {
 
 	protected function maybeRenderId(Component\Component $component, $tpl) {
 		$id = $this->bindJavaScript($component);
-		// Check if the button is acting as triggerer
-		if ($component instanceof Component\Triggerer && count($component->getTriggeredSignals())) {
-			$id = ($id === null) ? $this->createId() : $id;
-			$this->triggerRegisteredSignals($component, $id);
-		}
 		if ($id !== null) {
 			$tpl->setCurrentBlock("with_id");
 			$tpl->setVariable("ID", $id);
@@ -128,11 +123,6 @@ protected function renderMonth(Component\Button\Month $component, RendererInterf
 
 		$id = $this->bindJavaScript($component);
 
-		// Check if the button is acting as triggerer
-		if ($component instanceof Component\Triggerer && count($component->getTriggeredSignals())) {
-			$id = ($id === null) ? $this->createId() : $id;
-			$this->triggerRegisteredSignals($component, $id);
-		}
 		if ($id !== null) {
 			$tpl->setCurrentBlock("with_id");
 			$tpl->setVariable("ID", $id);

src/UI/Implementation/Component/Dropdown/Renderer.php

@@ -63,11 +63,6 @@ protected function renderItems($items, $tpl, $default_renderer)
 
 	protected function maybeRenderId(Component\Component $component, $tpl, $block, $template_var) {
 		$id = $this->bindJavaScript($component);
-		// Check if the component is acting as triggerer
-		if ($component instanceof Component\Triggerer && count($component->getTriggeredSignals())) {
-			$id = ($id === null) ? $this->createId() : $id;
-			$this->triggerRegisteredSignals($component, $id);
-		}
 		if ($id !== null) {
 			$tpl->setCurrentBlock($block);
 			$tpl->setVariable($template_var, $id);

src/UI/Implementation/Component/Glyph/Renderer.php

@@ -33,10 +33,6 @@ public function render(Component\Component $component, RendererInterface $defaul
 		$id = $this->bindJavaScript($component);
 
 		$tpl->touchBlock($component->getType());
-		if ($component instanceof Component\Triggerer && count($component->getTriggeredSignals())) {
-			$id = ($id === null) ? $this->createId() : $id;
-			$this->triggerRegisteredSignals($component, $id);
-		}
 
 		if ($id !== null) {
 			$tpl->setCurrentBlock("with_id");

src/UI/Implementation/Component/JavaScriptBindable.php

@@ -24,6 +24,21 @@ public function withOnLoadCode(\Closure $binder) {
 		return $clone;
 	}
 
+	/**
+	 * @see \ILIAS\UI\Component\JavaScriptBindable::withAdditionalOnLoadCode
+	 */
+	public function withAdditionalOnLoadCode(\Closure $binder) {
+		$current_binder = $this->getOnLoadCode();
+		if ($current_binder === null) {
+			return $this->withOnLoadCode($binder);
+		}
+
+		$this->checkBinder($binder);
+		return $this->withOnLoadCode(function($id) use ($current_binder, $binder) {
+			return $current_binder($id)."\n".$binder($id);
+		});		
+	}
+
 	/**
 	 * @see \ILIAS\UI\Component\JavaScriptBindable::getOnLoadCode
 	 */

src/UI/Implementation/Component/Modal/Renderer.php

@@ -47,27 +47,43 @@ public function registerResources(ResourceRegistry $registry) {
 	 * @param Component\Modal\Modal $modal
 	 * @param string $id
 	 */
-	protected function registerSignals(Component\Modal\Modal $modal, $id) {
+	protected function registerSignals(Component\Modal\Modal $modal) {
 		$show = $modal->getShowSignal();
 		$close = $modal->getCloseSignal();
-		$js = $this->getJavascriptBinding();
 		$options = json_encode(array(
 			'ajaxRenderUrl' => $modal->getAsyncRenderUrl(),
 			'keyboard' => $modal->getCloseWithKeyboard(),
 		));
-		$js->addOnLoadCode("
-			$(document).on('{$show}', function() { il.UI.modal.showModal('{$id}', {$options}); return false; });
-			$(document).on('{$close}', function() { il.UI.modal.closeModal('{$id}'); return false; });"
-		);
+		// ATTENTION, ATTENTION:
+		// with(Additional)OnLoadCode opens a wormhole into the future, where some unspecified
+		// entity magically created an id for the component that can be used to refer to it
+		// via javascript.
+		// This replaced a pattern, where an id was created manually and the java script
+		// code was manually inserted to the (now internal) js-binding of the
+		// AbstractComponentRenderer. (see commit 192144fd1f0e040cadc0149c3dc15fbc4b67858e).
+		// The wormhole solution is considered superior over the manual creation of ids because:
+		// * withAdditionalOnLoadCode introduces no new principles to the UI framework but reuses
+		//   an existing one
+		// * withAdditionalOnLoadCode does not require it to expose internals (js-binding) from
+		//   the AbstractComponentRenderer and thus does have less coupling
+		// * withAdditionalOnLoadCode allows the framework to decide, when ids are actually
+		//   created
+		// * since withAdditionalOnLoadCode refers to some yet unknown future, it disencourages
+		//   tempering with the id _here_.
+		return $modal->withAdditionalOnLoadCode(function($id) use ($show, $close, $options) {
+			return
+				"$(document).on('{$show}', function() { il.UI.modal.showModal('{$id}', {$options}); return false; });".
+				"$(document).on('{$close}', function() { il.UI.modal.closeModal('{$id}'); return false; });";
+		});
 	}
 
 	/**
 	 * @param Component\Modal\Modal $modal
 	 * @return string
 	 */
 	protected function renderAsync(Component\Modal\Modal $modal) {
-		$id = $this->createId();
-		$this->registerSignals($modal, $id);
+		$modal = $this->registerSignals($modal);
+		$id = $this->bindJavaScript($modal);
 		return "<span id='{$id}'></span>";
 	}
 
@@ -79,9 +95,8 @@ protected function renderAsync(Component\Modal\Modal $modal) {
 	 */
 	protected function renderInterruptive(Component\Modal\Interruptive $modal, RendererInterface $default_renderer) {
 		$tpl = $this->getTemplate('tpl.interruptive.html', true, true);
-		$id = $this->createId();
-		$this->registerSignals($modal, $id);
-		$this->triggerRegisteredSignals($modal, $id);
+		$modal = $this->registerSignals($modal);
+		$id = $this->bindJavaScript($modal);
 		$tpl->setVariable('ID', $id);
 		$tpl->setVariable('FORM_ACTION', $modal->getFormAction());
 		$tpl->setVariable('TITLE', $modal->getTitle());
@@ -113,9 +128,8 @@ protected function renderInterruptive(Component\Modal\Interruptive $modal, Rende
 	 */
 	protected function renderRoundTrip(Component\Modal\RoundTrip $modal, RendererInterface $default_renderer) {
 		$tpl = $this->getTemplate('tpl.roundtrip.html', true, true);
-		$id = $this->createId();
-		$this->registerSignals($modal, $id);
-		$this->triggerRegisteredSignals($modal, $id);
+		$modal = $this->registerSignals($modal);
+		$id = $this->bindJavaScript($modal);
 		$tpl->setVariable('ID', $id);
 		$tpl->setVariable('TITLE', $modal->getTitle());
 		foreach ($modal->getContent() as $content) {
@@ -141,11 +155,10 @@ protected function renderRoundTrip(Component\Modal\RoundTrip $modal, RendererInt
 	 */
 	protected function renderLightbox(Component\Modal\Lightbox $modal, RendererInterface $default_renderer) {
 		$tpl = $this->getTemplate('tpl.lightbox.html', true, true);
-		$id = $this->createId();
-		$this->registerSignals($modal, $id);
-		$this->triggerRegisteredSignals($modal, $id);
+		$modal = $this->registerSignals($modal);
+		$id = $this->bindJavaScript($modal);
 		$tpl->setVariable('ID', $id);
-		$id_carousel = $this->createId();
+		$id_carousel = "{$id}_carousel";
 		$pages = $modal->getPages();
 		$tpl->setVariable('TITLE', $pages[0]->getTitle());
 		$tpl->setVariable('ID_CAROUSEL', $id_carousel);

src/UI/Implementation/Component/Popover/Popover.php

@@ -5,6 +5,7 @@
 use \ILIAS\UI\Component;
 use ILIAS\UI\Component\Signal;
 use ILIAS\UI\Implementation\Component\ComponentHelper;
+use ILIAS\UI\Implementation\Component\JavaScriptBindable;
 use ILIAS\UI\Implementation\Component\SignalGeneratorInterface;
 
 /**
@@ -16,6 +17,8 @@
 abstract class Popover implements Component\Popover\Popover {
 
 	use ComponentHelper;
+	use JavaScriptBindable;
+
 	/**
 	 * @var string
 	 */
@@ -155,4 +158,4 @@ protected function initSignals() {
 		$this->show_signal = $this->signal_generator->create();
 		$this->replace_content_signal = $this->signal_generator->create("ILIAS\\UI\\Implementation\\Component\\Popover\\ReplaceContentSignal");
 	}
-}
+}

src/UI/Implementation/Component/Popover/Renderer.php

@@ -23,39 +23,49 @@ public function render(Component\Component $component, RendererInterface $defaul
 		$tpl = $this->getTemplate('tpl.popover.html', true, true);
 		$tpl->setVariable('FORCE_RENDERING', '');
 		/** @var Component\Popover\Popover $component */
+
 		$options = array(
 			'title'     => $this->escape($component->getTitle()),
 			'placement' => $component->getPosition(),
 			'multi'     => true,
 			'template'  => str_replace('"', '\"', $tpl->get()),
 		);
-		// Check if the content is rendered async or via DOM
-		$content_id = $this->createId();
-		if ($component->getAsyncContentUrl()) {
+
+		$is_async = $component->getAsyncContentUrl();
+		if ($is_async) {
 			$options['type'] = 'async';
 			$options['url'] = $component->getAsyncContentUrl();
-		} else {
-			$options['url'] = "#{$content_id}";
 		}
-		$options = json_encode($options);
+
 		$show = $component->getShowSignal();
-		$this->getJavascriptBinding()->addOnLoadCode("
-			$(document).on('{$show}', function(event, signalData) { 
-				il.UI.popover.showFromSignal(signalData, JSON.parse('{$options}'));
-			});");
 		$replace = $component->getReplaceContentSignal();
-		$this->getJavascriptBinding()->addOnLoadCode("
-			$(document).on('{$replace}', function(event, signalData) { 
-				il.UI.popover.replaceContentFromSignal('{$show}', signalData);
-			});");
+
+		$component = $component->withAdditionalOnLoadCode(function($id) use ($options, $show, $replace, $is_async) {
+			if (!$is_async) {
+				$options["url"] = "#{$id}";
+			}
+			$options = json_encode($options);
+
+			return
+				"$(document).on('{$show}', function(event, signalData) {
+					il.UI.popover.showFromSignal(signalData, JSON.parse('{$options}'));
+				});".
+				"$(document).on('{$replace}', function(event, signalData) {
+					il.UI.popover.replaceContentFromSignal('{$show}', signalData);
+				});";
+		});
+
+		$id = $this->bindJavaScript($component);
+
 		if ($component->getAsyncContentUrl()) {
 			return '';
 		}
+
 		if ($component instanceof Component\Popover\Standard) {
-			return $this->renderStandardPopover($component, $default_renderer, $content_id);
+			return $this->renderStandardPopover($component, $default_renderer, $id);
 		} else {
 			if ($component instanceof Component\Popover\Listing) {
-				return $this->renderListingPopover($component, $default_renderer, $content_id);
+				return $this->renderListingPopover($component, $default_renderer, $id);
 			}
 		}
 

src/UI/Implementation/Component/ViewControl/Renderer.php

@@ -105,11 +105,6 @@ protected function renderSectionButton(Component\Button\Button $component, $tpl,
 
 	protected function maybeRenderId(Component\Component $component, $tpl, $block, $template_var) {
 		$id = $this->bindJavaScript($component);
-		// Check if the component is acting as triggerer
-		if ($component instanceof Component\Triggerer && count($component->getTriggeredSignals())) {
-			$id = ($id === null) ? $this->createId() : $id;
-			$this->triggerRegisteredSignals($component, $id);
-		}
 		if ($id !== null) {
 			$tpl->setCurrentBlock($block);
 			$tpl->setVariable($template_var, $id);
@@ -128,4 +123,4 @@ protected function getComponentInterfaceName() {
 
 	}
 
-}
+}