Migrate dark-mode-web to new ab test framework#15299
Conversation
dskamiotis
left a comment
There was a problem hiding this comment.
This looks good to me 👍
ab-testing/config/abTests.ts
Outdated
| description: "Dark mode accessibility feature test on web", | ||
| owners: ["dotcom.platform@theguardian.com"], | ||
| status: "ON", | ||
| expirationDate: "2027-04-05", |
There was a problem hiding this comment.
This is a Monday right? Feels like a slightly dangerous time for a test to expire as there's noone working at the weekend to see an upcoming expiry notification. Maybe we should aim for mid week?
There was a problem hiding this comment.
Yeah, that's a good shout, would that be the case for any test? Should we add a check for tests ending them on a Monday?
There was a problem hiding this comment.
Yeah I did think about that! Would be nice to warn people about Monday/Friday/Saturday/Sunday expiries. I dunno if it's enough to actually fail a workflow though as there may be some tests which should intentionally expire on a weekend. This also doesn't account for UK bank holidays so didn't want to rely on that alone
| renderingTarget, | ||
| darkModeAvailable: | ||
| frontendData.config.abTests.darkModeWebVariant === 'variant', | ||
| darkModeAvailable, |
There was a problem hiding this comment.
Should we add this config to the other render.* files?
render.allEditorialNewslettersPage.web.tsx
render.article.apps.tsx
render.editionsCrossword.tsx
render.thrasher.apps.tsx
There was a problem hiding this comment.
For apps darkModeAvailable is always true :)
allEditorialNewslettersPage doesn't seem to have dark mode colours setup, so will consider that out of scope for this!
and editionsCrossword I'm not sure?
There was a problem hiding this comment.
Okay we will add the config when we implement dark mode across all pages
ab-testing/config/abTests.ts
Outdated
|
|
||
| const ABTests: ABTest[] = [ | ||
| { | ||
| name: "webex-dark-mode-web", |
There was a problem hiding this comment.
There was a long debate about this but the spelling 'WebX' is preferred :]
What does this change?
Migrate Dark Mode on Web 0% test to the new framework. Update the accessibility page for the new opt in behaviour.
The tickbox has been replaced with opt-in links, the domain that the cookie is on didn't play nicely with
setCookie, and there's no way to specify a domain so rather than mess around getting it working, links are fine, for this experimental feature.Regardless, this means anyone opted in will need to re-opt in to retain dark mode.
Why?
We need all tests on the legacy framework removed or migrated so we can delete it!
Screenshots