RotationConfigForm: Improve usablity

nilmerg · nilmerg · commit 091920f7006f · 2025-10-29T16:06:28.000+01:00
* Reorder elements
* Rename elements
* Change mode appearance
diff --git a/application/forms/RotationConfigForm.php b/application/forms/RotationConfigForm.php
@@ -18,6 +18,7 @@
 use Icinga\Web\Session;
 use ipl\Html\Attributes;
 use ipl\Html\DeferredText;
+use ipl\Html\FormDecoration\DescriptionDecorator;
 use ipl\Html\FormElement\FieldsetElement;
 use ipl\Html\HtmlDocument;
 use ipl\Html\HtmlElement;
@@ -602,7 +603,9 @@ protected function assembleModeSelection(): string
             '24-7' => $this->translate('24/7')
         ];
 
-        $modeList = new HtmlElement('ul');
+        $modeList = new HtmlElement('ul', Attributes::create([
+            'class' => ['rotation-mode', $this->disableModeSelection ? 'disabled' : '']
+        ]));
         foreach ($modes as $mode => $label) {
             $radio = $this->createElement('input', 'mode', [
                 'type' => 'radio',
@@ -684,8 +687,14 @@ protected function assembleModeSelection(): string
 
         $this->addHtml(new HtmlElement(
             'div',
-            Attributes::create(['class' => ['rotation-mode', $this->disableModeSelection ? 'disabled' : '']]),
-            new HtmlElement('h2', null, Text::create($this->translate('Mode'))),
+            Attributes::create([
+                'class' => ['control-group']
+            ]),
+            new HtmlElement(
+                'div',
+                Attributes::create(['class' => 'control-label-group']),
+                Text::create($this->translate('Rotation Mode'))
+            ),
             $modeList
         ));
 
@@ -704,12 +713,15 @@ protected function assembleTwentyFourSevenOptions(FieldsetElement $options): Dat
         $options->addElement('number', 'interval', [
             'required' => true,
             'label' => $this->translate('Handoff every'),
+            'description' => $this->translate('Have multiple rotation members take turns after this interval.'),
             'step' => 1,
             'min' => 1,
             'value' => 1,
             'validators' => [new GreaterThanValidator()]
         ]);
         $interval = $options->getElement('interval');
+        $interval->getDecorators()
+            ->replaceDecorator('Description', DescriptionDecorator::class, ['class' => 'description']);
 
         $frequency = $options->createElement('select', 'frequency', [
             'required' => true,
@@ -796,11 +808,15 @@ protected function assemblePartialDayOptions(FieldsetElement $options): DateTime
         $options->addElement('number', 'interval', [
             'required' => true,
             'label' => $this->translate('Handoff every'),
+            'description' => $this->translate('Have multiple rotation members take turns after this interval.'),
             'step' => 1,
             'min' => 1,
             'value' => 1,
             'validators' => [new GreaterThanValidator()]
         ]);
+        $interval = $options->getElement('interval');
+        $interval->getDecorators()
+            ->replaceDecorator('Description', DescriptionDecorator::class, ['class' => 'description']);
 
         $selectedFromTime = $from->getValue();
         foreach ($timeOptions as $key => $value) {
@@ -830,7 +846,6 @@ protected function assemblePartialDayOptions(FieldsetElement $options): DateTime
             )
         );
 
-        $interval = $options->getElement('interval');
         $interval->prependWrapper(
             (new HtmlDocument())->addHtml(
                 $interval,
@@ -912,8 +927,12 @@ protected function assembleMultiDayOptions(FieldsetElement $options): DateTime
             'step' => 1,
             'min' => 1,
             'value' => 1,
-            'label' => $this->translate('Handoff every')
+            'label' => $this->translate('Handoff every'),
+            'description' => $this->translate('Have multiple rotation members take turns after this interval.')
         ]);
+        $interval = $options->getElement('interval');
+        $interval->getDecorators()
+            ->replaceDecorator('Description', DescriptionDecorator::class, ['class' => 'description']);
 
         $timeOptions = $this->getTimeOptions();
         $fromAt = $options->createElement('select', 'from_at', [
@@ -988,7 +1007,6 @@ protected function assembleMultiDayOptions(FieldsetElement $options): DateTime
             )
         );
 
-        $interval = $options->getElement('interval');
         $interval->prependWrapper(
             (new HtmlDocument())->addHtml(
                 $interval,
@@ -1029,17 +1047,9 @@ protected function assemble()
 
         $this->addElement('hidden', 'priority', ['ignore' => true]);
 
-        $mode = $this->assembleModeSelection();
-
-        $autoSubmittedBy = $this->getRequest()->getHeader('X-Icinga-Autosubmittedby')[0] ?? '';
-        if ($autoSubmittedBy === 'mode') {
-            $this->clearPopulatedValue('options');
-            $this->clearPopulatedValue('first_handoff');
-        }
-
         $this->addElement('text', 'name', [
             'required' => true,
-            'label' => $this->translate('Title'),
+            'label' => $this->translate('Rotation Name'),
             'validators' => [
                 new CallbackValidator(function ($value, $validator) {
                     $rotations = Rotation::on($this->db)
@@ -1051,7 +1061,7 @@ protected function assemble()
                     }
 
                     if ($rotations->first() !== null) {
-                        $validator->addMessage($this->translate('A rotation with this title already exists'));
+                        $validator->addMessage($this->translate('A rotation with this name already exists'));
 
                         return false;
                     }
@@ -1061,6 +1071,73 @@ protected function assemble()
             ]
         ]);
 
+        $termValidator = function (array $terms) {
+            $contactTerms = [];
+            $groupTerms = [];
+            foreach ($terms as $term) {
+                /** @var TermInput\Term $term */
+                if (strpos($term->getSearchValue(), ':') === false) {
+                    // TODO: Auto-correct this to a valid type:id pair, if possible
+                    $term->setMessage($this->translate('Is not a contact nor a group of contacts'));
+                    continue;
+                }
+
+                list($type, $id) = explode(':', $term->getSearchValue(), 2);
+                if ($type === 'contact') {
+                    $contactTerms[$id] = $term;
+                } elseif ($type === 'group') {
+                    $groupTerms[$id] = $term;
+                }
+            }
+
+            if (! empty($contactTerms)) {
+                $contacts = (Contact::on(Database::get()))
+                    ->filter(Filter::equal('id', array_keys($contactTerms)));
+                foreach ($contacts as $contact) {
+                    $contactTerms[$contact->id]
+                        ->setLabel($contact->full_name)
+                        ->setClass('contact');
+                }
+            }
+
+            if (! empty($groupTerms)) {
+                $groups = (Contactgroup::on(Database::get()))
+                    ->filter(Filter::equal('id', array_keys($groupTerms)));
+                foreach ($groups as $group) {
+                    $groupTerms[$group->id]
+                        ->setLabel($group->name)
+                        ->setClass('group');
+                }
+            }
+        };
+
+        $members = (new TermInput('members'))
+            ->setIgnored()
+            ->setRequired()
+            ->setOrdered()
+            ->setReadOnly()
+            ->setVerticalTermDirection()
+            ->setLabel($this->translate('Rotation Members'))
+            ->setSuggestionUrl($this->suggestionUrl->with(['showCompact' => true, '_disableLayout' => 1]))
+            ->on(TermInput::ON_ENRICH, $termValidator)
+            ->on(TermInput::ON_ADD, $termValidator)
+            ->on(TermInput::ON_SAVE, $termValidator)
+            ->on(TermInput::ON_PASTE, $termValidator);
+        $this->addElement($members);
+
+        // TODO: TermInput is not compatible with the new decorators yet: https://github.com/Icinga/ipl-web/pull/317
+        $legacyDecorator = new IcingaFormDecorator();
+        $members->setDefaultElementDecorator($legacyDecorator);
+        $legacyDecorator->decorate($members);
+
+        $mode = $this->assembleModeSelection();
+
+        $autoSubmittedBy = $this->getRequest()->getHeader('X-Icinga-Autosubmittedby')[0] ?? '';
+        if ($autoSubmittedBy === 'mode') {
+            $this->clearPopulatedValue('options');
+            $this->clearPopulatedValue('first_handoff');
+        }
+
         $this->addElement('fieldset', 'options');
         /** @var FieldsetElement $options */
         $options = $this->getElement('options');
@@ -1098,7 +1175,7 @@ protected function assemble()
             'aria-describedby' => 'first-handoff-description',
             'min' => $earliestHandoff !== null ? $earliestHandoff->format('Y-m-d') : null,
             'max' => $latestHandoff->format('Y-m-d'),
-            'label' => $this->translate('First Handoff'),
+            'label' => $this->translate('Rotation Start'),
             'value' => $firstHandoffDefault,
             'validators' => [
                 new CallbackValidator(
@@ -1110,14 +1187,14 @@ function ($value, $validator) use ($earliestHandoff, $firstHandoff, $latestHando
                         );
                         if ($earliestHandoff !== null && $chosenHandoff < $earliestHandoff) {
                             $validator->addMessage(sprintf(
-                                $this->translate('The first handoff can only happen after %s'),
+                                $this->translate('The rotation can only start after %s'),
                                 $earliestHandoff->format('Y-m-d') // TODO: Use intl here
                             ));
 
                             return false;
                         } elseif ($chosenHandoff > $latestHandoff) {
                             $validator->addMessage(sprintf(
-                                $this->translate('The first handoff can only happen before %s'),
+                                $this->translate('The rotation can only start before %s'),
                                 $latestHandoff->format('Y-m-d') // TODO: Use intl here
                             ));
 
@@ -1142,10 +1219,10 @@ function ($value, $validator) use ($earliestHandoff, $firstHandoff, $latestHando
 
                     $actualFirstHandoff = $ruleGenerator->current()[0]->getStartDate();
                     if ($actualFirstHandoff < new DateTime()) {
-                        return $this->translate('The first handoff will happen immediately');
+                        return $this->translate('The rotation will start immediately');
                     } else {
                         return sprintf(
-                            $this->translate('The first handoff will happen on %s'),
+                            $this->translate('The rotation will start on %s'),
                             (new \IntlDateFormatter(
                                 \Locale::getDefault(),
                                 \IntlDateFormatter::MEDIUM,
@@ -1157,65 +1234,6 @@ function ($value, $validator) use ($earliestHandoff, $firstHandoff, $latestHando
             ));
         }
 
-        $termValidator = function (array $terms) {
-            $contactTerms = [];
-            $groupTerms = [];
-            foreach ($terms as $term) {
-                /** @var TermInput\Term $term */
-                if (strpos($term->getSearchValue(), ':') === false) {
-                    // TODO: Auto-correct this to a valid type:id pair, if possible
-                    $term->setMessage($this->translate('Is not a contact nor a group of contacts'));
-                    continue;
-                }
-
-                list($type, $id) = explode(':', $term->getSearchValue(), 2);
-                if ($type === 'contact') {
-                    $contactTerms[$id] = $term;
-                } elseif ($type === 'group') {
-                    $groupTerms[$id] = $term;
-                }
-            }
-
-            if (! empty($contactTerms)) {
-                $contacts = (Contact::on(Database::get()))
-                    ->filter(Filter::equal('id', array_keys($contactTerms)));
-                foreach ($contacts as $contact) {
-                    $contactTerms[$contact->id]
-                        ->setLabel($contact->full_name)
-                        ->setClass('contact');
-                }
-            }
-
-            if (! empty($groupTerms)) {
-                $groups = (Contactgroup::on(Database::get()))
-                    ->filter(Filter::equal('id', array_keys($groupTerms)));
-                foreach ($groups as $group) {
-                    $groupTerms[$group->id]
-                        ->setLabel($group->name)
-                        ->setClass('group');
-                }
-            }
-        };
-
-        $members = (new TermInput('members'))
-            ->setIgnored()
-            ->setRequired()
-            ->setOrdered()
-            ->setReadOnly()
-            ->setVerticalTermDirection()
-            ->setLabel($this->translate('Members'))
-            ->setSuggestionUrl($this->suggestionUrl->with(['showCompact' => true, '_disableLayout' => 1]))
-            ->on(TermInput::ON_ENRICH, $termValidator)
-            ->on(TermInput::ON_ADD, $termValidator)
-            ->on(TermInput::ON_SAVE, $termValidator)
-            ->on(TermInput::ON_PASTE, $termValidator);
-        $this->addElement($members);
-
-        // TODO: TermInput is not compatible with the new decorators yet: https://github.com/Icinga/ipl-web/pull/317
-        $legacyDecorator = new IcingaFormDecorator();
-        $members->setDefaultElementDecorator($legacyDecorator);
-        $legacyDecorator->decorate($members);
-
         $this->addElement('submit', 'submit', [
             'label' => $this->getSubmitLabel()
         ]);
@@ -1243,7 +1261,7 @@ function ($value, $validator) use ($earliestHandoff, $firstHandoff, $latestHando
             $this->getElement('submit')->prependWrapper((new HtmlDocument())->setHtmlContent(...$removeButtons));
         }
 
-        $this->addElement($this->createCsrfCounterMeasure(Session::getSession()->getId()));
+        $this->addCsrfCounterMeasure(Session::getSession()->getId());
     }
 
     /**
diff --git a/public/css/form.less b/public/css/form.less
@@ -33,50 +33,40 @@
 
 .rotation-config {
   .rotation-mode {
-    width: 50em;
-    padding: .5em 1em;
-    margin: 0 auto;
+    display: flex;
+    justify-content: space-between;
+    flex: 1 1 auto;
+    list-style: none;
+    margin: 0 1em 0 0;
+    padding: 0;
+
+    li {
+      flex: 1 1 auto;
+      width: 0;
 
-    h2 {
-      margin: 0;
+      &:not(:last-child) {
+        margin-right: 1em;
+      }
     }
 
-    ul {
+    label {
       display: flex;
-      justify-content: space-between;
+      flex-direction: column;
+      width: auto;
 
-      list-style: none;
-      margin: 0;
-      padding: 0;
-
-      li {
-        flex: 1 1 auto;
-        width: 0;
-
-        &:not(:last-child) {
-          margin-right: 1em;
-        }
+      input {
+        display: none;
       }
 
-      label {
-        display: flex;
-        flex-direction: column;
-        width: auto;
-
-        input {
-          display: none;
-        }
-
-        .mode-img {
-          width: 8em;
-          margin-bottom: .5em;
-          outline: 3px solid @icinga-blue;
-        }
+      .mode-img {
+        width: 8em;
+        margin-bottom: .5em;
+        outline: 3px solid @icinga-blue;
       }
     }
   }
 
-  .control-group {
+  .control-group:not(:has(.rotation-mode)) {
     align-items: baseline;
   }
 
@@ -133,6 +123,7 @@
   }
 
   .rotation-mode {
+    padding: .75em;
     border: 1px solid @gray-light;
     .rounded-corners();
 
