Merge pull request #9902 from cdrini/feature/toc-authors-etc

Add authors, subtitle, and description to TOC on books page

openlibrary/core/lists/model.py

@@ -14,7 +14,7 @@
 
 from openlibrary.core import helpers as h
 from openlibrary.core import cache
-from openlibrary.core.models import Image, Subject, Thing, ThingKey
+from openlibrary.core.models import Image, Subject, Thing, ThingKey, ThingReferenceDict
 from openlibrary.plugins.upstream.models import Author, Changeset, Edition, User, Work
 
 from openlibrary.plugins.worksearch.search import get_solr
@@ -24,10 +24,6 @@
 logger = logging.getLogger("openlibrary.lists.model")
 
 
-class ThingReferenceDict(TypedDict):
-    key: ThingKey
-
-
 SeedSubjectString = str
 """
 When a subject is added to a list, it's added as a string like:

openlibrary/core/models.py

@@ -8,7 +8,7 @@
 import web
 import json
 import requests
-from typing import Any
+from typing import Any, TypedDict
 from collections import defaultdict
 from dataclasses import dataclass, field
 
@@ -219,6 +219,10 @@ def _get_d(self):
         }
 
 
+class ThingReferenceDict(TypedDict):
+    key: ThingKey
+
+
 class Edition(Thing):
     """Class to represent /type/edition objects in OL."""
 

openlibrary/macros/TableOfContents.html

@@ -1,31 +1,38 @@
-$def with (table_of_contents, ocaid=None, highlighting=False, cls='', attrs='')
+$def with (table_of_contents, ocaid=None, cls='', attrs='')
 
 $ min_level = min(chapter.level for chapter in table_of_contents)
 <div class="toc $cls" $:attrs>
   $for chapter in table_of_contents:
-    $ is_link = ocaid and chapter.pagenum and chapter.pagenum.isdigit()
-    $ tag = 'a' if is_link else 'div'
-    <$tag
+    <div
       class="toc__entry"
-      $:cond(is_link, 'href="//archive.org/details/%s/page/%s"' % (ocaid, chapter.pagenum))
-      $:cond(is_link, 'data-ol-link-track="BookPage|TOCClick"')
       data-level="$chapter.level"
       style="margin-left:$((chapter.level - min_level) * 2)ch"
     >
-      <span class="toc__name">
-        $ label = chapter.label
-        $if label and not label.endswith('.'):
-          $ label = label.strip() + '. '
-        $if highlighting:
-          $# This isn't html injection, because solr returns everything already html escaped except for the em of the highlight
-          $:label
-          $:chapter.title
-        $else:
-          $label
-          $chapter.title
-      </span>
-      $if chapter.pagenum:
-        <span class="toc__dots" style="flex:1; border-bottom: 1px dotted;"></span>
-        <span class="toc__pagenum">$_('Page %s', chapter.pagenum)</span>
-    </$tag>
+      $ is_link = ocaid and chapter.pagenum and chapter.pagenum.isdigit()
+      $ tag = 'a' if is_link else 'div'
+      <$tag
+        class="toc__main"
+        $:cond(is_link, 'href="//archive.org/details/%s/page/%s"' % (ocaid, chapter.pagenum))
+        $:cond(is_link, 'data-ol-link-track="BookPage|TOCClick"')
+      >
+        <div class="toc__name">
+          $ label = chapter.label
+          $if label and not label.endswith('.'):
+            $ label = label.strip() + '. '
+
+          <div class="toc__title">$label $chapter.title</div>
+
+          $if chapter.subtitle:
+            <div class="toc__subtitle">$chapter.subtitle</div>
+          $if chapter.authors:
+            <div class="toc__authors">$:macros.BookByline(chapter.authors)</div>
+        </div>
+        $if chapter.pagenum:
+          <span class="toc__dots"></span>
+          <span class="toc__pagenum">$_('Page %s', chapter.pagenum)</span>
+      </$tag>
+
+      $if chapter.description:
+        <div class="toc__description">$chapter.description</div>
+    </div>
 </div>

openlibrary/plugins/upstream/models.py

@@ -17,6 +17,7 @@
 from openlibrary.core.models import Image
 from openlibrary.core import lending
 
+from openlibrary.plugins.upstream.table_of_contents import TocEntry
 from openlibrary.plugins.upstream.utils import MultiDict, parse_toc, get_edition_config
 from openlibrary.plugins.upstream import account
 from openlibrary.plugins.upstream import borrow
@@ -414,24 +415,18 @@ def format_row(r):
 
         return "\n".join(format_row(r) for r in self.get_table_of_contents())
 
-    def get_table_of_contents(self):
+    def get_table_of_contents(self) -> list[TocEntry]:
         def row(r):
             if isinstance(r, str):
-                level = 0
-                label = ""
-                title = r
-                pagenum = ""
+                return TocEntry(level=0, title=r)
             else:
-                level = safeint(r.get('level', '0'), 0)
-                label = r.get('label', '')
-                title = r.get('title', '')
-                pagenum = r.get('pagenum', '')
+                return TocEntry.from_dict(r)
 
-            r = web.storage(level=level, label=label, title=title, pagenum=pagenum)
-            return r
-
-        d = [row(r) for r in self.table_of_contents]
-        return [row for row in d if any(row.values())]
+        return [
+            toc_entry
+            for r in self.table_of_contents
+            if not (toc_entry := row(r)).is_empty()
+        ]
 
     def set_toc_text(self, text):
         self.table_of_contents = parse_toc(text)

openlibrary/plugins/upstream/table_of_contents.py

@@ -0,0 +1,40 @@
+from dataclasses import dataclass
+from typing import TypedDict
+
+from openlibrary.core.models import ThingReferenceDict
+
+
+class AuthorRecord(TypedDict):
+    name: str
+    author: ThingReferenceDict | None
+
+
+@dataclass
+class TocEntry:
+    level: int
+    label: str | None = None
+    title: str | None = None
+    pagenum: str | None = None
+
+    authors: list[AuthorRecord] | None = None
+    subtitle: str | None = None
+    description: str | None = None
+
+    @staticmethod
+    def from_dict(d: dict) -> 'TocEntry':
+        return TocEntry(
+            level=d.get('level', 0),
+            label=d.get('label'),
+            title=d.get('title'),
+            pagenum=d.get('pagenum'),
+            authors=d.get('authors'),
+            subtitle=d.get('subtitle'),
+            description=d.get('description'),
+        )
+
+    def is_empty(self) -> bool:
+        return all(
+            getattr(self, field) is None
+            for field in self.__annotations__
+            if field != 'level'
+        )

static/css/components/toc.less

@@ -2,28 +2,67 @@
 @import (reference) "../less/colors.less";
 
 .toc__entry {
+  line-height: 1.2em;
+
   border-radius: 4px;
   padding: 3px 8px;
   @media only screen and (hover: none) {
-    padding: 6px 8px; /* Increase padding for touch-only devices */
+    padding: 8px; /* Increase padding for touch-only devices */
+  }
+
+  .toc__main {
+    display: flex;
+    gap: 4px;
+    position: relative;
+  }
+
+  .toc__dots {
+    flex: 1;
+    border-bottom: 1px dotted;
+    height: 1.2em;
+  }
+
+  .toc__subtitle {
+    font-style: oblique;
+    color: @accessible-grey;
   }
-  display: flex;
-  gap: 4px;
-  line-height: 1.2em;
-  transition: background-color .2s;
 
-  em {
-    font-weight: bold;
+  .toc__subtitle, .toc__authors, .toc__description {
+    font-size: @font-size-label-large;
+    text-decoration: none;
+  }
+
+  .toc__description {
+    margin-top: 5px;
   }
 }
 
-a.toc__entry {
+a.toc__main {
   text-decoration: none;
-  .toc__name {
+  border-radius: 4px;
+
+  .toc__title {
     text-decoration: underline;
   }
-  &:hover {
-    background: rgba(0, 124, 255, .2);
+
+  &:hover:after {
+    display: block;
+    content: "";
+    inset: -2px;
+    position: absolute;
+    border-radius: 4px;
+    pointer-events: none;
+    animation: fade-in .2s;
+    background-color: rgba(0, 124, 255, .15);
+  }
+}
+
+@keyframes fade-in {
+  from {
+    background-color: rgba(0, 124, 255, 0);
+  }
+  to {
+    background-color: rgba(0, 124, 255, .15);
   }
 }
 
@@ -33,6 +72,10 @@ a.toc__entry {
 
 @media (max-width: @width-breakpoint-mobile) {
   .toc__entry {
+    padding: 8px 0;
+  }
+
+  .toc__main {
     flex-direction: column;
     gap: 0;
   }
@@ -42,7 +85,7 @@ a.toc__entry {
   }
 
   .toc__pagenum {
-    font-size: .9em;
+    font-size: @font-size-label-large;
     color: @accessible-grey;
   }
 }