From a6f88903ee3d2b0e86a6aeecd2274587d2f264b9 Mon Sep 17 00:00:00 2001
From: Jan-Hendrik Willms <tleilax+studip@gmail.com>
Date: Tue, 28 Nov 2023 09:07:03 +0000
Subject: prevent tabbing to disabled action menu links and in institute basic
 data, fixes #3010, fixes #3007

Closes #3010 and #3007

Merge request studip/studip!2039
---
 app/controllers/institute/basicdata.php           |   2 +-
 app/views/institute/basicdata/index.php           |   4 +-
 lib/classes/ActionMenu.php                        |   7 +-
 lib/classes/LinkButton.class.php                  |   7 ++
 resources/assets/stylesheets/scss/actionmenu.scss |  28 +++---
 resources/vue/components/StudipActionMenu.vue     | 108 ++++++++++++----------
 resources/vue/mixins/MyCoursesMixin.js            |   2 +-
 templates/shared/action-menu-single.php           |  18 +++-
 templates/shared/action-menu.php                  |  25 ++++-
 9 files changed, 121 insertions(+), 80 deletions(-)

diff --git a/app/controllers/institute/basicdata.php b/app/controllers/institute/basicdata.php
index 030749f..fb1ce96 100644
--- a/app/controllers/institute/basicdata.php
+++ b/app/controllers/institute/basicdata.php
@@ -76,7 +76,7 @@ class Institute_BasicdataController extends AuthenticatedController
             $this->question = (string) QuestionBox::create(
                 $message,
                 $this->url_for('institute/basicdata/delete/' . $i_view, $post),
-                $this->url_for('institute/basicdata/delete/' . $i_view, [])
+                $this->url_for('institute/basicdata/index/' . $i_view)
             );
         }
 
diff --git a/app/views/institute/basicdata/index.php b/app/views/institute/basicdata/index.php
index 5d29572..abb0487 100644
--- a/app/views/institute/basicdata/index.php
+++ b/app/views/institute/basicdata/index.php
@@ -131,8 +131,8 @@
         <input type="hidden" name="i_id" value="<?= $institute->id ?>">
         <?= Studip\Button::createAccept(_('Übernehmen'), 'i_edit') ?>
         <?= Studip\LinkButton::create(_('Löschen'),
-                                      $controller->url_for('institute/basicdata/index/' . $i_view, ['i_trykill' => 1]),
-                                      !$may_delete ? ['disabled' => ''] : []) ?>
+            $controller->url_for('institute/basicdata/index/' . $i_view, ['i_trykill' => 1]),
+            !$may_delete ? ['disabled' => ''] : []) ?>
         <? if (!$may_delete && mb_strlen($reason_txt) > 0): ?>
             <?= tooltipIcon($reason_txt, true) ?>
         <? endif; ?>
diff --git a/lib/classes/ActionMenu.php b/lib/classes/ActionMenu.php
index 613bea5..0822f72 100644
--- a/lib/classes/ActionMenu.php
+++ b/lib/classes/ActionMenu.php
@@ -143,6 +143,7 @@ class ActionMenu
                     'attributes' => $attributes,
                 ];
             }
+
             $index = $index ?: md5($action['link'] . json_encode($action['attributes']));
             $action['index'] = $index;
             //now insert it possibly at the desired position:
@@ -326,10 +327,10 @@ class ActionMenu
                        : self::TEMPLATE_FILE_MULTIPLE;
 ;
         $template = $GLOBALS['template_factory']->open($template_file);
-        $template->actions = array_map(function ($action) {
-            $disabled = isset($action['attributes']['disabled'])
+        $template->actions = array_map(function ($action): array {
+            $action['disabled'] = isset($action['attributes']['disabled'])
                      && $action['attributes']['disabled'] !== false;
-            if ($disabled && $action['icon']) {
+            if ($action['disabled'] && $action['icon']) {
                 $action['icon'] = $action['icon']->copyWithRole(Icon::ROLE_INACTIVE);
             }
             return $action;
diff --git a/lib/classes/LinkButton.class.php b/lib/classes/LinkButton.class.php
index 848d2e6..40044b6 100644
--- a/lib/classes/LinkButton.class.php
+++ b/lib/classes/LinkButton.class.php
@@ -35,6 +35,13 @@ class LinkButton extends Interactable
      */
     public function __toString()
     {
+        if (
+            isset($this->attributes['disabled'])
+            && $this->attributes['disabled'] !== false
+        ) {
+            return (string) Button::create($this->label, 'none', $this->attributes);
+        }
+
         // add "button" to attribute @class
         if (empty($this->attributes['class'])) {
             $this->attributes['class'] = '';
diff --git a/resources/assets/stylesheets/scss/actionmenu.scss b/resources/assets/stylesheets/scss/actionmenu.scss
index a25beed..dfc1fd1 100644
--- a/resources/assets/stylesheets/scss/actionmenu.scss
+++ b/resources/assets/stylesheets/scss/actionmenu.scss
@@ -102,21 +102,7 @@ $action-menu-shadow: 1px 1px 1px var(--dark-gray-color-60);
             display: block;
         }
 
-        &.action-menu-item-disabled {
-            > a,
-            > label {
-                &,
-                &:hover {
-                    color: var(--dark-gray-color-80);
-                    cursor: default;
-                }
-            }
-        }
-
-        a img,
-        a svg,
-        .action-menu-no-icon,
-        input[type="image"] {
+        .action-menu-item-icon {
             display: inline-block;
             margin: 0 0.25em;
             vertical-align: middle;
@@ -147,6 +133,18 @@ $action-menu-shadow: 1px 1px 1px var(--dark-gray-color-60);
             border-top: thin solid var(--dark-gray-color-45);
             margin: 4px 0;
         }
+
+        &.action-menu-item-disabled {
+            > label,
+            > button {
+                color: var(--dark-gray-color-80);
+                cursor: default;
+
+                &:hover {
+                    color: var(--dark-gray-color-80);
+                }
+            }
+        }
     }
 
     &.is-open {
diff --git a/resources/vue/components/StudipActionMenu.vue b/resources/vue/components/StudipActionMenu.vue
index 434336f..918814c 100644
--- a/resources/vue/components/StudipActionMenu.vue
+++ b/resources/vue/components/StudipActionMenu.vue
@@ -10,28 +10,40 @@
                 {{ title }}
             </div>
             <ul class="action-menu-list">
-                <li v-for="item in navigationItems" :key="item.id" class="action-menu-item">
-                    <hr v-if="item.type === 'separator'">
-                    <a v-else-if="item.type === 'link'" v-bind="linkAttributes(item)" v-on="linkEvents(item)">
-                        <studip-icon v-if="item.icon !== false" :shape="item.icon.shape" :role="item.icon.role"></studip-icon>
+                <li v-for="item in navigationItems" :key="item.id"
+                    class="action-menu-item"
+                    :class="{'action-menu-item-disabled': item.disabled}"
+                >
+                    <label v-if="item.disabled" aria-disabled="true" v-bind="item.attributes">
+                        <studip-icon v-if="item.icon"
+                                     :shape="item.icon"
+                                     role="inactive"
+                                     class="action-menu-item-icon"
+                        />
                         <span v-else class="action-menu-no-icon"></span>
-                        {{ item.label }}
-                    </a>
-                    <label v-else-if="item.type === 'checkbox'" class="undecorated" v-on="linkEvents(item)">
-                        <studip-icon :shape="item.checked ? 'checkbox-checked' : 'checkbox-unchecked'" :role="item.icon.role" :name="item.name" :title="item.label" aria-role="checkbox" :aria-checked="item.checked.toString()" v-bind="linkAttributes(item)"></studip-icon>
+
                         {{ item.label }}
                     </label>
-                    <label v-else-if="item.type === 'radio'" class="undecorated" v-on="linkEvents(item)">
-                        <studip-icon :shape="item.checked ? 'radiobutton-checked' : 'radiobutton-unchecked'" :role="item.icon.role" :name="item.name" :title="item.label" aria-role="radio" :aria-checked="item.checked.toString()" v-bind="linkAttributes(item)"></studip-icon>
+                    <hr v-else-if="item.type === 'separator'">
+                    <a v-else-if="item.type === 'link'" v-bind="item.attributes" v-on="linkEvents(item)">
+                        <studip-icon v-if="item.icon"
+                                     :shape="item.icon"
+                                     class="action-menu-item-icon"
+                        />
+                        <span v-else class="action-menu-no-icon"></span>
                         {{ item.label }}
-                    </label>
+                    </a>
                     <label v-else-if="item.icon" class="undecorated" v-on="linkEvents(item)">
-                        <studip-icon :shape="item.icon.shape" :role="item.icon.role" :name="item.name" :title="item.label" v-bind="linkAttributes(item)"></studip-icon>
+                        <studip-icon :shape="item.icon"
+                                     :name="item.name"
+                                     class="action-menu-item-icon"
+                                     v-bind="item.attributes"
+                        />
                         {{ item.label }}
                     </label>
                     <template v-else>
                         <span class="action-menu-no-icon"></span>
-                        <button :name="item.name" v-bind="linkAttributes(item)" v-on="linkEvents(item)">
+                        <button :name="item.name" v-bind="item.attributes" v-on="linkEvents(item)">
                             {{ item.label }}
                         </button>
                     </template>
@@ -40,10 +52,22 @@
         </div>
     </div>
     <div v-else>
-        <a v-for="item in navigationItems" :key="item.id" v-bind="linkAttributes(item)" v-on="linkEvents(item)">
-            <span v-if="item.type === 'separator'" class="quiet">|</span>
-            <studip-icon v-else :title="item.label" :shape="item.icon.shape" :role="item.icon.role" :size="20"></studip-icon>
-        </a>
+        <template v-for="item in navigationItems">
+            <label v-if="item.disabled" :key="item.id" aria-disabled="true" v-bind="item.attributes">
+                <studip-icon :shape="item.icon"
+                             :title="item.label"
+                             role="inactive"
+                             class="action-menu-item-icon"
+                />
+            </label>
+            <span v-else-if="item.type === 'separator'" :key="item.id" class="quiet">|</span>
+            <a v-else :key="item.id" v-bind="item.attributes" v-on="linkEvents(item)">
+                <studip-icon :shape="item.icon"
+                             :title="item.label"
+                             class="action-menu-item-icon"
+                ></studip-icon>
+            </a>
+        </template>
     </div>
 </template>
 
@@ -72,26 +96,12 @@ export default {
         };
     },
     methods: {
-        linkAttributes (item) {
-            let attributes = item.attributes;
-            attributes.class = item.classes;
-
-            if (item.disabled) {
-                attributes.disabled = true;
-            }
-
-            if (item.url) {
-                attributes.href = item.url;
-            }
-
-            return attributes;
-        },
         linkEvents (item) {
             let events = {};
             if (item.emit) {
                 events.click = (e) => {
                     e.preventDefault();
-                    this.$emit.apply(this, [item.emit].concat(item.emitArguments));
+                    this.$emit.apply(this, [item.emit].concat(item.emitArguments ?? []));
                     this.close();
                 };
             }
@@ -104,26 +114,22 @@ export default {
     computed: {
         navigationItems () {
             return this.items.map((item) => {
-                let classes = item.classes ?? '';
-                if (item.disabled) {
-                    classes += " action-menu-item-disabled";
+                item.type = item.type ?? 'link';
+                item.attributes = item.attributes ?? {};
+
+                if (item.type === 'link') {
+                    item.attributes.href = item.url ?? '#';
+                } else if (item.type === 'checkbox') {
+                    item.attributes['aria-role'] = item.type;
+                    item.attributes['aria-checked'] = item.checked.toString();
+                    item.icon = item.checked ? 'checkbox-checked' : 'checkbox-unchecked';
+                } else if (item.type === 'radio') {
+                    item.attributes['aria-role'] = item.type;
+                    item.attributes['aria-checked'] = item.checked.toString();
+                    item.icon = item.checked ? 'radiobutton-checked' : 'radiobutton-unchecked';
                 }
-                return {
-                    label: item.label,
-                    url: item.url || '#',
-                    emit: item.emit || false,
-                    emitArguments: item.emitArguments || [],
-                    icon: item.icon ? {
-                        shape: item.icon,
-                        role: item.disabled ? 'inactive' : 'clickable'
-                    } : false,
-                    type: item.type || 'link',
-                    name: item.name ?? null,
-                    classes: classes.trim(),
-                    attributes: item.attributes || {},
-                    disabled: item.disabled,
-                    checked: item.checked,
-                };
+
+                return item;
             });
         },
         shouldCollapse () {
diff --git a/resources/vue/mixins/MyCoursesMixin.js b/resources/vue/mixins/MyCoursesMixin.js
index 39e11e7..dcf73a6 100644
--- a/resources/vue/mixins/MyCoursesMixin.js
+++ b/resources/vue/mixins/MyCoursesMixin.js
@@ -77,7 +77,7 @@ export default {
                     label: this.$gettext('Veranstaltungsdetails'),
                     icon: 'info-circle',
                     attributes: {
-                        'data-dialog': ''
+                        'data-dialog': '',
                     },
                 });
             }
diff --git a/templates/shared/action-menu-single.php b/templates/shared/action-menu-single.php
index 455bc44..dbe38c1 100644
--- a/templates/shared/action-menu-single.php
+++ b/templates/shared/action-menu-single.php
@@ -12,17 +12,29 @@
  */
 ?>
 <? foreach ($actions as $action): ?>
-    <? if ($action['type'] === 'link'): ?>
+    <? if ($action['disabled']): ?>
+        <label class="undecorated action-menu-item-disabled" aria-disabled="true" <?= arrayToHtmlAttributes($action['attributes'] + ['title' => $action['label']]) ?>>
+        <? if ($action['icon']): ?>
+            <?= $action['icon']->asImg(['class' => 'action-menu-item-icon']) ?>
+        <? else: ?>
+            <?= htmlReady($action['label']) ?>
+        <? endif ?>
+        </label>
+    <? elseif ($action['type'] === 'link'): ?>
         <a href="<?= htmlReady($action['link']) ?>" <?= arrayToHtmlAttributes($action['attributes'] + ['title' => $action['label']]) ?>>
             <? if ($action['icon']): ?>
-                <?= $action['icon']->asImg() ?>
+                <?= $action['icon']->asImg(['class' => 'action-menu-item-icon']) ?>
             <? else: ?>
                 <?= htmlReady($action['label']) ?>
             <? endif ?>
         </a>
     <? elseif ($action['type'] === 'button'): ?>
         <? if ($action['icon']): ?>
-            <?= $action['icon']->asInput($action['attributes'] + ['name' => $action['name'], 'title' => $action['label']]) ?>
+            <?= $action['icon']->asInput($action['attributes'] + [
+                'class' => 'action-menu-item-icon',
+                'name'  => $action['name'],
+                'title' => $action['label'],
+            ]) ?>
         <? else: ?>
             <button name="<?= htmlReady($action['name']) ?>" <?= arrayToHtmlAttributes($action['attributes']) ?>>
                 <?= htmlReady($action['label']) ?>
diff --git a/templates/shared/action-menu.php b/templates/shared/action-menu.php
index bc2cc50..e95b92d 100644
--- a/templates/shared/action-menu.php
+++ b/templates/shared/action-menu.php
@@ -9,6 +9,9 @@
  *     icon: Icon,
  *     attributes: array
  * }> $actions
+ * @var string $title
+ * @var string $action_menu_title
+ * @var array $attributes
  */
 ?>
 <? // class "action-menu" will be set from API ?>
@@ -24,11 +27,21 @@
         </div>
         <ul class="action-menu-list" aria-label="<?= htmlReady($title) ?>">
         <? foreach ($actions as $action): ?>
-            <li class="action-menu-item <? if (isset($action['attributes']['disabled'])) echo 'action-menu-item-disabled'; ?>">
-            <? if ($action['type'] === 'link'): ?>
+            <li class="action-menu-item <? if ($action['disabled']) echo 'action-menu-item-disabled'; ?>">
+            <? if ($action['disabled']): ?>
+                <label class="undecorated" aria-disabled="true" <?= arrayToHtmlAttributes($action['attributes']) ?>>
+                    <? if ($action['icon']): ?>
+                        <?= $action['icon']->asImg(false, ['class' => 'action-menu-item-icon']) ?>
+                    <? else: ?>
+                        <span class="action-menu-no-icon"></span>
+                    <? endif ?>
+
+                    <?= htmlReady($action['label']) ?>
+                </label>
+            <? elseif ($action['type'] === 'link'): ?>
                 <a href="<?= htmlReady($action['link']) ?>" <?= arrayToHtmlAttributes($action['attributes']) ?>>
                     <? if ($action['icon']): ?>
-                        <?= $action['icon']->asImg(false) ?>
+                        <?= $action['icon']->asImg(false, ['class' => 'action-menu-item-icon']) ?>
                     <? else: ?>
                         <span class="action-menu-no-icon"></span>
                     <? endif ?>
@@ -37,7 +50,11 @@
             <? elseif ($action['type'] === 'button'): ?>
                 <? if ($action['icon']): ?>
                     <label class="undecorated">
-                        <?= $action['icon']->asInput(false, $action['attributes'] + ['name' => $action['name'], 'title' => $action['label']]) ?>
+                        <?= $action['icon']->asInput(false, $action['attributes'] + [
+                            'class' => 'action-menu-item-icon',
+                            'name'  => $action['name'],
+                            'title' => $action['label'],
+                        ]) ?>
                         <?= htmlReady($action['label']) ?>
                     </label>
                 <? else: ?>
-- 
cgit v1.0