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

feat(language-menu): add "Remember language" experiment #11518

Merged
merged 34 commits into from
Sep 25, 2024
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
7dd78b8
chore(deps): add cookie-parser to /cloud-function
caugner Jul 23, 2024
7b79f24
refactor(cloud-function): extract canonicals module
caugner Jul 23, 2024
91de9b8
feat(cloud-function): always redirect to preferred locale
caugner Jul 23, 2024
f047dec
chore(cloud-function): ignore referer for preferred locale redirect
caugner Jul 23, 2024
6fe04de
Merge branch 'main' into MP-1242-redirect-to-preferred-locale
caugner Sep 12, 2024
213869f
refactor(language-menu): extract {get,set}CookieValue()
caugner Sep 12, 2024
1cc7ef7
fix(switch): use em instead of rem
caugner Sep 12, 2024
dcfcaa0
feat(language-menu): add automatic redirect setting
caugner Sep 12, 2024
c5a4c62
chore(language-menu): reduce item padding
caugner Sep 12, 2024
3c01161
chore(language-menu): add slight padding between items
caugner Sep 12, 2024
b72d27e
chore(language-menu): revisit thumbs confirmationg
caugner Sep 12, 2024
867749e
Merge branch 'main' into MP-1242-redirect-to-preferred-locale
caugner Sep 12, 2024
256fc31
fix(language-menu): avoid document.cookie access on SSR
caugner Sep 12, 2024
f95cb93
fix(language-menu): set preferredlocale on redirect
caugner Sep 12, 2024
a3be882
fix(language-menu): measure locale-redirct clicks
caugner Sep 12, 2024
3de136b
fix(switch): remove margin-left from label
caugner Sep 12, 2024
f5b9bad
fixup! fix(switch): use em instead of rem
caugner Sep 12, 2024
dd6a4f1
chore(language-menu): improve remember design
caugner Sep 12, 2024
a3986b2
fix(switch): correct lengths
caugner Sep 12, 2024
dc9293e
fix(thumbs): use em over rem
caugner Sep 12, 2024
dbf9392
fix(language-menu): correct spacing
caugner Sep 12, 2024
60feaee
chore(language-menu): make thumbs text italic
caugner Sep 12, 2024
258738d
feat(language-menu): reuse preferredlocale, make opt-in
caugner Sep 16, 2024
3f0fe62
chore(language-menu): clarify padding
caugner Sep 23, 2024
ddeaf6d
Apply suggestions from code review
caugner Sep 23, 2024
2f348f7
refactor(language-menu): extract cookie max-age to constant
caugner Sep 23, 2024
579b715
Merge branch 'main' into MP-1242-redirect-to-preferred-locale
LeoMcA Sep 23, 2024
afe16f1
Merge branch 'main' into MP-1242-redirect-to-preferred-locale
caugner Sep 24, 2024
c15f057
fix(language-menu): enable "Remember language" only on matching locale
caugner Sep 24, 2024
8f7819b
Merge branch 'main' into MP-1242-redirect-to-preferred-locale
caugner Sep 25, 2024
299dfc1
Merge branch 'main' into MP-1242-redirect-to-preferred-locale
caugner Sep 25, 2024
3bf6894
refactor(telemtry): rename language_{redirect => remember}
caugner Sep 25, 2024
1f640ff
fix(language-menu): set "Remember language" locale unconditionally
caugner Sep 25, 2024
5bcbea6
fix(language-menu): improve "Remember language" measurement
caugner Sep 25, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions client/src/telemetry/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,5 +82,6 @@ export const BASELINE = Object.freeze({

export const CLIENT_SIDE_NAVIGATION = "client_side_nav";
export const LANGUAGE = "language";
export const LANGUAGE_REDIRECT = "language_redirect";
export const THEME_SWITCHER = "theme_switcher";
export const SURVEY = "survey";
8 changes: 4 additions & 4 deletions client/src/ui/atoms/switch/index.scss
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
height: 0;
margin: 0;
opacity: 0;
width: 3rem;
width: 3em;

&:checked + .slider {
background-color: var(--text-link);
Expand All @@ -29,10 +29,10 @@
background-color: var(--text-secondary);
border-radius: 1.5em;
cursor: pointer;
height: 1.5rem;
height: 1.5em;
position: absolute;
transition: 0.4s;
width: 3rem;
width: 3em;

&:before {
background-color: var(--background-primary);
Expand All @@ -48,6 +48,6 @@
}

.label {
margin-left: 0.5rem;
margin-left: 0.5em;
}
}
4 changes: 2 additions & 2 deletions client/src/ui/atoms/thumbs/index.scss
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
align-items: center;
display: flex;
flex-direction: row;
gap: 0.5rem;
height: 1.5rem;
gap: 0.5em;
height: 1.5em;

.confirmation {
animation-duration: 2.5s;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,44 @@
}

.language-menu {
li {
&:not(:first-child) {
padding-top: 1px;
}

&:not(:last-child) {
padding-bottom: 1px;
}
}

.submenu-item {
padding: 0.5rem;
padding: 0.5rem !important;

&.locale-redirect-setting {
border-bottom: 1px solid var(--border-secondary) !important;
border-radius: 0 !important;
caugner marked this conversation as resolved.
Show resolved Hide resolved
display: block;
font-size: var(--type-tiny-font-size);
padding: 0.25em 0.5em;
caugner marked this conversation as resolved.
Show resolved Hide resolved

&:hover {
background-color: unset;
}

.switch {
display: flex;
}

.glean-thumbs {
font-style: italic;
font-variation-settings: "slnt" -10;
margin-top: 0.5em;

.icon {
margin-right: unset;
}
}
}
}

@media (min-width: $screen-md) {
Expand Down
101 changes: 67 additions & 34 deletions client/src/ui/organisms/article-actions/language-menu/index.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useState } from "react";
import { useEffect, useState } from "react";
import { useLocation } from "react-router-dom";

import { useGleanClick } from "../../../../telemetry/glean-context";
Expand All @@ -9,7 +9,14 @@ import { Submenu } from "../../../molecules/submenu";
import "./index.scss";
import { DropdownMenu, DropdownMenuWrapper } from "../../../molecules/dropdown";
import { useLocale } from "../../../../hooks";
import { LANGUAGE } from "../../../../telemetry/constants";
import { LANGUAGE, LANGUAGE_REDIRECT } from "../../../../telemetry/constants";
import {
deleteCookie,
getCookieValue,
setCookieValue,
} from "../../../../utils";
import { GleanThumbs } from "../../../atoms/thumbs";
import { Switch } from "../../../atoms/switch";

// This needs to match what's set in 'libs/constants.js' on the server/builder!
const PREFERRED_LOCALE_COOKIE_NAME = "preferredlocale";
Expand All @@ -29,51 +36,43 @@ export function LanguageMenu({
const [isOpen, setIsOpen] = useState<boolean>(false);

const changeLocale: React.MouseEventHandler<HTMLAnchorElement> = (event) => {
const preferredLocale = event.currentTarget.dataset.locale;
const newLocale = event.currentTarget.dataset.locale;
// The default is the current locale itself. If that's what's chosen,
// don't bother redirecting.
if (preferredLocale !== locale) {
let cookieValueBefore = document.cookie
.split("; ")
.find((row) => row.startsWith(`${PREFERRED_LOCALE_COOKIE_NAME}=`));
if (cookieValueBefore && cookieValueBefore.includes("=")) {
cookieValueBefore = cookieValueBefore.split("=")[1];
}
if (newLocale !== locale) {
const cookieValueBefore = getCookieValue(PREFERRED_LOCALE_COOKIE_NAME);

for (const translation of translations) {
if (translation.locale === preferredLocale) {
let cookieValue = `${PREFERRED_LOCALE_COOKIE_NAME}=${
translation.locale
};max-age=${60 * 60 * 24 * 365 * 3};path=/`;
if (
!(
document.location.hostname === "localhost" ||
document.location.hostname === "localhost.org"
)
) {
cookieValue += ";secure";
if (cookieValueBefore === locale) {
for (const translation of translations) {
if (translation.locale === newLocale) {
setCookieValue(PREFERRED_LOCALE_COOKIE_NAME, translation.locale, {
maxAge: 60 * 60 * 24 * 365 * 3,
});
}
document.cookie = cookieValue;
}
}

const oldValue = cookieValueBefore || "none";
gleanClick(`${LANGUAGE}: ${oldValue} -> ${preferredLocale}`);
gleanClick(`${LANGUAGE}: ${locale} -> ${newLocale}`);
}
};

const menuEntry = {
label: "Languages",
id: menuId,
items: translations.map((translation) => ({
component: () => (
<LanguageMenuItem
native={native}
translation={translation}
changeLocale={changeLocale}
/>
),
})),
items: [
{
component: () => <LocaleRedirectSetting />,
},
...translations.map((translation) => ({
component: () => (
<LanguageMenuItem
native={native}
translation={translation}
changeLocale={changeLocale}
/>
),
})),
],
};

return (
Expand Down Expand Up @@ -127,3 +126,37 @@ function LanguageMenuItem({
</a>
);
}

function LocaleRedirectSetting() {
const gleanClick = useGleanClick();
const locale = useLocale();
const [value, setValue] = useState(false);

useEffect(() => {
setValue(!!getCookieValue(PREFERRED_LOCALE_COOKIE_NAME));
caugner marked this conversation as resolved.
Show resolved Hide resolved
}, [setValue]);
caugner marked this conversation as resolved.
Show resolved Hide resolved

function toggle(event) {
const newValue = event.target.checked;
if (newValue) {
if (!getCookieValue(PREFERRED_LOCALE_COOKIE_NAME)) {
setCookieValue(PREFERRED_LOCALE_COOKIE_NAME, locale, {
maxAge: 60 * 60 * 24 * 365 * 3,
caugner marked this conversation as resolved.
Show resolved Hide resolved
});
}
} else {
deleteCookie(PREFERRED_LOCALE_COOKIE_NAME);
}
setValue(event.target.checked);
caugner marked this conversation as resolved.
Show resolved Hide resolved
gleanClick(`${LANGUAGE_REDIRECT}: ${locale} -> ${Number(newValue)}`);
}

return (
<form className="submenu-item locale-redirect-setting">
<Switch name="locale-redirect" checked={value} toggle={toggle}>
Remember language
</Switch>
<GleanThumbs feature="locale-redirect" question="Is this useful?" />
</form>
);
}
38 changes: 38 additions & 0 deletions client/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,3 +136,41 @@ export function splitQuery(term: string): string[] {
return term.split(/[ ,.]+/);
}
}

export function getCookieValue(name: string) {
let value = document.cookie
.split("; ")
.find((row) => row.startsWith(`${name}=`));

if (value && value.includes("=")) {
value = value.split("=")[1];
}

return value;
}

export function setCookieValue(
name: string,
value: string,
{
expires,
maxAge,
path = "/",
}: { expires?: Date; maxAge?: number; path?: string }
) {
const cookieValue = [
`${name}=${value}`,
expires && `expires=${expires.toUTCString()}`,
maxAge && `max-age=${maxAge}`,
`path=${path}`,
document.location.hostname !== "localhost" && "secure",
]
.filter(Boolean)
.join(";");

document.cookie = cookieValue;
}

export function deleteCookie(name: string) {
setCookieValue(name, "", { expires: new Date(0) });
}
34 changes: 34 additions & 0 deletions cloud-function/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions cloud-function/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
"@yari-internal/pong": "file:src/internal/pong",
"@yari-internal/slug-utils": "file:src/internal/slug-utils",
"accept-language-parser": "^1.5.0",
"cookie-parser": "^1.4.6",
"dotenv": "^16.0.3",
"express": "^4.19.2",
"http-proxy-middleware": "^3.0.0",
Expand All @@ -44,6 +45,7 @@
"devDependencies": {
"@swc/core": "^1.3.38",
"@types/accept-language-parser": "^1.5.3",
"@types/cookie-parser": "^1.4.7",
"@types/http-proxy": "^1.17.10",
"@types/http-server": "^0.12.1",
"cross-env": "^7.0.3",
Expand Down
4 changes: 4 additions & 0 deletions cloud-function/src/app.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import cookieParser from "cookie-parser";
import express, { Request, Response } from "express";
import { Router } from "express";

Expand All @@ -16,6 +17,7 @@ import { redirectMovedPages } from "./middlewares/redirect-moved-pages.js";
import { redirectEnforceTrailingSlash } from "./middlewares/redirect-enforce-trailing-slash.js";
import { redirectFundamental } from "./middlewares/redirect-fundamental.js";
import { redirectLocale } from "./middlewares/redirect-locale.js";
import { redirectPreferredLocale } from "./middlewares/redirect-preferred-locale.js";
import { redirectTrailingSlash } from "./middlewares/redirect-trailing-slash.js";
import { requireOrigin } from "./middlewares/require-origin.js";
import { notFound } from "./middlewares/not-found.js";
Expand All @@ -25,6 +27,7 @@ import { stripForwardedHostHeaders } from "./middlewares/stripForwardedHostHeade
import { proxyPong } from "./handlers/proxy-pong.js";

const router = Router();
router.use(cookieParser());
router.use(stripForwardedHostHeaders);
router.use(redirectLeadingSlash);
// MDN Plus plans.
Expand Down Expand Up @@ -85,6 +88,7 @@ router.get(
requireOrigin(Origin.main),
redirectFundamental,
redirectLocale,
redirectPreferredLocale,
redirectTrailingSlash,
redirectMovedPages,
resolveIndexHTML,
Expand Down
5 changes: 5 additions & 0 deletions cloud-function/src/canonicals.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { createRequire } from "node:module";

const require = createRequire(import.meta.url);

export const CANONICALS = require("../canonicals.json");
Loading