Skip to content

Commit

Permalink
fix: pickReadableTextColor a11y concerns (#970)
Browse files Browse the repository at this point in the history
* fix: pickReadableTextColor a11y concerns

Removed pickReadableTextColor and replaced it with getTextColor.

Closes D2IQ-97209

* refactor: move getCSSVarValue to shared utils

getCSSVarValue is now exported and able to be used as util in other apps.
  • Loading branch information
nataliepina authored May 12, 2023
1 parent f6a6dce commit 3da0b7f
Show file tree
Hide file tree
Showing 19 changed files with 74 additions and 91 deletions.
2 changes: 1 addition & 1 deletion packages/accordion/style.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
borderRadiusDefault
} from "../design-tokens/build/js/designTokens";
import { lighten } from "../shared/styles/color";
import { getCSSVarValue } from "../utilities";
import { getCSSVarValue } from "../shared/styles/styleUtils/typography/color";

export const fillWidth = css`
display: block;
Expand Down
9 changes: 5 additions & 4 deletions packages/appChrome/components/SidebarSubMenuItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,18 @@ import { textSize, flex, tintContent } from "../../shared/styles/styleUtils";
import Clickable from "../../clickable/components/clickable";
import ResetLink from "../../link/components/ResetLink";
import { useTheme } from "@emotion/react";
import getCSSVarValue from "../../utilities/getCSSVarValue";

import {
themeTextColorPrimary,
themeTextColorPrimaryInverted,
themeTextColorSecondaryInverted,
themeBgPrimaryInverted,
themeBgSelected
} from "../../design-tokens/build/js/designTokens";
import { pickReadableTextColor } from "../../shared/styles/color";
import { getTextColor } from "../../shared/styles/color";
import { AppChromeTheme, SidebarNavItemProps } from "../types";
import { IconSize } from "../../shared/types/iconSize";
import { getCSSVarValue } from "../../shared/styles/styleUtils/typography/color";

export interface SidebarSubMenuItemTextProps {
menuHasIcon: boolean;
Expand Down Expand Up @@ -64,7 +65,7 @@ const SidebarSubMenuItem = ({
appChromeInsetContent(theme?.sidebarItemPaddingHor),
sidebarNavItem(Boolean(isActive), Boolean(disabled), theme),
tintContent(
pickReadableTextColor(
getTextColor(
sidebarBgColor,
getCSSVarValue(themeTextColorPrimary),
getCSSVarValue(themeTextColorSecondaryInverted)
Expand All @@ -75,7 +76,7 @@ const SidebarSubMenuItem = ({
subMenuItem,
{
[tintContent(
pickReadableTextColor(
getTextColor(
subMenuItemBgColor,
getCSSVarValue(themeTextColorPrimary),
getCSSVarValue(themeTextColorPrimaryInverted)
Expand Down
2 changes: 1 addition & 1 deletion packages/appChrome/style.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
} from "../design-tokens/build/js/designTokens";
import { padding, tintContent } from "../shared/styles/styleUtils";
import { pickHoverBg, getTextColor } from "../shared/styles/color";
import getCSSVarValue from "../utilities/getCSSVarValue";
import { getCSSVarValue } from "../shared/styles/styleUtils/typography/color";
import { AppChromeTheme } from "./types";
import {
spaceSizes,
Expand Down
6 changes: 3 additions & 3 deletions packages/badge/style.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,16 @@ import {
themeTextColorPrimaryInverted,
themeBorder
} from "../design-tokens/build/js/designTokens";
import { pickReadableTextColor } from "../shared/styles/color";
import getCSSVarValue from "../utilities/getCSSVarValue";
import { getTextColor } from "../shared/styles/color";
import { getCSSVarValue } from "../shared/styles/styleUtils/typography/color";

const badgeAppearanceStyle = (color, isOutlined?: boolean) => {
const bgColor = isOutlined ? getCSSVarValue(themeBgPrimary) : color;

return css`
background-color: ${bgColor};
border-color: ${color};
color: ${pickReadableTextColor(
color: ${getTextColor(
bgColor,
getCSSVarValue(themeTextColorPrimary),
getCSSVarValue(themeTextColorPrimaryInverted)
Expand Down
2 changes: 1 addition & 1 deletion packages/badge/tests/__snapshots__/badge.test.tsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ exports[`Badge primary 1`] = `
exports[`Badge success 1`] = `
<DocumentFragment>
<span
class="css-1nxr7gm"
class="css-1qzm5de"
data-cy="badge"
>
success
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ exports[`BadgeButton primary 1`] = `
exports[`BadgeButton success 1`] = `
<DocumentFragment>
<span
class="css-92fng0"
class="css-uu7e6j"
data-cy="badgeButton"
role="button"
tabindex="-1"
Expand Down
6 changes: 3 additions & 3 deletions packages/button/style.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@ import {
themeWarningInverted,
themeWarning
} from "../design-tokens/build/js/designTokens";
import { darken, pickReadableTextColor } from "../shared/styles/color";
import { darken, getTextColor } from "../shared/styles/color";
import { ButtonAppearances } from "./components/ButtonBase";
import { tintContent } from "../shared/styles/styleUtils";
import getCSSVarValue from "../utilities/getCSSVarValue";
import { getCSSVarValue } from "../shared/styles/styleUtils/typography/color";

export const buttonPadding = {
vert: "10px",
Expand All @@ -41,7 +41,7 @@ export const filledButton = (
) => {
const contentColor = isInverse
? themeTextColorPrimaryInverted
: pickReadableTextColor(
: getTextColor(
baseColor,
getCSSVarValue(themeTextColorPrimary),
getCSSVarValue(themeTextColorPrimaryInverted)
Expand Down
8 changes: 4 additions & 4 deletions packages/button/tests/__snapshots__/ButtonBase.test.tsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -922,8 +922,8 @@ exports[`ButtonBase renders all appearances with props 5`] = `
overflow: visible;
padding: 0;
text-align: inherit;
color: #FFFFFF;
fill: #FFFFFF;
color: #1B2029;
fill: #1B2029;
background-color: #14C684;
border-radius: 4px;
padding: 10px 18px;
Expand All @@ -949,8 +949,8 @@ exports[`ButtonBase renders all appearances with props 5`] = `
.emotion-1[href],
.emotion-1[href]:visited {
color: #FFFFFF;
fill: #FFFFFF;
color: #1B2029;
fill: #1B2029;
}
.emotion-1:hover {
Expand Down
6 changes: 4 additions & 2 deletions packages/donutChart/components/DonutChart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@ import {
themeBorder,
themeBrandPrimary
} from "../../design-tokens/build/js/designTokens";
import { tintContentSecondary } from "../../shared/styles/styleUtils/typography/color";
import {
getCSSVarValue,
tintContentSecondary
} from "../../shared/styles/styleUtils/typography/color";
import { hexToRgbA } from "../../shared/styles/color";
import getCSSVarValue from "../../utilities/getCSSVarValue";

interface DonutChartDatum {
percentage: number;
Expand Down
2 changes: 1 addition & 1 deletion packages/infobox/style.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
themeWarning
} from "../design-tokens/build/js/designTokens";
import { lighten, darken } from "../shared/styles/color";
import getCSSVarValue from "../utilities/getCSSVarValue";
import { getCSSVarValue } from "../shared/styles/styleUtils/typography/color";

const infoBoxAppearances = appearance => {
switch (appearance) {
Expand Down
6 changes: 3 additions & 3 deletions packages/popover/components/PopoverListItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ import {
themeError,
themeTextColorDisabled
} from "../../design-tokens/build/js/designTokens";
import getCSSVarValue from "../../utilities/getCSSVarValue";
import { darken, pickReadableTextColor } from "../../shared/styles/color";
import { getCSSVarValue } from "../../shared/styles/styleUtils/typography/color";
import { darken, getTextColor } from "../../shared/styles/color";
import { PopoverListItemAppearances } from "../../shared/types/popoverListItemAppearances";
import PopoverListItemIcon from "./PopoverListItemIcon";
import PopoverListItemAvatar from "./PopoverListItemAvatar";
Expand Down Expand Up @@ -48,7 +48,7 @@ const PopoverListItem = (props: PopoverListItemProps) => {
// on the :root
const menuListItemSelected = css`
background-color: ${themeBgSelected};
color: ${pickReadableTextColor(
color: ${getTextColor(
getCSSVarValue(themeBgSelected),
getCSSVarValue(themeTextColorPrimary),
getCSSVarValue(themeTextColorPrimaryInverted)
Expand Down
2 changes: 1 addition & 1 deletion packages/progressbar/style.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {
themeBrandPrimary
} from "../design-tokens/build/js/designTokens";
import { hexToRgbA } from "../shared/styles/color";
import getCSSVarValue from "../utilities/getCSSVarValue";
import { getCSSVarValue } from "../shared/styles/styleUtils/typography/color";
import { ProgressBarSizes } from "./components/ProgressBar";

export const defaultBarColor = themeBrandPrimary;
Expand Down
10 changes: 5 additions & 5 deletions packages/segmentedControl/style.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ import {
borderRadiusDefault,
themeBorder
} from "../design-tokens/build/js/designTokens";
import { pickReadableTextColor, darken } from "../shared/styles/color";
import getCSSVarValue from "../utilities/getCSSVarValue";
import { getTextColor, darken } from "../shared/styles/color";
import { getCSSVarValue } from "../shared/styles/styleUtils/typography/color";
import { buttonPadding } from "../button/style";

export const staticKeyboardFocusClassname = "static_segmentKeyboardFocus";
Expand Down Expand Up @@ -39,7 +39,7 @@ export const segmentedControlButton = css`
&:hover,
&:focus-within {
background-color: ${themeBgHover};
color: ${pickReadableTextColor(
color: ${getTextColor(
getCSSVarValue(themeBgHover),
getCSSVarValue(themeTextColorPrimary),
getCSSVarValue(themeTextColorPrimaryInverted)
Expand All @@ -55,7 +55,7 @@ export const segmentedControlButtonInner = css`
export const segmentedControlButtonActive = css`
background-color: ${themeBorder};
border-color: ${themeBorder};
color: ${pickReadableTextColor(
color: ${getTextColor(
getCSSVarValue(themeBorder),
getCSSVarValue(themeTextColorPrimary),
getCSSVarValue(themeTextColorPrimaryInverted)
Expand All @@ -64,7 +64,7 @@ export const segmentedControlButtonActive = css`
&:hover,
&:focus-within {
background-color: ${themeBorder};
color: ${pickReadableTextColor(
color: ${getTextColor(
getCSSVarValue(themeBorder),
getCSSVarValue(themeTextColorPrimary),
getCSSVarValue(themeTextColorPrimaryInverted)
Expand Down
29 changes: 0 additions & 29 deletions packages/shared/styles/color.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,38 +72,9 @@ const getHexBrightness = (hex: string) => {

export const isHexDark = (hex: string): boolean => getHexBrightness(hex) < 140;

// Normally this function would compare the contrast
// between the baseTextOption and invertedTextOption,
// but that forces text to be black on our success buttons.
//
// Checking with brightness still returns us the most readable
// option on the background color, but will let the success
// button have white text.
//
// The design team should try a different success color
// to improve legibility and accessibility.
export const pickReadableTextColor = (
bgColor,
baseTextOption,
invertedTextOption
) => {
const baseTextBrightness = getHexBrightness(baseTextOption);
const invertedTextBrightness = getHexBrightness(invertedTextOption);

if (!isHexDark(bgColor)) {
return baseTextBrightness < invertedTextBrightness
? baseTextOption
: invertedTextOption;
}
return baseTextBrightness > invertedTextBrightness
? baseTextOption
: invertedTextOption;
};

/*
* This function will select a WCAG-compliant text color based on the background color.
* Reference: https://wunnle.com/dynamic-text-color-based-on-background
* TODO: We should deprecate pickReadableTextColor and update components that use it.
*/
export const getTextColor = (
bgColor,
Expand Down
3 changes: 2 additions & 1 deletion packages/shared/styles/formStyles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,9 @@ import {
display,
tintContent
} from "./styleUtils";
import getCSSVarValue from "../../utilities/getCSSVarValue";
import { getCSSVarValue } from "./styleUtils/typography/color";
import { hexToRgbA } from "./color";

const getFocusFieldBg = color => hexToRgbA(color, 0.05);

export const textInputHeight = 36;
Expand Down
31 changes: 31 additions & 0 deletions packages/shared/styles/styleUtils/typography/color.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,34 @@ export const tintContentSecondary = css`
${tintContent(themeTextColorSecondaryInverted)};
}
`;

/**
* This function retrieves the value of a CSS variable
* from the document root or a provided fallback value.
*
* @param {string} cssVar - The CSS variable in the format of "var(--variable-name, fallback-value)"
*
* @returns {string} - Returns the computed value of the CSS variable if it's defined in the document root.
*
* If the variable is not defined or the function is called outside of a browser context, it returns the fallback value provided.
*/
export const getCSSVarValue = cssVar => {
// Get the content between the parentheses in "var()"
// and split the args from a string to an array
const cssVarArgsString = cssVar.match(/\(([^)]+)\)/)[1];
const cssVarArgs = cssVarArgsString.replace(/\s/g, "").split(",");

if (typeof window === "undefined") {
return cssVarArgs[1];
}

if (document?.documentElement !== null) {
// If there's a custom property defined on :root (<html>), return that.
// Otherwise, return the fallback value.
return (
getComputedStyle(document.documentElement)
.getPropertyValue(cssVarArgs[0])
.trim() || cssVarArgs[1]
);
}
};
16 changes: 8 additions & 8 deletions packages/shared/tests/color.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {
rgbToHex,
getHexContrast,
isHexDark,
pickReadableTextColor,
getTextColor,
pickHoverBg,
mixHex
} from "../styles/color";
Expand Down Expand Up @@ -80,16 +80,16 @@ describe("color utilities", () => {
});
});

describe("pickReadableTextColor", () => {
describe("getTextColor", () => {
it("returns inverted text color if the background and the base text color are dark", () => {
expect(
pickReadableTextColor("#000000", "#1B2029", "#FFFFFF").toUpperCase()
).toBe("#FFFFFF");
expect(getTextColor("#000000", "#1B2029", "#FFFFFF").toUpperCase()).toBe(
"#FFFFFF"
);
});
it("returns base text color if the background and the inverted text color are not dark", () => {
expect(
pickReadableTextColor("#FFFFFF", "#1B2029", "#999999").toUpperCase()
).toBe("#1B2029");
expect(getTextColor("#FFFFFF", "#1B2029", "#999999").toUpperCase()).toBe(
"#1B2029"
);
});
});

Expand Down
22 changes: 0 additions & 22 deletions packages/utilities/getCSSVarValue.ts

This file was deleted.

1 change: 0 additions & 1 deletion packages/utilities/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,2 @@
export { default as findNestedPropertyInObject } from "./findNestedPropertyInObject";
export { default as getCSSVarValue } from "./getCSSVarValue";
export { default as resizeEventManager } from "./resizeEventManager";

0 comments on commit 3da0b7f

Please sign in to comment.