From 297cf047f9fda542a66016e6125499e1c496c3f1 Mon Sep 17 00:00:00 2001 From: Sarah Schneider Date: Tue, 27 Apr 2021 12:56:14 -0400 Subject: [PATCH 1/5] AB test! --- javascripts/experiment.js | 21 ++++++++++++++++----- javascripts/index.js | 2 +- javascripts/toggle-images.js | 13 +++++++++---- 3 files changed, 26 insertions(+), 10 deletions(-) diff --git a/javascripts/experiment.js b/javascripts/experiment.js index 9a090665bb..cbedd44bd4 100644 --- a/javascripts/experiment.js +++ b/javascripts/experiment.js @@ -1,5 +1,6 @@ import murmur from 'imurmurhash' import { getUserEventsId, sendEvent } from './events' +import toggleImages from './toggle-images' // import h from './hyperscript' const TREATMENT = 'TREATMENT' @@ -20,11 +21,21 @@ export function sendSuccess (test) { }) } +function applyTreatment (toggleButton) { + // Treatment-specific options. + const hideImagesByDefault = true + const focusButtonByDefault = true + + toggleImages(hideImagesByDefault, focusButtonByDefault) +} + export default function () { // *** Example test code *** - // const testName = '$test-name$' - // const xbucket = bucket(testName) - // const x = document.querySelector(...) - // x.addEventListener('click', () => { sendSuccess(testName) }) - // if (xbucket === TREATMENT) applyTreatment(x) + const testName = 'toggle-images' + const xbucket = bucket(testName) + const x = document.getElementById('js-toggle-images') + if (!x) return + + x.addEventListener('click', () => { sendSuccess(testName) }) + if (xbucket === TREATMENT) applyTreatment(x) } diff --git a/javascripts/index.js b/javascripts/index.js index d0ef8efce6..2a53cc4e34 100644 --- a/javascripts/index.js +++ b/javascripts/index.js @@ -41,7 +41,7 @@ document.addEventListener('DOMContentLoaded', async () => { airgapLinks() releaseNotes() initializeEvents() - experiment() helpfulness() toggleImages() + experiment() }) diff --git a/javascripts/toggle-images.js b/javascripts/toggle-images.js index 2c40d505b7..75dc9866ef 100644 --- a/javascripts/toggle-images.js +++ b/javascripts/toggle-images.js @@ -1,8 +1,6 @@ // import { sendEvent } from './events' import Cookies from 'js-cookie' -// Determines whether images are hidden or displayed on first visit. -const hideImagesByDefault = false // Set the image placeholder icon here. const placeholderImagePath = '/assets/images/octicons/image.svg' @@ -11,7 +9,7 @@ const placeholderImagePath = '/assets/images/octicons/image.svg' * This module adds a new icon button in the margin to toggle all images on the page. * It uses cookies to keep track of a user's selected image preference. */ -export default function () { +export default function (hideImagesByDefault = false, focusButtonByDefault = false) { const toggleImagesBtn = document.getElementById('js-toggle-images') if (!toggleImagesBtn) return @@ -25,7 +23,7 @@ export default function () { toggleImagesBtn.removeAttribute('hidden') // Look for a cookie with image visibility preference; otherwise, use the default. - const hideImagesPreferred = (Cookies.get('hideImagesPreferred') === 'true') || hideImagesByDefault + const hideImagesPreferred = hideImagesByDefault || (Cookies.get('hideImagesPreferred') === 'true') // Hide the images if that is the preference. if (hideImagesPreferred) { @@ -42,10 +40,17 @@ export default function () { // Set the starting state depending on user preferences. if (hideImagesPreferred) { + onIcon.setAttribute('hidden', true) offIcon.removeAttribute('hidden') toggleImagesBtn.setAttribute('aria-label', tooltipImagesOff) + // Show the tooltip if images are hidden by default to help users see the toggle button. + // Downside: the button will begin with focus whenever the user goes to a new page. + if (focusButtonByDefault) { + toggleImagesBtn.focus({ preventScroll: true }) + } } else { onIcon.removeAttribute('hidden') + offIcon.setAttribute('hidden', true) toggleImagesBtn.setAttribute('aria-label', tooltipImagesOn) } From cc500a4602e165c274a3d5444630e5f0ed1cf57d Mon Sep 17 00:00:00 2001 From: Sarah Schneider Date: Tue, 27 Apr 2021 13:10:44 -0400 Subject: [PATCH 2/5] lint --- javascripts/toggle-images.js | 1 - 1 file changed, 1 deletion(-) diff --git a/javascripts/toggle-images.js b/javascripts/toggle-images.js index 75dc9866ef..fef70361ce 100644 --- a/javascripts/toggle-images.js +++ b/javascripts/toggle-images.js @@ -1,7 +1,6 @@ // import { sendEvent } from './events' import Cookies from 'js-cookie' - // Set the image placeholder icon here. const placeholderImagePath = '/assets/images/octicons/image.svg' From 8cdef3997f60edf36b425c788631d8ceaa1ca316 Mon Sep 17 00:00:00 2001 From: Sarah Schneider Date: Wed, 28 Apr 2021 15:26:34 -0400 Subject: [PATCH 3/5] allow force treatment with env variable --- javascripts/experiment.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/javascripts/experiment.js b/javascripts/experiment.js index cbedd44bd4..8a4bb0d304 100644 --- a/javascripts/experiment.js +++ b/javascripts/experiment.js @@ -32,7 +32,9 @@ function applyTreatment (toggleButton) { export default function () { // *** Example test code *** const testName = 'toggle-images' - const xbucket = bucket(testName) + const xbucket = process.env.TEST_TREATMENT + ? 'TREATMENT' + : bucket(testName) const x = document.getElementById('js-toggle-images') if (!x) return From 0d08b8c78f85e8505125c204eb9034eaaf9db15f Mon Sep 17 00:00:00 2001 From: Sarah Schneider Date: Thu, 29 Apr 2021 11:07:08 -0400 Subject: [PATCH 4/5] tidy up the code, var names, etc --- javascripts/experiment.js | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/javascripts/experiment.js b/javascripts/experiment.js index 8a4bb0d304..fed4befb5c 100644 --- a/javascripts/experiment.js +++ b/javascripts/experiment.js @@ -21,23 +21,23 @@ export function sendSuccess (test) { }) } -function applyTreatment (toggleButton) { +function applyTreatment () { // Treatment-specific options. const hideImagesByDefault = true const focusButtonByDefault = true + // Run toggleImages a second time on each page, but with treatment defaults. toggleImages(hideImagesByDefault, focusButtonByDefault) } export default function () { - // *** Example test code *** const testName = 'toggle-images' - const xbucket = process.env.TEST_TREATMENT - ? 'TREATMENT' - : bucket(testName) - const x = document.getElementById('js-toggle-images') - if (!x) return + const xbucket = bucket(testName) - x.addEventListener('click', () => { sendSuccess(testName) }) - if (xbucket === TREATMENT) applyTreatment(x) + const toggleImagesBtn = document.getElementById('js-toggle-images') + if (!toggleImagesBtn) return + + toggleImagesBtn.addEventListener('click', () => { sendSuccess(testName) }) + + if (xbucket === TREATMENT) applyTreatment() } From afea41a43d11affef268958db1af1d97615424c3 Mon Sep 17 00:00:00 2001 From: Sarah Schneider Date: Thu, 29 Apr 2021 11:07:45 -0400 Subject: [PATCH 5/5] fix logic of treatment bucket vs. cookie preference --- javascripts/toggle-images.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/javascripts/toggle-images.js b/javascripts/toggle-images.js index fef70361ce..b47d6d72c3 100644 --- a/javascripts/toggle-images.js +++ b/javascripts/toggle-images.js @@ -1,4 +1,3 @@ -// import { sendEvent } from './events' import Cookies from 'js-cookie' // Set the image placeholder icon here. @@ -22,7 +21,9 @@ export default function (hideImagesByDefault = false, focusButtonByDefault = fal toggleImagesBtn.removeAttribute('hidden') // Look for a cookie with image visibility preference; otherwise, use the default. - const hideImagesPreferred = hideImagesByDefault || (Cookies.get('hideImagesPreferred') === 'true') + const hideImagesPreferred = Cookies.get('hideImagesPreferred') === 'false' + ? false + : Cookies.get('hideImagesPreferred') === 'true' || hideImagesByDefault // Hide the images if that is the preference. if (hideImagesPreferred) { @@ -42,6 +43,7 @@ export default function (hideImagesByDefault = false, focusButtonByDefault = fal onIcon.setAttribute('hidden', true) offIcon.removeAttribute('hidden') toggleImagesBtn.setAttribute('aria-label', tooltipImagesOff) + // Show the tooltip if images are hidden by default to help users see the toggle button. // Downside: the button will begin with focus whenever the user goes to a new page. if (focusButtonByDefault) { @@ -73,16 +75,14 @@ export default function (hideImagesByDefault = false, focusButtonByDefault = fal } // Remove focus from the button after click so the tooltip does not stay displayed. - toggleImagesBtn.blur() + // Use settimeout to work around Firefox-specific issue. + setTimeout(() => { toggleImagesBtn.blur() }, 100) // Save this preference as a cookie. - Cookies.set('hideImagesPreferred', showOnNextClick) + Cookies.set('hideImagesPreferred', showOnNextClick, { sameSite: 'strict', secure: true }) // Toggle the action on every click. showOnNextClick = !showOnNextClick - - // TODO Track image toggle events - // sendEvent({ type: 'imageToggle' }) }) }