Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add warning for low contrast and equal colors #86

Closed
wants to merge 13 commits into from
Closed
1 change: 1 addition & 0 deletions LICENSE.md
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ The icons `assets/qr-icon*.svg` (used in `src/icons/icon-small-*.svg`) are made
* [`common/img/info-dark.svg`](common/img/info-dark.svg) by Mozilla, license: [MPL v2.0](https://www.mozilla.org/en-US/MPL/2.0/)
* [`common/img/info-light.svg`](common/img/info-light.svg) by Mozilla, license: [MPL v2.0](https://www.mozilla.org/en-US/MPL/2.0/)
* [`common/img/open-in-new.svg`](common/img/open-in-new.svg) by Mozilla, license: [MPL v2.0](https://www.mozilla.org/en-US/MPL/2.0/)
* [`common/img/warning-dark.svg`](common/img/warning-dark.svg) by Mozilla, license: [MPL v2.0](https://www.mozilla.org/en-US/MPL/2.0/)

The MPLv2.0 license is reproduced below:

Expand Down
8 changes: 8 additions & 0 deletions src/_locales/de/messages.json
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,14 @@
"message": "Bist du sicher, dass du alle Optionen zurücksetzen willst?",
"description": "The message shown, when the user is prompted to confirm the resetting of all options"
},
"lowContrastRatio": {
"message": "Das Kontrastverhältnis zwischen der Farbe des QR-Codes und dem Hintergrund ist wahrscheinlich zu niedrig um von QR-Code-Scannern erkannt zu werden",
"desription": "The message shown when the contrast ratio is too low"
},
"sameColor": {
"message": "Die Farbe des QR-Codes und des Hintergrunds ist identisch. Das solltest du ändern!",
"description": "The message shown when the color for the QR code and the background are equal"
},

// tips
"tipYouLikeAddon": {
Expand Down
8 changes: 8 additions & 0 deletions src/_locales/en/messages.json
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,14 @@
"message": "Are you sure that you want to reset all options?",
"description": "The message shown, when the user is prompted to confirm the resetting of all options"
},
"lowContrastRatio": {
"message": "The contrast ratio between the QR code color and the background might be too low to be recognized by QR code readers",
"desription": "The message shown when the contrast ratio is too low"
},
"sameColor": {
"message": "The background and the foreground color are equal. You should change this!",
"description": "The message shown when the color for the QR code and the background are equal"
},

// tips
"tipYouLikeAddon": {
Expand Down
8 changes: 8 additions & 0 deletions src/common/common.css
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,11 @@ a:active {
background-color: var(--green-50);
}

.warning {
color: var(--yellow-90);
background-color: var(--yellow-50);
}

/* message box action button */
.message-action-button {
margin-left: 8px;
Expand Down Expand Up @@ -188,6 +193,9 @@ a:active {
.success::before {
background-image: url('/common/img/check.svg');
}
.warning::before {
background-image: url('/common/img/warning-dark.svg');
}

.icon-dismiss {
box-sizing: content-box;
Expand Down
6 changes: 3 additions & 3 deletions src/common/common.js
Original file line number Diff line number Diff line change
Expand Up @@ -732,7 +732,7 @@ const MessageHandler = (function () {// eslint-disable-line no-unused-vars
if (typeof args[0] === "boolean") {
isDismissable = args.shift();
}
if (typeof args[0] !== undefined && args[0].text !== undefined && args[0].action !== undefined) {
if (args[0] !== undefined && args[0].text !== "undefined" && args[0].action !== "undefined") {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW I cherry-picked this change into the master as it is just a bug fix for a issue I introduced. But BTW the last undefines do not need (or maybe even should not) be quoted. (typeof is not even used there).

Basically I suggest you to merge the master into my branch, that should fix these conflicts.

actionButton = args.shift();
}

Expand All @@ -748,7 +748,7 @@ const MessageHandler = (function () {// eslint-disable-line no-unused-vars
if (isDismissable === true && elDismissIcon) {
// add an icon which dismisses the message if clicked
elDismissIcon.classList.remove("invisible");
} else {
} else if (elDismissIcon) {
elDismissIcon.classList.add("invisible");
}

Expand All @@ -770,7 +770,7 @@ const MessageHandler = (function () {// eslint-disable-line no-unused-vars

elActionButton.textContent = browser.i18n.getMessage(actionButton.text) || actionButton.text;
elActionButton.classList.remove("invisible");
} else {
} else if (elActionButton) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, good point!

Dismiss icon (https://github.com/rugk/offline-qr-code/pull/86/files#diff-ccdc4da75e7ac1061a8cc0851e3c7122R751) needs the same adjustments. (but I can also do them by myself)

elActionButton.classList.add("invisible");
}

Expand Down
6 changes: 6 additions & 0 deletions src/common/img/warning-dark.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
6 changes: 6 additions & 0 deletions src/common/variables.css
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,12 @@
--green-80: #006504;
--green-90: #003706;

--yellow-50: #ffe900;
--yellow-60: #d7b600;
--yellow-70: #a47f00;
--yellow-80: #715100;
--yellow-90: #3e2800;

--red-60: #d70022;

--grey-20: #ededf0;
Expand Down
4 changes: 4 additions & 0 deletions src/options.html
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@
<span class="message-text">An error happened.</span>
<img class="icon-dismiss invisible" src="/common/img/close.svg" width="24" height="24" tabindex="0" data-i18n data-i18n-aria-label="__MSG_dismissIconDescription__"></span>
</div>
<div id="messageWarning" class="message-box warning invisible fade-hide">
<span class="message-text">There were some difficulties.</span>
<img class="icon-dismiss invisible" src="/common/img/close.svg" width="24" height="24" tabindex="0" data-i18n data-i18n-aria-label="__MSG_dismissIconDescription__"></span>
</div>
</div>
<form>
<ul>
Expand Down
81 changes: 78 additions & 3 deletions src/options.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ const OptionHandler = (function () {
/**
* Applies option to element.
*
* @name OptionHandler.applyOptionToElementToElement
* @name OptionHandler.applyOptionToElement
* @function
* @private
* @param {string} option string ob object ID
Expand Down Expand Up @@ -151,6 +151,8 @@ const OptionHandler = (function () {
// run for each option, which we know to handle
applyOptionLive("popupIconColored", res.popupIconColored);
applyOptionLive("qrCodeSize", res.qrCodeSize);
applyOptionLive("qrColor", res.qrColor);
applyOptionLive("qrBackgroundColor", res.qrBackgroundColor);
});
}

Expand Down Expand Up @@ -200,11 +202,84 @@ const OptionHandler = (function () {

case "debugMode":
Logger.setDebugMode(optionValue);
break;

case "qrColor":
case "qrBackgroundColor": {
const elQrColor = document.getElementById("qrColor");
const elQrBackgroundColor = document.getElementById("qrBackgroundColor");

const qrCodeColor = hexToRgb(elQrColor.value);
const qrCodeBackgroundColor = hexToRgb(elQrBackgroundColor.value);

const colorContrast = contrast(qrCodeColor, qrCodeBackgroundColor);
// TODO: what is the right value here?
if (colorContrast <= 4) {
MessageHandler.hideError();
MessageHandler.showWarning("lowContrastRatio", false);
} else {
MessageHandler.hideWarning();
}

if (elQrColor.value === elQrBackgroundColor.value) {
MessageHandler.hideWarning();
MessageHandler.showError("sameColor", false);
} else {
MessageHandler.hideError();
}
}
}

return null;
}

/**
* Calculates the contrast between the QR code color and the background.
*
* @name OptionHandler.contrast
* @function
* @private
* @param {Array} rgb1
* @param {Array} rgb2
* @returns {int}
*/
function contrast(rgb1, rgb2) {
const colors = [rgb1[0], rgb1[1], rgb1[2], rgb2[0], rgb2[1], rgb2[2]];
for (let i = 0; i < colors.length; i++) {
// Formula: https://www.w3.org/TR/2008/REC-WCAG20-20081211/#relativeluminancedef
const c = colors[i] / 255;
// I'm using 0.04045 here instead of 0.03928 because 0.04045 is the number
// used in the actual sRGB standard.
// See also: https://github.com/w3c/wcag21/issues/815
colors[i] = c < 0.04045 ? c / 12.92 : Math.pow((c + 0.055) / 1.055, 2.4);
}
// Formula: https://www.w3.org/TR/2008/REC-WCAG20-20081211/#contrast-ratiodef
const l1 = 0.2126 * colors[0] + 0.7152 * colors[1] + 0.0722 * colors[2];
const l2 = 0.2126 * colors[3] + 0.7152 * colors[4] + 0.0722 * colors[5];
return (Math.max(l1, l2) + 0.05) / (Math.min(l1, l2) + 0.05);
}

/**
* Converts a hex color string to RGB.
*
* @name OptionHandler.hexToRgb
* @function
* @private
* @param {string} hex
* @returns {Array|null}
*/
function hexToRgb(hex) {
const result = /^#?([a-f\d]{2})([a-f\d]{2})([a-f\d]{2})$/i.exec(hex);
if (result) {
return [
parseInt(result[1], 16),
parseInt(result[2], 16),
parseInt(result[3], 16)
];
}
return null;
}

/**
* Saves all settings.
*
Expand Down Expand Up @@ -475,9 +550,9 @@ const OptionHandler = (function () {
}

/**
* Localizes static strings in the HTML file.
* Initializes the options.
*
* @name Localizer.init
* @name OptionHandler.init
* @function
* @returns {void}
*/
Expand Down