From 2204157ac04daaa09fe9a15bcac48d9e21c32d9c Mon Sep 17 00:00:00 2001
From: Sami Mazouz <sychocouldy@gmail.com>
Date: Thu, 19 Sep 2024 22:35:20 +0100
Subject: [PATCH] fix(a11y): convert empty links to buttons (#3926)

---
 extensions/tags/js/src/forum/addTagComposer.js    |  8 ++++++--
 extensions/tags/less/forum.less                   |  1 +
 .../src/forum/components/DiscussionListItem.tsx   | 12 +++++++++---
 .../core/js/src/forum/components/LogInModal.tsx   | 10 ++++++++--
 .../core/js/src/forum/components/PostMeta.js      |  5 +++--
 .../js/src/forum/components/PostStreamScrubber.js | 13 +++++++------
 .../core/js/src/forum/components/SignUpModal.tsx  |  6 +++++-
 framework/core/less/common/Button.less            |  7 ++++++-
 framework/core/less/common/mixins.less            |  1 +
 framework/core/less/common/mixins/typography.less |  9 +++++++++
 framework/core/less/common/scaffolding.less       |  8 +-------
 framework/core/less/forum/Post.less               |  2 +-
 framework/core/less/forum/Scrubber.less           | 15 +++------------
 13 files changed, 60 insertions(+), 37 deletions(-)
 create mode 100644 framework/core/less/common/mixins/typography.less

diff --git a/extensions/tags/js/src/forum/addTagComposer.js b/extensions/tags/js/src/forum/addTagComposer.js
index 7cffe7ce1..a55cbdd30 100644
--- a/extensions/tags/js/src/forum/addTagComposer.js
+++ b/extensions/tags/js/src/forum/addTagComposer.js
@@ -2,6 +2,7 @@ import app from 'flarum/forum/app';
 import { extend, override } from 'flarum/common/extend';
 import IndexSidebar from 'flarum/forum/components/IndexSidebar';
 import classList from 'flarum/common/utils/classList';
+import Button from 'flarum/common/components/Button';
 
 import tagsLabel from '../common/helpers/tagsLabel';
 import getSelectableTags from './utils/getSelectableTags';
@@ -47,13 +48,16 @@ export default function addTagComposer() {
 
     items.add(
       'tags',
-      <a className={classList(['DiscussionComposer-changeTags', !selectableTags.length && 'disabled'])} onclick={this.chooseTags.bind(this)}>
+      <Button
+        className={classList(['DiscussionComposer-changeTags', 'Button Button--ua-reset', !selectableTags.length && 'disabled'])}
+        onclick={this.chooseTags.bind(this)}
+      >
         {tags.length ? (
           tagsLabel(tags)
         ) : (
           <span className="TagLabel untagged">{app.translator.trans('flarum-tags.forum.composer_discussion.choose_tags_link')}</span>
         )}
-      </a>,
+      </Button>,
       10
     );
   });
diff --git a/extensions/tags/less/forum.less b/extensions/tags/less/forum.less
index bca726db6..15798ee4b 100644
--- a/extensions/tags/less/forum.less
+++ b/extensions/tags/less/forum.less
@@ -32,6 +32,7 @@
 }
 .DiscussionComposer-changeTags {
   margin-right: 15px;
+  line-height: inherit;
 
   &.disabled {
     opacity: 0.5;
diff --git a/framework/core/js/src/forum/components/DiscussionListItem.tsx b/framework/core/js/src/forum/components/DiscussionListItem.tsx
index e71d5597d..d0c04855b 100644
--- a/framework/core/js/src/forum/components/DiscussionListItem.tsx
+++ b/framework/core/js/src/forum/components/DiscussionListItem.tsx
@@ -153,9 +153,15 @@ export default class DiscussionListItem<CustomAttrs extends IDiscussionListItemA
         text={app.translator.trans('core.forum.discussion_list.started_text', { user, ago: humanTime(discussion.createdAt()) })}
         position="right"
       >
-        <Link className="DiscussionListItem-author-avatar" href={user ? app.route.user(user) : '#'}>
-          <Avatar user={user || null} title="" />
-        </Link>
+        {user ? (
+          <Link className="DiscussionListItem-author-avatar" href={app.route.user(user)}>
+            <Avatar user={user} title="" />
+          </Link>
+        ) : (
+          <span className="DiscussionListItem-author-avatar">
+            <Avatar user={null} title="" />
+          </span>
+        )}
       </Tooltip>,
       100
     );
diff --git a/framework/core/js/src/forum/components/LogInModal.tsx b/framework/core/js/src/forum/components/LogInModal.tsx
index 71747c4f5..00157a4e0 100644
--- a/framework/core/js/src/forum/components/LogInModal.tsx
+++ b/framework/core/js/src/forum/components/LogInModal.tsx
@@ -123,10 +123,16 @@ export default class LogInModal<CustomAttrs extends ILoginModalAttrs = ILoginMod
     return (
       <>
         <p className="LogInModal-forgotPassword">
-          <a onclick={this.forgotPassword.bind(this)}>{app.translator.trans('core.forum.log_in.forgot_password_link')}</a>
+          <Button className="Button Button--text Button--link" onclick={this.forgotPassword.bind(this)}>
+            {app.translator.trans('core.forum.log_in.forgot_password_link')}
+          </Button>
         </p>
         {app.forum.attribute<boolean>('allowSignUp') && (
-          <p className="LogInModal-signUp">{app.translator.trans('core.forum.log_in.sign_up_text', { a: <a onclick={this.signUp.bind(this)} /> })}</p>
+          <p className="LogInModal-signUp">
+            {app.translator.trans('core.forum.log_in.sign_up_text', {
+              a: <Button className="Button Button--text Button--link" onclick={this.signUp.bind(this)} />,
+            })}
+          </p>
         )}
       </>
     );
diff --git a/framework/core/js/src/forum/components/PostMeta.js b/framework/core/js/src/forum/components/PostMeta.js
index 78573db5b..892f54fcd 100644
--- a/framework/core/js/src/forum/components/PostMeta.js
+++ b/framework/core/js/src/forum/components/PostMeta.js
@@ -2,6 +2,7 @@ import app from '../../forum/app';
 import Component from '../../common/Component';
 import humanTime from '../../common/helpers/humanTime';
 import fullTime from '../../common/helpers/fullTime';
+import Button from '../../common/components/Button';
 
 /**
  * The `PostMeta` component displays the time of a post, and when clicked, shows
@@ -29,9 +30,9 @@ export default class PostMeta extends Component {
 
     return (
       <div className="Dropdown PostMeta">
-        <a className="Dropdown-toggle" onclick={selectPermalink} data-toggle="dropdown">
+        <Button className="Dropdown-toggle Button Button--text Button--link" onclick={selectPermalink} data-toggle="dropdown">
           {humanTime(time)}
-        </a>
+        </Button>
 
         <div className="Dropdown-menu dropdown-menu">
           <span className="PostMeta-number">{app.translator.trans('core.forum.post.number_tooltip', { number: post.number() })}</span>{' '}
diff --git a/framework/core/js/src/forum/components/PostStreamScrubber.js b/framework/core/js/src/forum/components/PostStreamScrubber.js
index 6880bc9ab..a16949786 100644
--- a/framework/core/js/src/forum/components/PostStreamScrubber.js
+++ b/framework/core/js/src/forum/components/PostStreamScrubber.js
@@ -3,6 +3,7 @@ import Component from '../../common/Component';
 import formatNumber from '../../common/utils/formatNumber';
 import ScrollListener from '../../common/utils/ScrollListener';
 import Icon from '../../common/components/Icon';
+import Button from '../../common/components/Button';
 
 /**
  * The `PostStreamScrubber` component displays a scrubber which can be used to
@@ -63,9 +64,9 @@ export default class PostStreamScrubber extends Component {
 
         <div className="Dropdown-menu dropdown-menu">
           <div className="Scrubber">
-            <a className="Scrubber-first" onclick={this.goToFirst.bind(this)}>
-              <Icon name={'fas fa-angle-double-up'} /> {app.translator.trans('core.forum.post_scrubber.original_post_link')}
-            </a>
+            <Button className="Scrubber-first Button Button--link" onclick={this.goToFirst.bind(this)} icon="fas fa-angle-double-up">
+              {app.translator.trans('core.forum.post_scrubber.original_post_link')}
+            </Button>
 
             <div className="Scrubber-scrollbar">
               <div className="Scrubber-before" />
@@ -83,9 +84,9 @@ export default class PostStreamScrubber extends Component {
               </div>
             </div>
 
-            <a className="Scrubber-last" onclick={this.goToLast.bind(this)}>
-              <Icon name={'fas fa-angle-double-down'} /> {app.translator.trans('core.forum.post_scrubber.now_link')}
-            </a>
+            <Button className="Scrubber-last Button Button--link" onclick={this.goToLast.bind(this)} icon="fas fa-angle-double-down">
+              {app.translator.trans('core.forum.post_scrubber.now_link')}
+            </Button>
           </div>
         </div>
       </div>
diff --git a/framework/core/js/src/forum/components/SignUpModal.tsx b/framework/core/js/src/forum/components/SignUpModal.tsx
index f989e1e15..e34da6bbe 100644
--- a/framework/core/js/src/forum/components/SignUpModal.tsx
+++ b/framework/core/js/src/forum/components/SignUpModal.tsx
@@ -137,7 +137,11 @@ export default class SignUpModal<CustomAttrs extends ISignupModalAttrs = ISignup
 
   footer() {
     return [
-      <p className="SignUpModal-logIn">{app.translator.trans('core.forum.sign_up.log_in_text', { a: <a onclick={this.logIn.bind(this)} /> })}</p>,
+      <p className="SignUpModal-logIn">
+        {app.translator.trans('core.forum.sign_up.log_in_text', {
+          a: <Button className="Button Button--text Button--link" onclick={this.logIn.bind(this)} />,
+        })}
+      </p>,
     ];
   }
 
diff --git a/framework/core/less/common/Button.less b/framework/core/less/common/Button.less
index eb92c9fa4..dca50dc78 100644
--- a/framework/core/less/common/Button.less
+++ b/framework/core/less/common/Button.less
@@ -163,8 +163,9 @@
 .Button--text {
   background: transparent !important;
   padding: 0;
-  color: inherit !important;
+  --button-color: inherit;
   line-height: inherit;
+  vertical-align: baseline;
 
   &:hover {
     text-decoration: underline;
@@ -175,6 +176,10 @@
     box-shadow: none;
   }
 }
+.Button--link.Button--text {
+  .text-link();
+  --button-color: currentColor;
+}
 .Button--primary {
   .Button--color-auto('button-primary');
   font-weight: bold;
diff --git a/framework/core/less/common/mixins.less b/framework/core/less/common/mixins.less
index 7a778a82e..b23bea675 100644
--- a/framework/core/less/common/mixins.less
+++ b/framework/core/less/common/mixins.less
@@ -5,3 +5,4 @@
 @import "mixins/light-contents.less";
 @import "mixins/header-background.less";
 @import "mixins/vendor-prefixes.less";
+@import "mixins/typography.less";
diff --git a/framework/core/less/common/mixins/typography.less b/framework/core/less/common/mixins/typography.less
new file mode 100644
index 000000000..85f86e583
--- /dev/null
+++ b/framework/core/less/common/mixins/typography.less
@@ -0,0 +1,9 @@
+.text-link() {
+  cursor: pointer;
+  color: var(--link-color);
+  text-decoration: none;
+
+  &:hover {
+    text-decoration: underline;
+  }
+}
diff --git a/framework/core/less/common/scaffolding.less b/framework/core/less/common/scaffolding.less
index c1e2c05bd..2270925ee 100644
--- a/framework/core/less/common/scaffolding.less
+++ b/framework/core/less/common/scaffolding.less
@@ -37,13 +37,7 @@ textarea {
 }
 
 a {
-  cursor: pointer;
-  color: var(--link-color);
-  text-decoration: none;
-
-  &:hover {
-    text-decoration: underline;
-  }
+  .text-link();
 }
 
 hr {
diff --git a/framework/core/less/forum/Post.less b/framework/core/less/forum/Post.less
index 488db7918..f317b3830 100644
--- a/framework/core/less/forum/Post.less
+++ b/framework/core/less/forum/Post.less
@@ -18,7 +18,7 @@
 .Post-header {
   margin-bottom: 15px;
 
-  &, a {
+  &, a, .Button--text.Button--link {
     color: var(--muted-color);
   }
   > ul {
diff --git a/framework/core/less/forum/Scrubber.less b/framework/core/less/forum/Scrubber.less
index 450229eed..53e00e34b 100644
--- a/framework/core/less/forum/Scrubber.less
+++ b/framework/core/less/forum/Scrubber.less
@@ -1,16 +1,7 @@
 .Scrubber {
-  & a {
-    margin-left: -1px;
-    color: var(--muted-color);
-
-    & .fa {
-      font-size: 14px;
-      margin-right: 2px;
-    }
-    &:hover, &:focus {
-      text-decoration: none;
-      color: var(--link-color);
-    }
+  &-first, &-last {
+    width: auto !important;
+    padding: 0;
   }
 }
 .Scrubber-scrollbar {