fix(tags): tag text color contrast (#3653)

Ornanovitch · davwheat · SychO9 · web-flow · commit 4d292263b55e · 2023-01-17T20:45:03.000+01:00
* add yiq calculator util * use the new contast util to differentiate light/dark tags * fix: invert logic * feat: add tag-dark and tag-light less config * fix: convert 3 chars hex to 6 chars hex * fix: rename import * fix: clarify util name * fix: rename function * fix: invert less variables when dark mode is enabled * fix: TagTiles contrast * refactor: simplify logic with a unique variable * refactor: simplify logic with a unique variable * feat: add text color variables not depending on the dark/light mode * refactor: use isDark rather than getContrast * refactor: change getContrast to isDark with for a more direct approach * fix: adjust snippet description * refactor: change getContrast to isDark with for a more direct approach * fix: adjust snippet description * fix: TagHero contrast * fix: DiscussionHero contrast * fix: newDiscussion contrast * fix(newDiscussion): restore less rule when tag is not colored * fix: TagTiles description * fix: TagTiles last posted * chore: change `var` to `let` * refactor: keep it for backwards compatibility * refactor: keep it for backwards compatibility * Apply suggestions from code review * fix: missed this when I was resolving * fix: remove dist files from pull request * Revert "Resolved merge conflict" This reverts commit c7f0d14, reversing changes made to 6753dfc. * fix: missed this when I was resolving * fix * Update isDark.ts * chore: flexible contrast color fixing * refactor(isDark): clarify the doc block * fix(isDark): increase the yiq threshold * typo * fix: preserve design coloring through light and dark modes Co-authored-by: David Wheatley <david@davwheat.dev> Co-authored-by: Sami Mazouz <sychocouldy@gmail.com>
diff --git a/extensions/tags/js/src/common/helpers/tagLabel.js b/extensions/tags/js/src/common/helpers/tagLabel.js
@@ -1,5 +1,7 @@
 import extract from 'flarum/common/utils/extract';
 import Link from 'flarum/common/components/Link';
+import classList from 'flarum/common/utils/classList';
+import textContrastClass from 'flarum/common/helpers/textContrastClass';
 import tagIcon from './tagIcon';
 
 export default function tagLabel(tag, attrs = {}) {
@@ -13,7 +15,7 @@ export default function tagLabel(tag, attrs = {}) {
     const color = tag.color();
     if (color) {
       attrs.style['--tag-bg'] = color;
-      attrs.className += ' colored';
+      attrs.className = classList(attrs.className, 'colored', textContrastClass(color));
     }
 
     if (link) {
diff --git a/extensions/tags/js/src/forum/addTagFilter.tsx b/extensions/tags/js/src/forum/addTagFilter.tsx
@@ -5,6 +5,7 @@ import IndexPage from 'flarum/forum/components/IndexPage';
 import DiscussionListState from 'flarum/forum/states/DiscussionListState';
 import GlobalSearchState from 'flarum/forum/states/GlobalSearchState';
 import classList from 'flarum/common/utils/classList';
+import textContrastClass from 'flarum/common/helpers/textContrastClass';
 
 import TagHero from './components/TagHero';
 import Tag from '../common/models/Tag';
@@ -90,7 +91,7 @@ export default function () {
       const newDiscussion = items.get('newDiscussion') as Mithril.Vnode<ComponentAttrs, {}>;
 
       if (color) {
-        newDiscussion.attrs.className = classList([newDiscussion.attrs.className, 'Button--tagColored']);
+        newDiscussion.attrs.className = classList([newDiscussion.attrs.className, 'Button--tagColored', textContrastClass(color)]);
         newDiscussion.attrs.style = { '--color': color };
       }
 
diff --git a/extensions/tags/js/src/forum/addTagLabels.js b/extensions/tags/js/src/forum/addTagLabels.js
@@ -1,6 +1,8 @@
 import { extend } from 'flarum/common/extend';
 import DiscussionListItem from 'flarum/forum/components/DiscussionListItem';
 import DiscussionHero from 'flarum/forum/components/DiscussionHero';
+import textContrastClass from 'flarum/common/helpers/textContrastClass';
+import classList from 'flarum/common/utils/classList';
 
 import tagsLabel from '../common/helpers/tagsLabel';
 import sortTags from '../common/utils/sortTags';
@@ -23,7 +25,7 @@ export default function () {
       const color = tags[0].color();
       if (color) {
         view.attrs.style = { '--hero-bg': color };
-        view.attrs.className += ' DiscussionHero--colored';
+        view.attrs.className = classList(view.attrs.className, 'DiscussionHero--colored', textContrastClass(color));
       }
     }
   });
diff --git a/extensions/tags/js/src/forum/components/TagHero.js b/extensions/tags/js/src/forum/components/TagHero.js
@@ -1,13 +1,18 @@
 import Component from 'flarum/common/Component';
+import textContrastClass from 'flarum/common/helpers/textContrastClass';
 import tagIcon from '../../common/helpers/tagIcon';
+import classList from 'flarum/common/utils/classList';
 
 export default class TagHero extends Component {
   view() {
     const tag = this.attrs.model;
     const color = tag.color();
 
     return (
-      <header className={'Hero TagHero' + (color ? ' TagHero--colored' : '')} style={color ? { '--hero-bg': color } : ''}>
+      <header
+        className={classList('Hero', 'TagHero', { 'TagHero--colored': color }, textContrastClass(color))}
+        style={color ? { '--hero-bg': color } : ''}
+      >
         <div className="container">
           <div className="containerNarrow">
             <h2 className="Hero-title">
diff --git a/extensions/tags/js/src/forum/components/TagsPage.js b/extensions/tags/js/src/forum/components/TagsPage.js
@@ -4,6 +4,8 @@ import Link from 'flarum/common/components/Link';
 import LoadingIndicator from 'flarum/common/components/LoadingIndicator';
 import listItems from 'flarum/common/helpers/listItems';
 import humanTime from 'flarum/common/helpers/humanTime';
+import textContrastClass from 'flarum/common/helpers/textContrastClass';
+import classList from 'flarum/common/utils/classList';
 
 import tagIcon from '../../common/helpers/tagIcon';
 import tagLabel from '../../common/helpers/tagLabel';
@@ -58,7 +60,7 @@ export default class TagsPage extends Page {
                 const children = sortTags(tag.children() || []);
 
                 return (
-                  <li className={'TagTile ' + (tag.color() ? 'colored' : '')} style={{ '--tag-bg': tag.color() }}>
+                  <li className={classList('TagTile', { colored: tag.color() }, textContrastClass(tag.color()))} style={{ '--tag-bg': tag.color() }}>
                     <Link className="TagTile-info" href={app.route.tag(tag)}>
                       {tag.icon() && tagIcon(tag, {}, { useColor: false })}
                       <h3 className="TagTile-name">{tag.name()}</h3>
diff --git a/extensions/tags/less/common/TagLabel.less b/extensions/tags/less/common/TagLabel.less
@@ -16,7 +16,7 @@
   }
 
   &.colored {
-    --tag-color: @body-bg;
+    --tag-color: var(--contrast-color, var(--body-bg));
 
     .TagLabel-text {
       color: var(--tag-color) !important;
@@ -31,13 +31,13 @@
     &.colored {
       --tag-color: var(--tag-bg);
       margin-right: 5px;
-      background: @body-bg !important;
+      background-color: var(--contrast-color, var(--body-bg));
     }
   }
 }
 .DiscussionHero--colored {
   &, a {
-    color: @body-bg;
+    color: var(--contrast-color, var(--body-bg));
   }
 }
 .TagsLabel {
diff --git a/extensions/tags/less/forum.less b/extensions/tags/less/forum.less
@@ -9,6 +9,7 @@
   --button-primary-bg: var(--color);
   --button-primary-bg-hover: var(--color);
   --button-primary-bg-active: var(--color);
+  --button-color: var(--contrast-color);
 }
 .DiscussionHero {
   .item-title {
diff --git a/extensions/tags/less/forum/TagHero.less b/extensions/tags/less/forum/TagHero.less
@@ -1,5 +1,5 @@
 .TagHero {
   &--colored {
-    --hero-color: #fff;
+    --hero-color: var(--body-bg);
   }
 }
diff --git a/extensions/tags/less/forum/TagTiles.less b/extensions/tags/less/forum/TagTiles.less
@@ -64,7 +64,7 @@
   }
   &.colored {
     &, a {
-      color: @body-bg;
+      color: var(--contrast-color, var(--body-bg));
     }
   }
 }
@@ -100,10 +100,7 @@
 .TagTile-description {
   font-size: 12px;
   margin: 0 0 10px;
-
-  .TagTile.colored & {
-    color: fade(@body-bg, 70%);
-  }
+  opacity: 70%;
 }
 .TagTile-children {
   font-weight: bold;
@@ -124,10 +121,7 @@
   font-size: 12px;
   border-top: 1px solid rgba(0, 0, 0, 0.15);
   margin: 0 20px;
-
-  .TagTile.colored & {
-    color: fade(@body-bg, 70%);
-  }
+  opacity: 70%;
 
   &:hover .TagTile-lastPostedDiscussion-title {
     text-decoration: underline;
diff --git a/framework/core/js/src/common/compat.ts b/framework/core/js/src/common/compat.ts
@@ -81,6 +81,7 @@ import highlight from './helpers/highlight';
 import username from './helpers/username';
 import userOnline from './helpers/userOnline';
 import listItems from './helpers/listItems';
+import textContrastClass from './helpers/textContrastClass';
 import Fragment from './Fragment';
 import DefaultResolver from './resolvers/DefaultResolver';
 import PaginatedListState from './states/PaginatedListState';
@@ -175,6 +176,7 @@ export default {
   'helpers/username': username,
   'helpers/userOnline': userOnline,
   'helpers/listItems': listItems,
+  'helpers/textContrastClass': textContrastClass,
   'resolvers/DefaultResolver': DefaultResolver,
   'states/PaginatedListState': PaginatedListState,
   'states/AlertManagerState': AlertManagerState,
diff --git a/framework/core/js/src/common/helpers/textContrastClass.ts b/framework/core/js/src/common/helpers/textContrastClass.ts
@@ -0,0 +1,5 @@
+import isDark from '../utils/isDark';
+
+export default function textContrastClass(hexcolor: string): string {
+  return isDark(hexcolor) ? 'text-contrast--light' : 'text-contrast--dark';
+}
diff --git a/framework/core/js/src/common/utils/isDark.ts b/framework/core/js/src/common/utils/isDark.ts
@@ -1,11 +1,13 @@
 /**
- * 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.
+ * The `isDark` utility converts a hex color to rgb, and then calculates a YIQ
+ * value in order to get the appropriate brightness value. See
+ * https://www.w3.org/TR/AERT/#color-contrast for references.
+ *
+ * A YIQ value >= 128 corresponds to a light color according to the W3C
+ * standards, but we use a custom threshold for each light and dark modes
+ * to preserve design consistency.
  */
-
-export default function isDark(hexcolor: String) {
+export default function isDark(hexcolor: string): boolean {
   let hexnumbers = hexcolor.replace('#', '');
 
   if (hexnumbers.length == 3) {
@@ -17,5 +19,7 @@ export default function isDark(hexcolor: String) {
   const b = parseInt(hexnumbers.substr(4, 2), 16);
   const yiq = (r * 299 + g * 587 + b * 114) / 1000;
 
-  return yiq >= 128 ? false : true;
+  const threshold = parseInt(window.getComputedStyle(document.body).getPropertyValue('--yiq-threshold'));
+
+  return yiq < threshold;
 }
diff --git a/framework/core/less/common/root.less b/framework/core/less/common/root.less
@@ -30,6 +30,19 @@
 
   --error-color:                   @error-color;
 
+  // ---------------------------------
+  // UTILITIES
+
+  --text-on-dark:                  @text-on-dark;
+  --text-on-light:                 @text-on-light;
+
+  & when (@config-dark-mode = true) {
+    --yiq-threshold: 108;
+  }
+  & when (@config-dark-mode = false) {
+    --yiq-threshold: 138;
+  }
+
   // ---------------------------------
   // COMPONENTS
 
diff --git a/framework/core/less/common/scaffolding.less b/framework/core/less/common/scaffolding.less
@@ -159,3 +159,14 @@ blockquote ol:last-child {
   white-space: nowrap;
   width: 1px;
 }
+
+.text-contrast {
+  &--dark {
+    --contrast-color: var(--text-on-light);
+    color: var(--contrast-color);
+  }
+  &--light {
+    --contrast-color: var(--text-on-dark);
+    color: var(--contrast-color);
+  }
+}
diff --git a/framework/core/less/common/variables.less b/framework/core/less/common/variables.less
@@ -15,6 +15,9 @@
 @secondary-hue:                    hue(@secondary-color);
 @secondary-sat:                    saturation(@secondary-color);
 
+@body-bg-light:                    #fff;
+@body-bg-dark:                     hsl(@secondary-hue, min(20%, @secondary-sat), 10%);
+
 // Derive the primary/secondary colors from the config variables. In dark mode,
 // we make the user-set colors a bit darker, otherwise they will stand out too
 // much.
@@ -23,7 +26,7 @@
   @primary-color:                  @config-primary-color;
   @secondary-color:                @config-secondary-color;
 
-  @body-bg:                        #fff;
+  @body-bg:                        @body-bg-light;
   @text-color:                     #111;
   @link-color:                     saturate(@primary-color, 10%);
   @heading-color:                  @text-color;
@@ -45,7 +48,7 @@
   @primary-color:                  @config-primary-color;
   @secondary-color:                @config-secondary-color;
 
-  @body-bg:                        hsl(@secondary-hue, min(20%, @secondary-sat), 10%);
+  @body-bg:                        @body-bg-dark;
   @text-color:                     #ddd;
   @link-color:                     saturate(@primary-color, 10%);
   @heading-color:                  @text-color;
@@ -68,8 +71,8 @@
 // of the parents element's background. This allow not to change the color
 // variable depending on the dark/light mode to get the same final color, and
 // thus to simplify the logic.
-@text-on-dark:                     #ddd;
-@text-on-light:                    #111;
+@text-on-dark:                     @body-bg-light;
+@text-on-light:                    @body-bg-dark;
 
 @hero-bg:                          @control-bg;
 @hero-color:                       @control-color;
diff --git a/framework/core/less/forum/Hero.less b/framework/core/less/forum/Hero.less
@@ -2,7 +2,7 @@
   margin-top: -1px;
   background: var(--hero-bg);
   text-align: center;
-  color: var(--hero-color);
+  color: var(--contrast-color, var(--hero-color));
 
   h2 {
     margin: 0;

Original file line number	Diff line number	Diff line change
`@@ -16,7 +16,7 @@`
`16`	`16`	`}`
`17`	`17`
`18`	`18`	`&.colored {`
`19`		`- --tag-color: @body-bg;`
	`19`	`+ --tag-color: var(--contrast-color, var(--body-bg));`
`20`	`20`
`21`	`21`	`.TagLabel-text {`
`22`	`22`	`color: var(--tag-color) !important;`
`@@ -31,13 +31,13 @@`
`31`	`31`	`&.colored {`
`32`	`32`	`--tag-color: var(--tag-bg);`
`33`	`33`	`margin-right: 5px;`
`34`		`- background: @body-bg !important;`
	`34`	`+ background-color: var(--contrast-color, var(--body-bg));`
`35`	`35`	`}`
`36`	`36`	`}`
`37`	`37`	`}`
`38`	`38`	`.DiscussionHero--colored {`
`39`	`39`	`&, a {`
`40`		`- color: @body-bg;`
	`40`	`+ color: var(--contrast-color, var(--body-bg));`
`41`	`41`	`}`
`42`	`42`	`}`
`43`	`43`	`.TagsLabel {`
Original file line number	Diff line number	Diff line change
`@@ -9,6 +9,7 @@`
`9`	`9`	`--button-primary-bg: var(--color);`
`10`	`10`	`--button-primary-bg-hover: var(--color);`
`11`	`11`	`--button-primary-bg-active: var(--color);`
	`12`	`+ --button-color: var(--contrast-color);`
`12`	`13`	`}`
`13`	`14`	`.DiscussionHero {`
`14`	`15`	`.item-title {`
Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,5 @@`
`1`	`1`	`.TagHero {`
`2`	`2`	`&--colored {`
`3`		`- --hero-color: #fff;`
	`3`	`+ --hero-color: var(--body-bg);`
`4`	`4`	`}`
`5`	`5`	`}`