Skip to content

Commit

Permalink
Merge pull request #522 from BloomBooks/BL12827
Browse files Browse the repository at this point in the history 
Show "Picture Books" in language card list (BL-12827) (#522)
  • Loading branch information
@JohnThomson
JohnThomson authored Nov 6, 2023
2 parents c4fb78e + 4c9f8c1 commit 3b689f4
Show file tree
Hide file tree
Showing 5 changed files with 99 additions and 46 deletions.
88 changes: 54 additions & 34 deletions src/components/ByLanguageGroups.tsx
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import React, { useContext, useEffect, useState, useMemo } from "react";
import { IFilter } from "../IFilter";
import { CachedTablesContext } from "../model/CacheProvider";
import { getDisplayNamesForLanguage } from "../model/Language";
import {
getDisplayNamesForLanguage,
kTagForNoLanguage,
} from "../model/Language";
import {
useSearchBooks,
IBasicBookInfo,
Expand All @@ -11,7 +14,6 @@ import {
bookHasFeatureInLanguage,
featureIsLanguageDependent,
} from "./FeatureHelper";
import { useIntl } from "react-intl";
import { BookOrderingScheme, ICollection } from "../model/ContentInterfaces";
import { doExpensiveClientSideSortingIfNeeded } from "../connection/sorting";
import { DefaultBookComparisonKeyIgnoringLanguages } from "../model/DuplicateBookFilter";
Expand Down Expand Up @@ -94,11 +96,6 @@ export function useGetLanguagesWithTheseBooks(
// We need to wait until we have divided into language groups since each group will have its own title language.
// collection.orderingScheme
);
const l10n = useIntl();
const unknown = l10n.formatMessage({
id: "language.unknown",
defaultMessage: "Unknown",
});
// The combination of useRef and useEffect allows us to run the search once
// const rows = useRef<Map<string, { phash: string; book: IBasicBookInfo }>>(
const [langTagToBooks, setLangTagToBooks] = useState(
Expand All @@ -118,9 +115,10 @@ export function useGetLanguagesWithTheseBooks(
// So the first book (in each set with the same comparisonKey) that has a particular language wins.
// eslint-disable-next-line no-loop-func
searchResults.books.forEach((book) => {
let foundOneLanguage = false;
let isBookAdded = false;
const key = DefaultBookComparisonKeyIgnoringLanguages(book);
const addBookToLang = (langCode: string) => {
isBookAdded = true;
if (!mapOfLangToBookArray.get(langCode)) {
mapOfLangToBookArray.set(langCode, []);
}
Expand All @@ -147,33 +145,55 @@ export function useGetLanguagesWithTheseBooks(
if (langCode) {
// When filtering by feature, a book only gets added to the list for a given language
// if it has the feature IN THAT LANGUAGE (BL-9257)
if (
needLangCheck &&
!bookHasFeatureInLanguage(
book.features,
filter.feature!,
langCode
)
) {
continue; // don't want this book in this language list.
if (needLangCheck) {
if (
!bookHasFeatureInLanguage(
book.features,
filter.feature!,
langCode
)
) {
continue; // don't want this book in this language list.
}
}
foundOneLanguage = true;
addBookToLang(langCode);
}
}
if (!foundOneLanguage) {
// book may have been uploaded without user setting the collection language for sign language.
// (Note: Not really the case anymore. Uploading as a sign language book requires the SL language code to be set,
// in both Bloom editor and in BulkUpload. However, still could be possible for the SL feature to be manually added
// after the fact, and if you don't specify the lang code (and the Staff Panel doesn't let you...),
// that'd lead to this case. [JS])
// We still want it to show up somewhere in the sign language collection page.
// Since this key is only used locally in this function, it doesn't matter that it's not a valid
// three-letter code; in fact, that ensures it won't conflict with a real one.
// We don't know how this can happen for any feature other than sign language, but
// there are a few cases where it does, so we decided to allow it for all of them
// so at least every book with the feature gets listed somehow.
addBookToLang("unknown");
if (!isBookAdded) {
// This handles two very distinct use cases.
//
// 1. The book has no language at all. This is what we refer to in the UI as a "Picture Book (no text)".
// In a context where we are listing out the languages (by-language-card/by-language-group),
// this will get its own "language" card/group. The query logic knows how to count and display these.
if (!book.languages || book.languages.length === 0)
addBookToLang(kTagForNoLanguage);
// 2. The book has one or more languages, but none of those match the language-specific feature
// we are displaying.
//
// Book may have been uploaded without user setting the collection language for sign language.
// (Note: Not really the case anymore. Uploading as a sign language book requires the SL language code to be set,
// in both Bloom editor and in BulkUpload. However, still could be possible for the SL feature to be manually added
// after the fact, and if you don't specify the lang code (and the Staff Panel doesn't let you...),
// that'd lead to this case. [JS])
// We still want it to show up somewhere in the sign language collection page.
// We don't know how this can happen for any feature other than sign language, but
// there are a few cases where it does, so we decided to allow it for all of them
// so at least every book with the feature gets listed somehow.
//
// When working on this Oct 2023, I (AP) discovered there are many (43) talking books
// with a mismatch of language feature tags. Some are simply missing the language tag
// for some of the languages in the book. For example, a book with English audio/text,
// Tok Pisin audio/text, and Hakö text was only tagged as having Hakö though the feature
// array was correctly set as talkingBook, talkingBook:en, talkingBook:tpi.
//
// Note, this was originally implemented with an "Unknown" language group at the end,
// but that didn't play well with the by-language-card layout/logic. Specifically, there
// is no (reasonable) way for the query logic to count or query for this special case directly.
// So now we just add it to *some* language group.
else
addBookToLang(
book.lang1Tag || book.languages[0].isoCode
);
}
});
sortBooksWithinEachLanguageRow(
Expand Down Expand Up @@ -213,13 +233,13 @@ export function useGetLanguagesWithTheseBooks(
)
);
result.push({
name: unknown,
isoCode: "unknown",
name: "", // Doesn't matter; it won't show up anywhere. Language.ts has code to get the right label.
isoCode: kTagForNoLanguage,
usageCount: 0,
objectId: "",
});
return result;
}, [languagesByBookCount, excludeLanguages, unknown]);
}, [languagesByBookCount, excludeLanguages]);
return {
waiting,
languagesWithTheseBooks: useMemo(
Expand Down
23 changes: 16 additions & 7 deletions src/connection/LibraryQueryHooks.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,11 @@ import { retrieveBookData, retrieveBookStats } from "./LibraryQueries";
import { Book, createBookFromParseServerData } from "../model/Book";
import { useContext, useMemo, useEffect, useState } from "react";
import { CachedTablesContext } from "../model/CacheProvider";
import { getCleanedAndOrderedLanguageList, ILanguage } from "../model/Language";
import {
getCleanedAndOrderedLanguageList,
ILanguage,
kTagForNoLanguage,
} from "../model/Language";
import { processRegExp } from "../Utilities";
import { kTopicList } from "../model/ClosedVocabularies";
import {
Expand Down Expand Up @@ -1335,12 +1339,17 @@ export function constructParseBookQuery(
// if f.language is set, add the query needed to restrict books to those with that language
if (f.language != null) {
delete params.where.language; // remove that, we need to make it more complicated because we need a join.
params.where.langPointers = {
$inQuery: {
where: { isoCode: f.language },
className: "language",
},
};

if (f.language === kTagForNoLanguage) {
params.where.langPointers = { $eq: [] };
} else {
params.where.langPointers = {
$inQuery: {
where: { isoCode: f.language },
className: "language",
},
};
}
}
// topic is handled below. This older version is not compatible with the possibility of other topics.
// Hopefully the old style really is gone. Certainly any update inserts topic:
Expand Down
4 changes: 0 additions & 4 deletions src/localization/Code Strings.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -473,10 +473,6 @@
"description": "",
"message": "If you continue, this version of the book <em>{title}</em> will be removed from BloomLibrary.org. There is no way to undo this except by uploading it again."
},
"language.unknown": {
"description": "Heading in a list of languages for a collection of books that have a particular feature, but we don't know which language(s) actually support the feature",
"message": "Unknown"
},
"downloadBook": {
"message": "Download Book"
},
Expand Down
3 changes: 2 additions & 1 deletion src/model/DuplicateBookFilter.ts
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { IBasicBookInfo } from "../connection/LibraryQueryHooks";
import { getBookTitleInLanguageOrUndefined } from "./Book";
import { kTagForNoLanguage } from "./Language";

// eventually we expect there to be more than one of these, and
// for Contentful to be able to specify which one to use, hence the use of the string.
Expand Down Expand Up @@ -44,7 +45,7 @@ export function PreferBooksWithL1MatchingFocusLanguage_DuplicateBookFilter(
book,
languageInFocus
);
if (!titleInContextLang) {
if (!titleInContextLang && languageInFocus !== kTagForNoLanguage) {
continue; // just skip it. There are surprisingly many books that have some English but don't have the title in English. E.g. 6jFUJ8jeEv
}
hash += (titleInContextLang ?? "").toLowerCase();
Expand Down
27 changes: 27 additions & 0 deletions src/model/Language.ts
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { getTranslation } from "../localization/GetLocalizations";

export interface ILanguage {
name: string;
isoCode: string;
Expand All @@ -7,6 +9,9 @@ export interface ILanguage {
objectId: string;
}

export const kTagForNoLanguage = "none";
const kPsuedoLanguageTags = [kTagForNoLanguage];

export function getDisplayNamesFromLanguageCode(
languageCode: string,
languages: ILanguage[]
Expand All @@ -17,6 +22,14 @@ export function getDisplayNamesFromLanguageCode(
combined: string;
}
| undefined {
if (kPsuedoLanguageTags.includes(languageCode))
return getDisplayNamesForLanguage({
name: "",
isoCode: languageCode,
usageCount: 0,
objectId: "",
});

const language = languages.find((l) => l.isoCode === languageCode);
if (language) return getDisplayNamesForLanguage(language);
return undefined;
Expand Down Expand Up @@ -49,6 +62,20 @@ export function getDisplayNamesForLanguage(
combined: "Chinese (简体中文)",
};
}

// Handle picture books
if (language.isoCode === kTagForNoLanguage) {
const localizedLabel = getTranslation(
"book.detail.pictureBook",
"Picture Book (no text)"
);
return {
primary: localizedLabel,
secondary: localizedLabel,
combined: localizedLabel,
};
}

// this may not be needed for long. We are working on some fixes in BL-11754
if (language.isoCode === "ase-ML") {
return navigator.language.startsWith("fr")
Expand Down

0 comments on commit 3b689f4

Please sign in to comment.