chore: improve group mentions parsing (#3723)

* chore: improve group mentions color handling * Avoid storing the `GroupMention--dark` and light classes on the database. * Avoid recreating YIQ logic on the backend. * Improve text color flexibility through CSS variables. * Apply fixes from StyleCI * chore: tweak tests * chre: unused import Signed-off-by: Sami Mazouz <[email protected]>
flarum · Jan 22, 2023 · 364472a · 364472a
1 parent b9ca434
commit 364472a
Show file tree

Hide file tree

Showing 5 changed files with 45 additions and 69 deletions.
diff --git a/js/src/forum/index.js b/js/src/forum/index.js
@@ -2,6 +2,8 @@ import { extend } from 'flarum/common/extend';
 import app from 'flarum/forum/app';
 import NotificationGrid from 'flarum/forum/components/NotificationGrid';
 import { getPlainContent } from 'flarum/common/utils/string';
+import textContrastClass from 'flarum/common/helpers/textContrastClass';
+import Post from 'flarum/forum/components/Post';
 
 import addPostMentionPreviews from './addPostMentionPreviews';
 import addMentionedByList from './addMentionedByList';
@@ -84,6 +86,13 @@ app.initializers.add('flarum-mentions', function () {
 
   // Remove post mentions when rendering post previews.
   getPlainContent.removeSelectors.push('a.PostMention');
+
+  // Apply color contrast fix on group mentions.
+  extend(Post.prototype, 'oncreate', function () {
+    this.$('.GroupMention--colored').each(function () {
+      this.classList.add(textContrastClass(getComputedStyle(this).getPropertyValue('--group-color')));
+    });
+  });
 });
 
 export * from './utils/textFormatter';

diff --git a/js/src/forum/utils/textFormatter.js b/js/src/forum/utils/textFormatter.js
@@ -1,7 +1,6 @@
 import app from 'flarum/forum/app';
 import username from 'flarum/common/helpers/username';
 import extractText from 'flarum/common/utils/extractText';
-import isDark from 'flarum/common/utils/isDark';
 
 export function filterUserMentions(tag) {
   let user;
@@ -41,7 +40,6 @@ export function filterGroupMentions(tag) {
       tag.setAttribute('groupname', extractText(group.namePlural()));
       tag.setAttribute('icon', group.icon());
       tag.setAttribute('color', group.color());
-      tag.setAttribute('class', isDark(group.color()) ? 'GroupMention--light' : 'GroupMention--dark');
 
       return true;
     }

diff --git a/less/forum.less b/less/forum.less
@@ -1,17 +1,17 @@
 .PostMention, .UserMention, .GroupMention {
-  background: @control-bg;
-  color: @control-color;
+  background: var(--control-bg);
+  color: var(--control-color);
   border-radius: @border-radius;
   padding: 2px 5px;
   border: 0 !important;
   font-weight: 600;
 
   blockquote & {
-    background: @body-bg;
+    background: var(--body-bg);
   }
   &:hover,
   &:active {
-    color: @link-color;
+    color: var(--link-color);
   }
 }
 .UserMention, .PostMention, .GroupMention {
@@ -98,35 +98,13 @@
   .Button--color(@tooltip-color, @tooltip-bg);
 }
 .GroupMention {
-  & when (@config-dark-mode = false) {
-    &,
-    &:hover,
-    &:active {
-      color: @text-on-light;
-    }
-  }
-  & when (@config-dark-mode = true) {
-    &,
-    &:hover,
-    &:active {
-      color: @text-on-dark;
-    }
-  }
+  background-color: var(--group-color, var(--control-bg));
+  color: var(--control-color);
+  --link-color: currentColor;
 
-  &--light {
-    &,
-    &:hover,
-    &:active {
-      color: @text-on-light;
-    }
-  }
-
-  &--dark {
-    &,
-    &:hover,
-    &:active {
-      color: @text-on-dark;
-    }
+  &--colored {
+    --control-color: var(--contrast-color, var(--body-bg));
+    --link-color: var(--control-color);
   }
 
   .icon {

diff --git a/src/ConfigureMentions.php b/src/ConfigureMentions.php
@@ -14,7 +14,6 @@
 use Flarum\Post\PostRepository;
 use Flarum\Settings\SettingsRepositoryInterface;
 use Flarum\User\User;
-use Illuminate\Support\Str;
 use s9e\TextFormatter\Configurator;
 use s9e\TextFormatter\Parser\Tag;
 
@@ -151,16 +150,37 @@ private function configureGroupMentions(Configurator $config)
         $tag->attributes->add('groupname');
         $tag->attributes->add('icon');
         $tag->attributes->add('color');
-        $tag->attributes->add('class');
         $tag->attributes->add('id')->filterChain->append('#uint');
 
         $tag->template = '
             <xsl:choose>
                 <xsl:when test="@deleted != 1">
-                    <span class="GroupMention {@class}" style="background: {@color}">@<xsl:value-of select="@groupname"/><i class="icon {@icon}"></i></span>
+                    <xsl:choose>
+                        <xsl:when test="string(@color) != \'\'">
+                            <span class="GroupMention GroupMention--colored" style="--group-color:{@color};">
+                                <span class="GroupMention-name">@<xsl:value-of select="@groupname"/></span>
+                                <xsl:if test="string(@icon) != \'\'">
+                                    <i class="icon {@icon}"></i>
+                                </xsl:if>
+                            </span>
+                        </xsl:when>
+                        <xsl:otherwise>
+                            <span class="GroupMention">
+                                <span class="GroupMention-name">@<xsl:value-of select="@groupname"/></span>
+                                <xsl:if test="string(@icon) != \'\'">
+                                    <i class="icon {@icon}"></i>
+                                </xsl:if>
+                            </span>
+                        </xsl:otherwise>
+                    </xsl:choose>
                 </xsl:when>
                 <xsl:otherwise>
-                    <span class="GroupMention GroupMention--deleted" style="background: {@color}">@<xsl:value-of select="@groupname"/><i class="icon {@icon}"></i></span>
+                    <span class="GroupMention GroupMention--deleted">
+                        <span class="GroupMention-name">@<xsl:value-of select="@groupname"/></span>
+                        <xsl:if test="string(@icon) != \'\'">
+                            <i class="icon {@icon}"></i>
+                        </xsl:if>
+                    </span>
                 </xsl:otherwise>
             </xsl:choose>';
         $tag->filterChain->prepend([static::class, 'addGroupId'])
@@ -182,40 +202,10 @@ public static function addGroupId($tag)
             $tag->setAttribute('groupname', $group->name_plural);
             $tag->setAttribute('icon', $group->icon ?? 'fas fa-at');
             $tag->setAttribute('color', $group->color);
-            if (! empty($group->color)) {
-                $tag->setAttribute('class', self::isDark($group->color) ? 'GroupMention--light' : 'GroupMention--dark');
-            } else {
-                $tag->setAttribute('class', '');
-            }
 
             return true;
         }
 
         $tag->invalidate();
     }
-
-    /**
-     * The `isDark` utility converts a hex color to rgb, and then calcul a YIQ
-     * value in order to get the appropriate brightness value (is it dark or is it
-     * light?) See https://www.w3.org/TR/AERT/#color-contrast for references. A YIQ
-     * value >= 128 is a light color.
-     */
-    public static function isDark(?string $hexColor): bool
-    {
-        if (! $hexColor) {
-            return false;
-        }
-
-        $hexNumbers = Str::replace('#', '', $hexColor);
-        if (Str::length($hexNumbers) === 3) {
-            $hexNumbers .= $hexNumbers;
-        }
-
-        $r = hexdec(Str::substr($hexNumbers, 0, 2));
-        $g = hexdec(Str::subStr($hexNumbers, 2, 2));
-        $b = hexdec(Str::subStr($hexNumbers, 4, 2));
-        $yiq = ($r * 299 + $g * 587 + $b * 114) / 1000;
-
-        return $yiq >= 128 ? false : true;
-    }
 }
diff --git a/tests/integration/api/GroupMentionsTest.php b/tests/integration/api/GroupMentionsTest.php
@@ -84,7 +84,8 @@ public function rendering_a_valid_group_mention_works()
 
         $response = json_decode($response->getBody(), true);
 
-        $this->assertStringContainsString('<p>One of the <span style="background:#80349E" class="GroupMention ">@Mods<i class="icon fas fa-bolt"></i></span> will look at this</p>', $response['data']['attributes']['contentHtml']);
+        $this->assertStringContainsString('GroupMention', $response['data']['attributes']['contentHtml']);
+        $this->assertStringContainsString('#80349E', $response['data']['attributes']['contentHtml']);
         $this->assertNotNull(CommentPost::find($response['data']['id'])->mentionsGroups->find(4));
     }