Render the visibility toggle as a plain link

The toggle carried an onclick that called toggleVisibility, which fetched
the url and swapped the icon by hand. Only the clicked icon though - rows
inheriting the value, as variants inherit "published" from their base
record, kept showing the old state until someone reloaded. Reproducing the
inheritance rules in the browser was never realistic.

The link now carries nothing but the scroll offset action. The server
flips the value and the list is rendered anew, so inherited states are
right without anyone tracking them client side. The handler redirects to
the list afterwards, which the toggle action does not render itself - of
no consequence while the answer was thrown away by an ajax call, but the
browser shows it now, and it keeps the url idempotent.

Two things surfaced by doing it:

The inline script that wires the drag and drop sorting declared "const
table" globally. It sits inside a loop, so it could already collide on a
page with several groups, and Turbo re-executes the scripts of a swapped
body - the toggle threw "Identifier 'table' has already been declared"
on every click. Scoped in a function expression now.

Three docblocks had piled up above buildToggleAttributes, two of them
stranded from an earlier extraction: buildCommand and buildDeepLinkTarget
stood without one while their text sat elsewhere. Handed back.

Verified: the variant case that started this is fixed - toggling the base
record updates all three inheriting rows without a reload, other groups
stay untouched, three runs in a row. Toggle 6/6 with no javascript errors
where there were two, drag and drop still persists, regression 19/19,
display conditions 5/5, legends 10/10, tree 5/5, back url 3/3, edit all
4/4, picker 6/6. phpcq unchanged.

Author: zonky2

src/Contao/View/Contao2BackendView/ActionHandler/ToggleHandler.php

@@ -26,6 +26,7 @@
 use ContaoCommunityAlliance\DcGeneral\Action;
 use ContaoCommunityAlliance\DcGeneral\Contao\DataDefinition\Definition\Contao2BackendViewDefinitionInterface;
 use ContaoCommunityAlliance\DcGeneral\Contao\RequestScopeDeterminator;
+use ContaoCommunityAlliance\DcGeneral\Contao\View\Contao2BackendView\ViewHelpers;
 use ContaoCommunityAlliance\DcGeneral\Contao\RequestScopeDeterminatorAwareTrait;
 use ContaoCommunityAlliance\DcGeneral\Data\ConfigInterface;
 use ContaoCommunityAlliance\DcGeneral\Data\DataProviderInterface;
@@ -105,7 +106,7 @@ public function handleEvent(ActionEvent $event)
      * @param ToggleCommandInterface $operation   The operation.
      * @param ModelIdInterface|null  $modelId     The model id.
      *
-     * @return void
+     * @return never This redirects to the list once the new state is stored.
      *
      * @SuppressWarnings(PHPMD.ExitExpression)
      * @SuppressWarnings(PHPMD.Superglobals)
@@ -167,6 +168,12 @@ protected function process(
             /** @var MultiLanguageDataProviderInterface $dataProvider */
             $dataProvider->setCurrentLanguage((string) $language);
         }
+
+        // Back to the list, which the action itself does not render. That was of no consequence while
+        // the toggle was an ajax call whose answer got thrown away, but it is a plain link now: the
+        // browser shows what comes back. Redirecting also keeps the url idempotent - reloading the
+        // list does not toggle a second time.
+        ViewHelpers::redirectHome($environment);
     }
 
     /**

src/Contao/View/Contao2BackendView/ButtonRenderer.php

@@ -24,10 +24,8 @@
 
 namespace ContaoCommunityAlliance\DcGeneral\Contao\View\Contao2BackendView;
 
-use Contao\Controller;
 use Contao\Image;
 use Contao\StringUtil;
-use Contao\System;
 use ContaoCommunityAlliance\Contao\Bindings\ContaoEvents;
 use ContaoCommunityAlliance\Contao\Bindings\Events\Backend\AddToUrlEvent;
 use ContaoCommunityAlliance\Contao\Bindings\Events\Image\GenerateHtmlEvent;
@@ -313,47 +311,19 @@ private function hasPasteNewButton(): bool
                 && ($basicDefinition->isEditable() && $basicDefinition->isCreatable()));
     }
 
-    /**
-     * Render a command button.
-     *
-     * @param CommandInterface    $command             The command to render the button for.
-     * @param ModelInterface      $model               The model to which the command shall get applied.
-     * @param ModelInterface|null $previous            The previous model in the collection.
-     * @param ModelInterface|null $next                The next model in the collection.
-     * @param bool                $isCircularReference Determinator if there exists a circular reference between the
-     *                                                 model and the model(s) contained in the clipboard.
-     * @param string[]            $childIds            The ids of all child models.
-     *
-     * @return string
-     *
-     * @SuppressWarnings(PHPMD.CyclomaticComplexity)
-     */
-    /**
-     * Build the target attribute of the "contao--deeplink" controller for a command.
-     *
-     * The controller opens a record on ctrl click or on a double tap. Contao derives its targets from a
-     * legacy "click2edit" class otherwise and warns about the deprecated helper behind it.
-     *
-     * @param CommandInterface $command The command to render.
-     *
-     * @return string The attribute including a leading space, or an empty string.
-     */
     /**
      * Build the attributes that turn a command into a visibility toggle.
      *
-     * @param array<array-key, mixed> $extra The extra information of the command.
-     * @param string                  $icon  The icon of the active state.
+     * A plain link, the way Contao renders its own toggle operation. The server flips the stored
+     * value and the list is rendered anew, so no script has to keep an icon in sync - and values
+     * inherited by other rows, as variants inherit "published", are correct without anyone
+     * reproducing the inheritance rules in the browser.
      *
      * @return string
      */
-    private function buildToggleAttributes(array $extra, string $icon): string
+    private function buildToggleAttributes(): string
     {
-        return sprintf(
-            ' data-action="contao--scroll-offset#store"'
-            . ' onclick="return BackendGeneral.toggleVisibility(this, \'%s\', \'%s\');"',
-            Controller::addStaticUrlTo(System::urlEncode($icon)),
-            Controller::addStaticUrlTo(System::urlEncode((string) ($extra['icon_disabled'] ?? 'invisible.svg')))
-        );
+        return ' data-action="contao--scroll-offset#store"';
     }
 
     /**
@@ -379,13 +349,38 @@ private function getToggleIcon(
         return (string) ($extra['icon_disabled'] ?? 'invisible.svg');
     }
 
+    /**
+     * Build the target attribute of the "contao--deeplink" controller for a command.
+     *
+     * The controller opens a record on ctrl click or on a double tap. Contao derives its targets from a
+     * legacy "click2edit" class otherwise and warns about the deprecated helper behind it.
+     *
+     * @param CommandInterface $command The command to render.
+     *
+     * @return string The attribute including a leading space, or an empty string.
+     */
     private function buildDeepLinkTarget(CommandInterface $command): string
     {
         $target = ['edit' => 'primary', 'children' => 'secondary'][$command->getName()] ?? null;
 
         return null === $target ? '' : ' data-contao--deeplink-target="' . $target . '"';
     }
 
+    /**
+     * Render a command button.
+     *
+     * @param CommandInterface    $command             The command to render the button for.
+     * @param ModelInterface      $model               The model to which the command shall get applied.
+     * @param ModelInterface|null $previous            The previous model in the collection.
+     * @param ModelInterface|null $next                The next model in the collection.
+     * @param bool                $isCircularReference Determinator if there exists a circular reference between the
+     *                                                 model and the model(s) contained in the clipboard.
+     * @param string[]            $childIds            The ids of all child models.
+     *
+     * @return string
+     *
+     * @SuppressWarnings(PHPMD.CyclomaticComplexity)
+     */
     private function buildCommand(
         CommandInterface $command,
         ModelInterface $model,
@@ -407,7 +402,7 @@ private function buildCommand(
         $attributes .= $this->buildDeepLinkTarget($command);
 
         if ($command instanceof ToggleCommandInterface) {
-            $attributes .= $this->buildToggleAttributes($extra, $icon);
+            $attributes .= $this->buildToggleAttributes();
             $icon         = $this->getToggleIcon($command, $model, $extra, $icon);
         }
 

src/Resources/contao/templates/dcbe_general_common_list.html5

@@ -129,9 +129,15 @@ assert($translator instanceof TranslatorInterface);
                         </tbody>
                     <?php if ($this->sortable) : ?>
                         <script>
-                            const table = document.getElementById('table_<?= $this->tableName ?>');
-                            const SortableTableDnD = new GeneralTableDnD();
-                            SortableTableDnD.init(table);
+                            /*
+                             * Scoped on purpose. This block sits inside a loop, so it can appear more than
+                             * once per page, and Turbo re-executes the scripts of a swapped body - both make
+                             * a global "const table" throw "Identifier has already been declared".
+                             */
+                            (function () {
+                                const table = document.getElementById('table_<?= $this->tableName ?>');
+                                new GeneralTableDnD().init(table);
+                            })();
                         </script>
                     <?php endif; ?>
                     <?php endforeach; ?>