From 4d292263b55e521f2239f79ac3bc2d64fa43ecdc Mon Sep 17 00:00:00 2001 From: ornanovitch <34077099+Ornanovitch@users.noreply.github.com> Date: Tue, 17 Jan 2023 20:45:03 +0100 Subject: [PATCH] fix(tags): tag text color contrast (#3653) * 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 c7f0d14aa8f2a18a5a0ae7530284e51d7aedd811, reversing changes made to 6753dfc2af6a125f503724aed129bfd98fa06df5. * 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 Co-authored-by: Sami Mazouz --- .../tags/js/src/common/helpers/tagLabel.js | 4 +++- extensions/tags/js/src/forum/addTagFilter.tsx | 3 ++- extensions/tags/js/src/forum/addTagLabels.js | 4 +++- .../tags/js/src/forum/components/TagHero.js | 7 ++++++- .../tags/js/src/forum/components/TagsPage.js | 4 +++- extensions/tags/less/common/TagLabel.less | 6 +++--- extensions/tags/less/forum.less | 1 + extensions/tags/less/forum/TagHero.less | 2 +- extensions/tags/less/forum/TagTiles.less | 12 +++--------- framework/core/js/src/common/compat.ts | 2 ++ .../js/src/common/helpers/textContrastClass.ts | 5 +++++ framework/core/js/src/common/utils/isDark.ts | 18 +++++++++++------- framework/core/less/common/root.less | 13 +++++++++++++ framework/core/less/common/scaffolding.less | 11 +++++++++++ framework/core/less/common/variables.less | 11 +++++++---- framework/core/less/forum/Hero.less | 2 +- 16 files changed, 75 insertions(+), 30 deletions(-) create mode 100644 framework/core/js/src/common/helpers/textContrastClass.ts diff --git a/extensions/tags/js/src/common/helpers/tagLabel.js b/extensions/tags/js/src/common/helpers/tagLabel.js index 1af28d732..eb520c068 100644 --- 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 index c7a23e7c1..e0972da3d 100644 --- 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; 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 index 12e2245ba..4f2ec89e2 100644 --- 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 index 1b24884f3..77b0721d5 100644 --- a/extensions/tags/js/src/forum/components/TagHero.js +++ b/extensions/tags/js/src/forum/components/TagHero.js @@ -1,5 +1,7 @@ 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() { @@ -7,7 +9,10 @@ export default class TagHero extends Component { const color = tag.color(); return ( -
+

diff --git a/extensions/tags/js/src/forum/components/TagsPage.js b/extensions/tags/js/src/forum/components/TagsPage.js index 66b6ab70b..690f71970 100755 --- 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 ( -
  • +
  • {tag.icon() && tagIcon(tag, {}, { useColor: false })}

    {tag.name()}

    diff --git a/extensions/tags/less/common/TagLabel.less b/extensions/tags/less/common/TagLabel.less index 8568925fe..79e9e2ec1 100644 --- 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 index 15b2ba32d..1356a8242 100644 --- 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 index 8d9a3f593..6a04658d7 100644 --- 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 index 9075cc270..8f03e6e79 100755 --- 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 index 5eb9e0ffb..ff9224a7e 100644 --- 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 new file mode 100644 index 000000000..19fd1004c --- /dev/null +++ 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 index 3b4563ee7..eb49aa85d 100644 --- 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 index 1b0e6988a..16d98438e 100644 --- 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 index e0cafb51f..6979f76bd 100644 --- 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 index dd4b8ca2a..c381877b7 100644 --- 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 index 9f39aa1a0..d335f019d 100644 --- 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;