Skip to content

Commit a8dc75d

Browse files
Ensure "Title Missing" shows for appropriate books (BL-16027)
1 parent db30093 commit a8dc75d

8 files changed

Lines changed: 231 additions & 64 deletions

File tree

src/BloomBrowserUI/collectionsTab/BooksOfCollection.tsx

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import Grid from "@mui/material/Grid";
44
import * as React from "react";
55
import { useState, useEffect } from "react";
66
import "./BooksOfCollection.less";
7-
import { useApiData, useWatchApiData } from "../utils/bloomApi";
7+
import { post, useApiData, useWatchApiData } from "../utils/bloomApi";
88
import {
99
BookButton,
1010
bookButtonHeight,
@@ -86,6 +86,17 @@ export const BooksOfCollection: React.FunctionComponent<{
8686
setReloadParameter("");
8787
}, [unfilteredBooks, props.filter]);
8888

89+
// Notify the backend that the collection pane is ready to receive book label updates.
90+
// Using a ref callback so this happens after the DOM is rendered, without needing useEffect.
91+
const collectionPaneRef = React.useCallback(
92+
(node: HTMLDivElement | null) => {
93+
if (node && props.isEditableCollection && books.length > 0) {
94+
post("collections/collectionPaneReady");
95+
}
96+
},
97+
[props.isEditableCollection, books.length],
98+
);
99+
89100
//const selectedBookInfo = useMonitorBookSelection();
90101
const collection: ICollection = useApiData(
91102
`collections/collectionProps?${collectionQuery}`,
@@ -123,6 +134,7 @@ export const BooksOfCollection: React.FunctionComponent<{
123134
key={"BookCollection-" + props.collectionId}
124135
className="bookButtonPane"
125136
style={{ cursor: "context-menu" }}
137+
ref={collectionPaneRef}
126138
>
127139
{books.length > 0 && (
128140
<Grid

src/BloomExe/Book/Book.cs

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -328,15 +328,7 @@ public virtual string TitleBestForUserDisplay
328328
/// </remarks>
329329
public virtual string NameBestForUserDisplay
330330
{
331-
get
332-
{
333-
if (BookInfo.FileNameLocked)
334-
{
335-
// The user has explicitly chosen a name to use for the book, distinct from its titles.
336-
return Path.GetFileName(FolderPath);
337-
}
338-
return TitleBestForUserDisplay;
339-
}
331+
get { return BookInfo.GetBestDisplayTitle(CollectionSettings, this); }
340332
}
341333

342334
/// <summary>

src/BloomExe/Book/BookInfo.cs

Lines changed: 95 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
using System.Threading;
1010
using System.Windows.Forms;
1111
using Bloom.Api;
12+
using Bloom.Collection;
1213
using Bloom.Edit;
1314
using Bloom.Utils;
1415
using L10NSharp;
@@ -341,6 +342,8 @@ public string Copyright
341342
/// </summary>
342343
public string QuickTitleUserDisplay => FolderName;
343344

345+
public string ThumbnailLabel;
346+
344347
public bool TryGetPremadeThumbnail(out Image image)
345348
{
346349
string path = Path.Combine(FolderPath, "thumbnail.png");
@@ -851,29 +854,104 @@ internal string GetBestTitleForUserDisplay(
851854
List<string> langCodes,
852855
bool ignoreFolderName = false
853856
)
857+
{
858+
return GetBestDisplayTitle(null, null, langCodes, ignoreFolderName);
859+
}
860+
861+
internal string GetBestDisplayTitle(
862+
CollectionSettings settings, // may be null
863+
Book book, // may be null, but if we have it, we can use it to get the title without having to read the html again
864+
List<string> langCodes = null,
865+
bool ignoreFolderName = false
866+
)
854867
{
855868
if (!ignoreFolderName && FileNameLocked)
856-
return FolderName;
857-
try
858869
{
859-
// JSON parsing requires newlines to be double quoted with backslashes inside string values.
860-
var jsonString =
861-
AllTitles == null ? "{}" : AllTitles.Replace("\r", "\\r").Replace("\n", "\\n");
862-
dynamic titles = DynamicJson.Parse(jsonString);
863-
IEnumerable<string> langs = titles.GetDynamicMemberNames();
864-
var multiText = new MultiTextBase();
865-
// I have no idea why "item" gets included...it's never a language id, so never in the json...
866-
// but sometimes it does, and then titles["item"] throws, and this method does not
867-
// behave as expected.
868-
foreach (var lang in langs.Where((l) => l != "item"))
869-
multiText[lang] = titles[lang].Trim();
870-
return Book.GetBestTitleForDisplay(multiText, langCodes, IsInEditableCollection);
870+
// The user has explicitly chosen a name to use for the book, distinct from its titles.
871+
return FolderName;
871872
}
872-
catch (Exception e)
873+
if (String.IsNullOrEmpty(Title))
873874
{
874-
Console.WriteLine(e);
875+
if (IsInEditableCollection)
876+
{
877+
// If we have the book, we can use that for getting the title.
878+
// If we have the settings but not the book, we can use the settings to get the title from the html.
879+
if (!string.IsNullOrEmpty(FolderPath) && (book != null || settings != null))
880+
{
881+
// Use the loaded book if we have it to get the best title.
882+
if (
883+
FolderPath == book?.FolderPath
884+
&& book.BookInfo.SaveContext == SaveContext
885+
)
886+
{
887+
if (langCodes == null)
888+
langCodes = book.BookData.GetBasicBookLanguageCodes().ToList();
889+
var best = Book.GetBestTitleForDisplay(
890+
book.BookData.GetMultiTextVariableOrEmpty("bookTitle"),
891+
langCodes,
892+
IsInEditableCollection
893+
);
894+
if (!string.IsNullOrEmpty(best))
895+
return best;
896+
}
897+
else
898+
{
899+
// If we can create a BookData, we can get the title from there.
900+
var htmlPath = BookStorage.FindBookHtmlInFolder(FolderPath);
901+
if (
902+
!string.IsNullOrEmpty(htmlPath)
903+
&& RobustFile.Exists(htmlPath)
904+
&& settings != null
905+
)
906+
{
907+
var dom = HtmlDom.CreateFromHtmlFile(htmlPath);
908+
var bookData = new BookData(dom, settings, null);
909+
if (langCodes == null)
910+
langCodes = bookData.GetBasicBookLanguageCodes().ToList();
911+
var best = Book.GetBestTitleForDisplay(
912+
bookData.GetMultiTextVariableOrEmpty("bookTitle"),
913+
langCodes,
914+
IsInEditableCollection
915+
);
916+
if (!string.IsNullOrEmpty(best))
917+
return best;
918+
}
919+
}
920+
}
921+
}
922+
// If we still don't have a title, try to get one from the metadata.
923+
// This code is used for all books that are not in the editable collection, and for any books
924+
// in the editable collection that don't seem to have any title in the HTML. (The latter case
925+
// may be hopeless, but this check isn't very expensive.)
926+
try
927+
{
928+
// JSON parsing requires newlines to be double quoted with backslashes inside string values.
929+
var jsonString =
930+
AllTitles == null
931+
? "{}"
932+
: AllTitles.Replace("\r", "\\r").Replace("\n", "\\n");
933+
dynamic titles = DynamicJson.Parse(jsonString);
934+
IEnumerable<string> langs = titles.GetDynamicMemberNames();
935+
var multiText = new MultiTextBase();
936+
// I have no idea why "item" gets included...it's never a language id, so never in the json...
937+
// but sometimes it does, and then titles["item"] throws, and this method does not
938+
// behave as expected.
939+
foreach (var lang in langs.Where((l) => l != "item"))
940+
multiText[lang] = titles[lang].Trim();
941+
if (langCodes == null)
942+
langCodes = langs.Where((l) => l != "item").ToList();
943+
return Book.GetBestTitleForDisplay(
944+
multiText,
945+
langCodes,
946+
IsInEditableCollection
947+
);
948+
}
949+
catch (Exception e)
950+
{
951+
Console.WriteLine(e);
952+
}
875953
}
876-
return Title;
954+
return Title; // Title may be empty, but we have no better option at this point.
877955
}
878956

879957
private static void SafelyAddToIdSet(

src/BloomExe/Collection/BookCollection.cs

Lines changed: 50 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,11 @@ public enum CollectionType
2424
SourceCollection,
2525
}
2626

27-
public delegate BookCollection Factory(string path, CollectionType collectionType); //autofac uses this
27+
public delegate BookCollection Factory(
28+
string path,
29+
CollectionType collectionType,
30+
CollectionSettings collectionSettings = null
31+
); //autofac uses this
2832

2933
public EventHandler CollectionChanged;
3034

@@ -37,6 +41,8 @@ public enum CollectionType
3741
private static HashSet<string> _changingFolders = new HashSet<string>();
3842
private BloomWebSocketServer _webSocketServer;
3943

44+
private CollectionSettings _collectionSettings;
45+
4046
public static event EventHandler CollectionCreated;
4147

4248
//for moq only
@@ -53,13 +59,15 @@ public BookCollection(
5359
CollectionType collectionType,
5460
BookSelection bookSelection,
5561
TeamCollectionManager tcm = null,
62+
CollectionSettings collectionSettings = null,
5663
BloomWebSocketServer webSocketServer = null
5764
)
5865
{
5966
_path = path;
6067
_bookSelection = bookSelection;
6168
_tcManager = tcm;
6269
_webSocketServer = webSocketServer;
70+
_collectionSettings = collectionSettings;
6371

6472
Type = collectionType;
6573

@@ -325,29 +333,35 @@ public string PathToDirectory
325333

326334
public void UpdateBookInfo(BookInfo info)
327335
{
328-
var oldIndex = _bookInfos.FindIndex(i => i.Id == info.Id);
329-
IComparer<string> comp = new NaturalSortComparer<string>();
330-
var newKey = Path.GetFileName(info.FolderPath);
331-
if (oldIndex >= 0)
336+
lock (_bookInfoLock)
332337
{
333-
// optimize: very often the new one will belong at the same index,
334-
// if that's the case we could just replace.
335-
_bookInfos.RemoveAt(oldIndex);
336-
}
338+
var oldIndex = _bookInfos.FindIndex(i => i.Id == info.Id);
339+
IComparer<string> comp = new NaturalSortComparer<string>();
340+
var newKey = Path.GetFileName(info.FolderPath);
341+
if (oldIndex >= 0)
342+
{
343+
// optimize: very often the new one will belong at the same index,
344+
// if that's the case we could just replace.
345+
_bookInfos.RemoveAt(oldIndex);
346+
}
337347

338-
int newIndex = _bookInfos.FindIndex(x =>
339-
comp.Compare(newKey, Path.GetFileName(x.FolderPath)) <= 0
340-
);
341-
if (newIndex < 0)
342-
newIndex = _bookInfos.Count;
343-
_bookInfos.Insert(newIndex, info);
348+
int newIndex = _bookInfos.FindIndex(x =>
349+
comp.Compare(newKey, Path.GetFileName(x.FolderPath)) <= 0
350+
);
351+
if (newIndex < 0)
352+
newIndex = _bookInfos.Count;
353+
_bookInfos.Insert(newIndex, info);
354+
}
344355
NotifyCollectionChanged();
345356
}
346357

347358
public void AddBookInfo(BookInfo bookInfo)
348359
{
349-
_bookInfos.Add(bookInfo);
350-
NotifyCollectionChanged();
360+
lock (_bookInfoLock)
361+
{
362+
_bookInfos.Add(bookInfo);
363+
NotifyCollectionChanged();
364+
}
351365
}
352366

353367
/// <summary>
@@ -356,22 +370,25 @@ public void AddBookInfo(BookInfo bookInfo)
356370
/// <param name="bookInfo"></param>
357371
public void InsertBookInfo(BookInfo bookInfo)
358372
{
359-
IComparer<string> comparer = new NaturalSortComparer<string>();
360-
for (int i = 0; i < _bookInfos.Count; i++)
373+
lock (_bookInfoLock)
361374
{
362-
var compare = comparer.Compare(_bookInfos[i].FolderPath, bookInfo.FolderPath);
363-
if (compare == 0)
375+
IComparer<string> comparer = new NaturalSortComparer<string>();
376+
for (int i = 0; i < _bookInfos.Count; i++)
364377
{
365-
_bookInfos[i] = bookInfo; // Replace
366-
return;
367-
}
368-
if (compare > 0)
369-
{
370-
_bookInfos.Insert(i, bookInfo);
371-
return;
378+
var compare = comparer.Compare(_bookInfos[i].FolderPath, bookInfo.FolderPath);
379+
if (compare == 0)
380+
{
381+
_bookInfos[i] = bookInfo; // Replace
382+
return;
383+
}
384+
if (compare > 0)
385+
{
386+
_bookInfos.Insert(i, bookInfo);
387+
return;
388+
}
372389
}
390+
_bookInfos.Add(bookInfo);
373391
}
374-
_bookInfos.Add(bookInfo);
375392
}
376393

377394
private bool BackupFileExists(string folderPath)
@@ -412,7 +429,10 @@ private void AddBookInfo(string folderPath)
412429
)
413430
? _bookSelection.CurrentSelection.BookInfo
414431
: new BookInfo(folderPath, editable, sc);
415-
432+
bookInfo.ThumbnailLabel = bookInfo.GetBestDisplayTitle(
433+
_collectionSettings,
434+
_bookSelection.CurrentSelection
435+
);
416436
_bookInfos.Add(bookInfo);
417437
}
418438
catch (Exception e)

src/BloomExe/CollectionTab/CollectionModel.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -360,7 +360,8 @@ private IEnumerable<BookCollection> GetBookCollectionsOnce()
360360
{
361361
editableCollection = _bookCollectionFactory(
362362
_pathToCollection,
363-
BookCollection.CollectionType.TheOneEditableCollection
363+
BookCollection.CollectionType.TheOneEditableCollection,
364+
_collectionSettings
364365
);
365366
if (_bookCollectionHolder != null)
366367
_bookCollectionHolder.TheOneEditableCollection = editableCollection;

0 commit comments

Comments
 (0)