Skip to content

BL-15812 Prefer autonyms in language cards #603

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.

Sign up for GitHub

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

Open
hatton wants to merge 1 commit into
base: master
Choose a base branch
from
Open

BL-15812 Prefer autonyms in language cards #603

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 8 additions & 4 deletions .vscode/settings.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
{
"workbench.colorCustomizations": {
"statusBar.background": "#49d668",
"statusBar.debuggingBackground": "#49d668",
"statusBar.background": "#549d3e",
"statusBar.noFolderBackground": "#49d668",
"statussBar.prominentBackground": "#D65649"
"statussBar.prominentBackground": "#D65649",
Copy link

Copilot AI Jan 29, 2026

Choose a reason for hiding this comment

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

The VS Code setting key statussBar.prominentBackground is misspelled (extra "s"), so it will be ignored by VS Code. Rename it to the correct statusBar.prominentBackground if the intent is to customize the prominent status bar background.

Suggested change
"statussBar.prominentBackground": "#D65649",
"statusBar.prominentBackground": "#D65649",

Copilot uses AI. Check for mistakes.
"statusBar.foreground": "#e7e7e7",
"statusBarItem.hoverBackground": "#6bbb53",
"statusBarItem.remoteBackground": "#549d3e",
"statusBarItem.remoteForeground": "#e7e7e7"
Comment on lines +3 to +9
Copy link

Copilot AI Jan 29, 2026

Choose a reason for hiding this comment

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

This PR is about language-card labeling, but it also changes editor/workbench settings in .vscode/settings.json (status bar colors, Peacock). These changes are unrelated to the feature and can create noisy diffs and merge conflicts; consider reverting them or moving them to a separate, explicitly-scoped PR.

Copilot uses AI. Check for mistakes.
},
"editor.formatOnSave": true,
"editor.defaultFormatter": "esbenp.prettier-vscode",
Expand All @@ -29,5 +32,6 @@
"uilang",
"voca",
"yagni"
]
],
"peacock.color": "#549d3e"
}
75 changes: 67 additions & 8 deletions src/components/LanguageCard.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,11 @@ import { css } from "@emotion/react";

import React from "react";
import { CheapCard } from "./CheapCard";
import { ILanguage, getDisplayNamesForLanguage } from "../model/Language";
import {
ILanguage,
getDisplayNamesForLanguage,
kTagForNoLanguage,
} from "../model/Language";
import { commonUI } from "../theme";
import { useResponsiveChoice } from "../responsiveUtilities";
import { FormattedMessage } from "react-intl";
Expand Down Expand Up @@ -43,13 +47,55 @@ export const LanguageCard: React.FunctionComponent<
...propsToPassDown
} = props; // Prevent React warnings

const { primary, secondary } = getDisplayNamesForLanguage(props);
const displayNames = getDisplayNamesForLanguage(props);
const getResponsiveChoice = useResponsiveChoice();
const { cardWidthPx, cardHeightPx } = useLanguageCardSpec(props.larger);
const urlPrefix = props.targetPrefix ?? "/language:";
const showCount = !useIsAppHosted();
const cardSpacing = useBaseCardSpec().cardSpacingPx;

const isPictureBook = isoCode === kTagForNoLanguage;

// BL-15812 Prefer the autonym (`name`) as the primary label; fall back to existing display logic
// for picture books or other special cases where `name` can be empty.
const primary = isPictureBook
? displayNames.primary
: name.trim()
? name
Comment on lines +61 to +64
Copy link

Copilot AI Jan 29, 2026

Choose a reason for hiding this comment

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

name.trim() is used only as a boolean check, but the displayed primary value uses the untrimmed name. If name has leading/trailing whitespace, the card will render the whitespace and comparisons like englishName !== primary may behave unexpectedly. Consider assigning const trimmedName = name.trim() and using trimmedName both for the emptiness check and as the displayed primary label.

Suggested change
const primary = isPictureBook
? displayNames.primary
: name.trim()
? name
const trimmedName = name.trim();
const primary = isPictureBook
? displayNames.primary
: trimmedName
? trimmedName

Copilot uses AI. Check for mistakes.
Copy link
Contributor

@JohnThomson JohnThomson Jan 30, 2026

Choose a reason for hiding this comment

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

I would agree, unless we have some way of knowing that displayNames.primary is already trimmed, or at least can't be all whitespace.

: displayNames.primary;

// Build the gray header labels explicitly so English and the tag can be separate lines.
const secondaryLinesToRender: string[] = [];
if (isPictureBook) {
// Preserve the historical duplication for picture-book cards.
secondaryLinesToRender.push(displayNames.primary);
} else {
if (englishName && englishName !== primary) {
secondaryLinesToRender.push(englishName);
}
// Match the tag as a whole token to avoid false positives like
// "fr" matching the autonym "français".
const secondaryTokens = displayNames.secondary
? displayNames.secondary.split(/\s+/)
: [];
const shouldShowTag =
!!isoCode &&
secondaryTokens.includes(isoCode) &&
isoCode !== englishName &&
isoCode !== primary;
if (shouldShowTag) {
secondaryLinesToRender.push(isoCode);
}
}
const secondaryLineText = secondaryLinesToRender.join(" ");
const hasMultipleSecondaryLines = secondaryLinesToRender.length > 1;
// Only used for the single-line case, where we can allow two lines of wrap.
const shouldTruncateSecondary = secondaryLineText.length >= 18;
const [
secondaryPrimaryLine,
...secondaryRemainingLines
] = secondaryLinesToRender;

// In the main website, we want language cards to be responsive: smaller and with smaller text on small screens.
// In the language chooser intended to be embedded in BloomReader, we want larger sizes.
// The description said "about a third larger" which happens to be, for most measurements, what the large-screen
Expand Down Expand Up @@ -119,12 +165,25 @@ export const LanguageCard: React.FunctionComponent<
line-height: 1em;
`}
>
<SmartTruncateMarkup
condition={(secondary ?? "").length >= 18}
lines={2}
>
<span>{secondary}</span>
</SmartTruncateMarkup>
{secondaryLinesToRender.length > 0 &&
(hasMultipleSecondaryLines ? (
<React.Fragment>
{/* Clamp the first line so the tag line always stays visible. */}
<SmartTruncateMarkup condition={true} lines={1}>
<span>{secondaryPrimaryLine}</span>
</SmartTruncateMarkup>
{secondaryRemainingLines.map((line, index) => (
<div key={`${line}-${index}`}>{line}</div>
))}
</React.Fragment>
) : (
<SmartTruncateMarkup
condition={shouldTruncateSecondary}
lines={2}
>
<span>{secondaryPrimaryLine}</span>
</SmartTruncateMarkup>
))}
</div>
</div>
<h2
Expand Down
Loading