diff --git a/src/UI/Component/JavaScriptBindable.php b/src/UI/Component/JavaScriptBindable.php index 635282ae949f..ce6ae61f8d1f 100644 --- a/src/UI/Component/JavaScriptBindable.php +++ b/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. * diff --git a/src/UI/Component/Triggerable.php b/src/UI/Component/Triggerable.php index 8265a5c9f4d8..c428b4863495 100644 --- a/src/UI/Component/Triggerable.php +++ b/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(); -} \ No newline at end of file +} diff --git a/src/UI/Component/Triggerer.php b/src/UI/Component/Triggerer.php index a53ca6fb93dd..74bae22a0c3b 100644 --- a/src/UI/Component/Triggerer.php +++ b/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(); - -} \ No newline at end of file +} diff --git a/src/UI/Implementation/Component/Button/Renderer.php b/src/UI/Implementation/Component/Button/Renderer.php index 933637a1fe4c..296fe9e1ebb8 100644 --- a/src/UI/Implementation/Component/Button/Renderer.php +++ b/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); diff --git a/src/UI/Implementation/Component/Dropdown/Renderer.php b/src/UI/Implementation/Component/Dropdown/Renderer.php index e3f2fdaa7fd1..9d3bfd3b7617 100644 --- a/src/UI/Implementation/Component/Dropdown/Renderer.php +++ b/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); diff --git a/src/UI/Implementation/Component/Glyph/Renderer.php b/src/UI/Implementation/Component/Glyph/Renderer.php index 3699debbe75a..f35e8e0731b8 100644 --- a/src/UI/Implementation/Component/Glyph/Renderer.php +++ b/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"); diff --git a/src/UI/Implementation/Component/JavaScriptBindable.php b/src/UI/Implementation/Component/JavaScriptBindable.php index dd27554f6073..bc38e7cdaa5c 100644 --- a/src/UI/Implementation/Component/JavaScriptBindable.php +++ b/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 */ diff --git a/src/UI/Implementation/Component/Modal/Renderer.php b/src/UI/Implementation/Component/Modal/Renderer.php index 75177a0a8faa..eec4525f7d62 100644 --- a/src/UI/Implementation/Component/Modal/Renderer.php +++ b/src/UI/Implementation/Component/Modal/Renderer.php @@ -47,18 +47,34 @@ 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; });"; + }); } /** @@ -66,8 +82,8 @@ protected function registerSignals(Component\Modal\Modal $modal, $id) { * @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 ""; } @@ -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); diff --git a/src/UI/Implementation/Component/Popover/Popover.php b/src/UI/Implementation/Component/Popover/Popover.php index 65afd8fbfebc..49a0d508baae 100644 --- a/src/UI/Implementation/Component/Popover/Popover.php +++ b/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"); } -} \ No newline at end of file +} diff --git a/src/UI/Implementation/Component/Popover/Renderer.php b/src/UI/Implementation/Component/Popover/Renderer.php index 6890168b77e6..45eba78b57fe 100755 --- a/src/UI/Implementation/Component/Popover/Renderer.php +++ b/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); } } diff --git a/src/UI/Implementation/Component/ViewControl/Renderer.php b/src/UI/Implementation/Component/ViewControl/Renderer.php index 56df1ae55c77..21ca614c23d9 100644 --- a/src/UI/Implementation/Component/ViewControl/Renderer.php +++ b/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() { } -} \ No newline at end of file +} diff --git a/src/UI/Implementation/Render/AbstractComponentRenderer.php b/src/UI/Implementation/Render/AbstractComponentRenderer.php index b44a0b8964d5..8bf714a2fe7b 100644 --- a/src/UI/Implementation/Render/AbstractComponentRenderer.php +++ b/src/UI/Implementation/Render/AbstractComponentRenderer.php @@ -78,13 +78,6 @@ final public function toJS($key) { $this->lng->toJS($key); } - /** - * @return JavaScriptBinding - */ - final protected function getJavascriptBinding() { - return $this->js_binding; - } - /** * Get template of component this renderer is made for. * @@ -113,11 +106,24 @@ final protected function getTemplate($name, $purge_unfilled_vars, $purge_unused_ * @return string|null */ final protected function bindJavaScript(JavaScriptBindable $component) { - $binder = $component->getOnLoadCode(); + if ($component instanceof Triggerer) { + $component = $this->addTriggererOnLoadCode($component); + } + return $this->bindOnloadCode($component); + } + + /** + * Bind the JavaScript onload-code. + * + * @param JavaScriptBindable $component + * @return string|null + */ + private function bindOnloadCode(JavaScriptBindable $component) { + $binder = $component->getOnLoadCode(); if ($binder === null) { return null; } - $id = $this->createId(); + $id = $this->js_binding->createId(); $on_load_code = $binder($id); if (!is_string($on_load_code)) { throw new \LogicException( @@ -125,43 +131,40 @@ final protected function bindJavaScript(JavaScriptBindable $component) { " (used component: ".get_class($component).")"); } $this->js_binding->addOnLoadCode($on_load_code); - return $id; - } - - - /** - * Create an ID - * - * @return string - */ - final protected function createId() { - $id = $this->js_binding->createId(); return $id; } /** - * Renderers of components acting as triggerer can use this method to trigger the registered signals + * Add onload-code for triggerer. * - * @param Triggerer $triggerer - * @param string $id The generated ID for the triggerer component + * @param Triggerer $triggerer + * @return Triggerer */ - protected function triggerRegisteredSignals(Triggerer $triggerer, $id) { - foreach ($triggerer->getTriggeredSignals() as $triggered_signal) { - $signal = $triggered_signal->getSignal(); - $event = $triggered_signal->getEvent(); - $options = json_encode($signal->getOptions()); - $this->js_binding->addOnLoadCode( - "$('#{$id}').{$event}( function(event) { - $(this).trigger('{$signal}', - { - 'id' : '{$signal}', 'event' : '{$event}', - 'triggerer' : $(this), - 'options' : JSON.parse('{$options}') - } - ); - return false; - });"); + private function addTriggererOnLoadCode(Triggerer $triggerer) { + $triggered_signals = $triggerer->getTriggeredSignals(); + if (count($triggered_signals) == 0) { + return $triggerer; } + return $triggerer->withAdditionalOnLoadCode(function($id) use ($triggered_signals) { + $code = ""; + foreach ($triggered_signals as $triggered_signal) { + $signal = $triggered_signal->getSignal(); + $event = $triggered_signal->getEvent(); + $options = json_encode($signal->getOptions()); + $code .= + "$('#{$id}').{$event}( function(event) { + $(this).trigger('{$signal}', + { + 'id' : '{$signal}', 'event' : '{$event}', + 'triggerer' : $(this), + 'options' : JSON.parse('{$options}') + } + ); + return false; + });"; + } + return $code; + }); } /** diff --git a/tests/UI/Component/JavaScriptBindableTest.php b/tests/UI/Component/JavaScriptBindableTest.php index adaab3d6d9d4..583cadcea6d9 100644 --- a/tests/UI/Component/JavaScriptBindableTest.php +++ b/tests/UI/Component/JavaScriptBindableTest.php @@ -45,4 +45,29 @@ public function test_withOnLoadCode_false_closure_2() { $this->assertTrue(true); } } + + public function test_withAdditionalOnLoadCode() { + $m = $this->mock + ->withOnLoadCode(function($id) { + return "Its me, $id!"; + }) + ->withAdditionalOnLoadCode(function($id) { + return "And again, me: $id."; + }); + + $binder = $m->getOnLoadCode(); + $this->assertInstanceOf(\Closure::class, $binder); + $this->assertEquals("Its me, Mario!\nAnd again, me: Mario.", $binder("Mario")); + } + + public function test_withAdditionalOnLoadCode_no_previous() { + $m = $this->mock + ->withAdditionalOnLoadCode(function($id) { + return "And again, me: $id."; + }); + + $binder = $m->getOnLoadCode(); + $this->assertInstanceOf(\Closure::class, $binder); + $this->assertEquals("And again, me: Mario.", $binder("Mario")); + } } diff --git a/tests/UI/Component/Modal/LightboxTest.php b/tests/UI/Component/Modal/LightboxTest.php index 88f7c15ff87d..398912ad02f6 100644 --- a/tests/UI/Component/Modal/LightboxTest.php +++ b/tests/UI/Component/Modal/LightboxTest.php @@ -45,7 +45,7 @@ protected function getExpectedHTML() {

title

-