diff --git a/geniza/entities/models.py b/geniza/entities/models.py index 42302bad8..d720fffbc 100644 --- a/geniza/entities/models.py +++ b/geniza/entities/models.py @@ -253,6 +253,9 @@ class Person(TrackChangesModel): ) gender = models.CharField(max_length=1, choices=GENDER_CHOICES) + # minimum documents to show a page if has_page is False + MIN_DOCUMENTS = 10 + class Meta: verbose_name_plural = "People" @@ -279,7 +282,10 @@ def __str__(self): def get_absolute_url(self): """url for this person""" - return reverse("entities:person", args=[self.slug]) + if self.documents.count() >= self.MIN_DOCUMENTS or self.has_page == True: + return reverse("entities:person", args=[str(self.slug)]) + else: + return None def merge_with(self, merge_people, user=None): """Merge the specified people into this one. Combines all metadata diff --git a/geniza/entities/templates/entities/person_list.html b/geniza/entities/templates/entities/person_list.html new file mode 100644 index 000000000..907a8f435 --- /dev/null +++ b/geniza/entities/templates/entities/person_list.html @@ -0,0 +1,117 @@ +{% extends 'base.html' %} +{% load static i18n humanize %} + +{% block meta_title %}{{ page_title }}{% endblock meta_title %} +{% block meta_description %}{{ page_description }}{% endblock meta_description %} + +{% block main %} +

{{ page_title }}

+
+

+ {# Translators: number of search results #} + {% blocktranslate with count_humanized=paginator.count|intcomma count counter=paginator.count trimmed %} + 1 result + {% plural %} + {{ count_humanized }} results + {% endblocktranslate %} +

+ {# list view table #} + + + {# Translators: Person "name" column header on the browse page #} + + {# Translators: Person "dates of activity" column header on the browse page #} + {% comment %} {% endcomment %} + {# Translators: Person "gender" column header on the browse page #} + + {# Translators: Person "social role" column header on the browse page #} + + + + + + + {% for person in people %} + + + {% comment %} {% endcomment %} + + + + + + + {% endfor %} + +
{% translate "Name" %}{% translate "Dates" %}{% translate "Gender" %}{% translate "Social role" %} + + {# Translators: Person "document count" column header on the browse page #} + {% translate "Number of related documents" %} + + + {# Translators: Person "related people count" column header on the browse page #} + {% translate "Number of related people" %} + + + {# Translators: Person "related place count" column header on the browse page #} + {% translate "Number of related places" %} +
+ {% if person.get_absolute_url %} + {{ person }} + {% else %} + {{ person }} + {% endif %} + {{ person.documents_date_range }}{{ person.get_gender_display }}{{ person.role }}{{ person.documents.count }}{{ person.relationships.count }}{{ person.personplacerelation_set.count }}
+ {# grid / mobile view #} +
+ {% for person in people %} +
+ {# Translators: accessible label for section showing metadata like name, gender, etc #} +
+
{% translate "Name" %}
+
+ {% if person.get_absolute_url %} + {{ person }} + {% else %} + {{ person }} + {% endif %} +
+
{% translate "Gender" %}
+
{{ person.get_gender_display }}
+ {% comment %}
{% translate 'Dates' %}
{% endcomment %} +
{% translate "Social role" %}
+
{{ person.role }}
+ {# Translators: label for person description / bio #} +
{% translate "Description / Bio" %}
+
{{ person.description|truncatewords:15 }}
+
+ {# Translators: accessible label for section showing counts of entries related to an entity #} +
+
{% translate "Number of related people" %}
+
+ + {{ person.relationships.count }} +
+
{% translate "Number of related places" %}
+
+ + {{ person.personplacerelation_set.count }} +
+
{% translate "Number of related documents" %}
+
+ + {{ person.documents.count }} +
+
+
+ {% endfor %} +
+
+
+ {# Translators: range of search results on the current page, out of total #} + {% blocktranslate with start=page_obj.start_index end=page_obj.end_index count_humanized=paginator.count|intcomma %} +
{{ start }} – {{ end }} of {{ count_humanized }}
+ {% endblocktranslate %} + {% include "corpus/snippets/pagination.html" %} +
+{% endblock %} diff --git a/geniza/entities/templates/entities/snippets/person_header.html b/geniza/entities/templates/entities/snippets/person_header.html index 94fb675a2..3f8d20488 100644 --- a/geniza/entities/templates/entities/snippets/person_header.html +++ b/geniza/entities/templates/entities/snippets/person_header.html @@ -1,5 +1,12 @@ {% load i18n %} +
+ + {# Translators: Link to return to the list of people from a person page #} + {% translate "Back to People" %} + +
+ {# title #}

{{ person }}

@@ -8,8 +15,7 @@

{{ person }}

- {# Translators: Person edit link for admins #} - {% translate 'Edit' %} + Edit
{% endif %} diff --git a/geniza/entities/tests/test_entities_models.py b/geniza/entities/tests/test_entities_models.py index a414729bb..d8f0401fb 100644 --- a/geniza/entities/tests/test_entities_models.py +++ b/geniza/entities/tests/test_entities_models.py @@ -320,8 +320,19 @@ def test_merge_with_log_entries(self): ) def test_get_absolute_url(self): - # should get person page url in user's language by slug + # should not have an absolute url if has_page is false and < MIN_DOCUMENTS associated docs person = Person.objects.create() + assert person.get_absolute_url() == None + + # has_page is true, should get the url in user's language by slug + person.has_page = True + assert person.get_absolute_url() == "/en/people/%s/" % person.slug + + # has_page is false but has >= MIN_DOCUMENTS, should get url + person.has_page = False + for _ in range(Person.MIN_DOCUMENTS): + d = Document.objects.create() + person.documents.add(d) assert person.get_absolute_url() == "/en/people/%s/" % person.slug def test_save(self): diff --git a/geniza/entities/tests/test_entities_views.py b/geniza/entities/tests/test_entities_views.py index 3db929dde..5c89f8209 100644 --- a/geniza/entities/tests/test_entities_views.py +++ b/geniza/entities/tests/test_entities_views.py @@ -11,6 +11,7 @@ from geniza.entities.views import ( PersonAutocompleteView, PersonDetailView, + PersonListView, PersonMerge, PlaceAutocompleteView, ) @@ -193,7 +194,7 @@ def test_get_queryset(self, client): assert response.status_code == 404 # should 200 on person with 10+ associated documents - for _ in range(PersonDetailView.MIN_DOCUMENTS): + for _ in range(Person.MIN_DOCUMENTS): d = Document.objects.create() person.documents.add(d) response = client.get(reverse("entities:person", args=(person.slug,))) @@ -211,11 +212,37 @@ def test_get_context_data(self, client): assert response.context["page_type"] == "person" +@pytest.mark.django_db +class TestPersonListView: + def test_get_queryset(self): + # create people with and without diacritics in their names + p1 = Person.objects.create() + Name.objects.create(name="Example", content_object=p1, primary=True) + p2 = Person.objects.create() + Name.objects.create(name="Ḥalfon b. Menashshe", content_object=p2, primary=True) + p3 = Person.objects.create() + Name.objects.create(name="Zed", content_object=p3, primary=True) + Name.objects.create(name="Apple", content_object=p3, primary=False) + personlist_view = PersonListView() + + # should order diacritics unaccented + qs = personlist_view.get_queryset() + assert qs.first().pk == p1.pk # Example + assert qs[1].pk == p2.pk # Halfon + # should order by primary name only + assert qs[2].pk == p3.pk # Zed + + def test_get_context_data(self, client): + # context should include "page_type": "people" + response = client.get(reverse("entities:person-list")) + assert response.context["page_type"] == "people" + + @pytest.mark.django_db class TestPersonDetailMixin: def test_get(self, client): # should redirect on past slug - person = Person.objects.create() + person = Person.objects.create(has_page=True) name1 = Name.objects.create(name="Imran", content_object=person, primary=True) person.generate_slug() person.save() @@ -230,6 +257,3 @@ def test_get(self, client): response = client.get(reverse("entities:person", args=(old_slug,))) assert response.status_code == 301 assert response.url == person.get_absolute_url() - - # should still raise 404 if conditions aren't met (has_page or MIN_DOCUMENTS) - assert client.get(response.url).status_code == 404 diff --git a/geniza/entities/urls.py b/geniza/entities/urls.py index a5976b4fc..857722e68 100644 --- a/geniza/entities/urls.py +++ b/geniza/entities/urls.py @@ -5,6 +5,7 @@ app_name = "entities" urlpatterns = [ + path("people/", entities_views.PersonListView.as_view(), name="person-list"), path( "people//", entities_views.PersonDetailView.as_view(), diff --git a/geniza/entities/views.py b/geniza/entities/views.py index 97fe9ca55..860b0a4e7 100644 --- a/geniza/entities/views.py +++ b/geniza/entities/views.py @@ -8,7 +8,8 @@ from django.urls import reverse from django.utils.safestring import mark_safe from django.utils.text import Truncator -from django.views.generic import DetailView, FormView +from django.utils.translation import gettext as _ +from django.views.generic import DetailView, FormView, ListView from geniza.entities.forms import PersonMergeForm from geniza.entities.models import PastPersonSlug, Person, Place @@ -129,7 +130,6 @@ class PersonDetailView(PersonDetailMixin): model = Person context_object_name = "person" - MIN_DOCUMENTS = 10 def page_title(self): """page title, for metadata; uses Person primary name""" @@ -149,7 +149,9 @@ def get_queryset(self, *args, **kwargs): doc_count=Count("documents", distinct=True), ) ) - return queryset.filter(Q(doc_count__gte=self.MIN_DOCUMENTS) | Q(has_page=True)) + return queryset.filter( + Q(doc_count__gte=Person.MIN_DOCUMENTS) | Q(has_page=True) + ) def get_context_data(self, **kwargs): """extend context data to add page metadata""" @@ -163,3 +165,38 @@ def get_context_data(self, **kwargs): } ) return context_data + + +class PersonListView(ListView): + model = Person + context_object_name = "people" + template_name = "entities/person_list.html" + # Translators: title of people list/browse page + page_title = _("People") + # Translators: description of people list/browse page + page_description = _("Browse people present in Geniza documents.") + paginate_by = 50 + + def get_queryset(self, *args, **kwargs): + """modify queryset to sort and filter on people in the list""" + # order people by primary name unaccented + return ( + Person.objects.filter(names__primary=True) + .annotate( + name_unaccented=ArrayAgg("names__name__unaccent", distinct=True), + ) + .order_by("name_unaccented") + ) + + def get_context_data(self, **kwargs): + """extend context data to add page metadata""" + context_data = super().get_context_data(**kwargs) + + context_data.update( + { + "page_title": self.page_title, + "page_description": self.page_description, + "page_type": "people", + } + ) + return context_data diff --git a/sitemedia/img/ui/all/all/related-documents-icon.svg b/sitemedia/img/ui/all/all/related-documents-icon.svg new file mode 100644 index 000000000..a20bcbe09 --- /dev/null +++ b/sitemedia/img/ui/all/all/related-documents-icon.svg @@ -0,0 +1,6 @@ + + + diff --git a/sitemedia/img/ui/all/all/related-people-icon.svg b/sitemedia/img/ui/all/all/related-people-icon.svg new file mode 100644 index 000000000..4c909e2de --- /dev/null +++ b/sitemedia/img/ui/all/all/related-people-icon.svg @@ -0,0 +1,6 @@ + + + diff --git a/sitemedia/img/ui/all/all/related-places-icon.svg b/sitemedia/img/ui/all/all/related-places-icon.svg new file mode 100644 index 000000000..ebf063c87 --- /dev/null +++ b/sitemedia/img/ui/all/all/related-places-icon.svg @@ -0,0 +1,6 @@ + + + diff --git a/sitemedia/scss/base/_typography.scss b/sitemedia/scss/base/_typography.scss index 95c85f5fc..52a526dd7 100644 --- a/sitemedia/scss/base/_typography.scss +++ b/sitemedia/scss/base/_typography.scss @@ -16,7 +16,9 @@ $text-size-xl: 1.25rem; // = 20px $text-size-2xl: 1.375rem; // = 22px $text-size-3xl: 1.5rem; // = 24px $text-size-4xl: 1.75rem; // = 28px -$text-size-5xl: 2rem; // = 32px +$text-size-5xl: 1.875rem; // = 30px +$text-size-6xl: 2rem; // = 32px +$text-size-7xl: 2.25rem; // = 36px // Mixins to apply typographic styles to text elements. diff --git a/sitemedia/scss/main.scss b/sitemedia/scss/main.scss index c313379c1..d87f0e0a3 100644 --- a/sitemedia/scss/main.scss +++ b/sitemedia/scss/main.scss @@ -22,6 +22,7 @@ @use "pages/document"; @use "pages/error"; @use "pages/home"; +@use "pages/people"; @use "pages/person"; @use "pages/related"; @use "pages/scholarship"; diff --git a/sitemedia/scss/pages/_people.scss b/sitemedia/scss/pages/_people.scss new file mode 100644 index 000000000..26ee83955 --- /dev/null +++ b/sitemedia/scss/pages/_people.scss @@ -0,0 +1,219 @@ +// ----------------------------------------------------------------------------- +// Person detail page. +// ----------------------------------------------------------------------------- + +@use "../base/breakpoints"; +@use "../base/colors"; +@use "../base/fonts"; +@use "../base/spacing"; +@use "../base/typography"; + +// TODO: standardize typography here once all redesigns are implemented + +main.people * { + max-width: none; + @include breakpoints.for-tablet-landscape-up { + max-width: 56rem; // = 896px + } +} +main.people { + padding: 0 1rem; + @include breakpoints.for-tablet-landscape-up { + padding: 0; + } + h1 { + width: 100%; + font-family: fonts.$primary; + font-size: typography.$text-size-5xl; + @include breakpoints.for-tablet-landscape-up { + font-size: typography.$text-size-7xl; + } + } + // Primary container + section#person-list { + width: 100%; + h2 { + @include breakpoints.for-tablet-landscape-up { + font-size: typography.$text-size-2xl; + } + } + table { + // display table only on desktop; will be ul on mobile + display: none; + @include breakpoints.for-tablet-landscape-up { + display: table; + } + margin-top: spacing.$spacing-sm; + width: 100%; + border-collapse: separate; + border-spacing: 0 spacing.$spacing-xs; + th { + text-align: left; + font-size: typography.$text-size-md; + } + th:first-of-type { + padding-left: spacing.$spacing-md; + } + th:last-of-type { + padding-right: spacing.$spacing-md; + } + th.related { + text-align: center; + min-width: 2rem; + svg { + width: 15px; + height: 16px; + vertical-align: middle; + } + &:last-of-type { + min-width: 3rem; + } + } + tbody tr { + background-color: var(--background-light); + td { + padding: spacing.$spacing-md 0; + font-size: typography.$text-size-lg; + border-top: 1px solid var(--background-gray); + border-bottom: 1px solid var(--background-gray); + } + td:first-of-type { + padding-left: spacing.$spacing-md; + border-left: 1px solid var(--background-gray); + border-top-left-radius: 5px; + border-bottom-left-radius: 5px; + } + td:last-of-type { + padding-right: spacing.$spacing-md; + min-width: 3rem; + border-right: 1px solid var(--background-gray); + border-top-right-radius: 5px; + border-bottom-right-radius: 5px; + } + td.name { + font-family: fonts.$primary-bold; + font-weight: bold; + } + td.related { + min-width: 2rem; + text-align: center; + &:last-of-type { + min-width: 3rem; + } + } + } + } + div.grid { + // TODO: implement dekstop grid view. for now, "grid" is mobile-only single column + @include breakpoints.for-tablet-landscape-up { + display: none; + } + display: flex; + flex-flow: column; + div.person { + display: flex; + flex-flow: column; + margin: 0.5rem 0; + padding: 1rem; + background-color: var(--background-light); + border: 1px solid var(--background-gray); + border-radius: 5px; + dd { + font-size: typography.$text-size-md; + line-height: calc(24 / 16); + } + dd.name { + @include typography.body-bold; + line-height: calc(27 / 18); + &:has(a) { + margin-bottom: 0.5rem; + } + } + dd.description { + font-size: typography.$text-size-sm; + line-height: calc(17 / 14); + margin: 0.5rem 0; + } + dl.relations { + display: flex; + flex-flow: row nowrap; + border-top: 1px solid colors.$gray; + padding-top: 0.5rem; + dd { + flex: 1 0 33%; + display: flex; + align-items: center; + justify-content: center; + gap: 0.5rem; + svg { + width: 15px; + height: 16px; + vertical-align: middle; + } + &:not(:first-of-type) { + border-left: 1px solid colors.$gray; + } + } + } + } + } + } + .pagination-container { + display: flex; + flex-flow: row; + justify-content: space-between; + align-items: center; + width: 100%; + margin-top: 1rem; + nav.pagination { + margin: 0 auto; + } + @include breakpoints.for-tablet-landscape-up { + padding: 1rem 1.5rem; + border-width: 1px 0px 1px 0px; + border-style: solid; + border-color: var(--background-gray); + font-size: typography.$text-size-md; + nav.pagination { + margin: 0; + a, + span { + font-size: typography.$text-size-md; + } + a { + margin-bottom: -1px; + } + // button icons + .prev::before { + margin: -1px spacing.$spacing-xs 0 0; + font-family: "Phosphor"; + content: "\f0c3"; // phosphor caret-left icon + } + .next::after { + margin: -1px 0 0 spacing.$spacing-xs; + font-family: "Phosphor"; + content: "\f0c4"; // phosphor caret-right icon + } + } + } + div { + display: none; + @include breakpoints.for-tablet-landscape-up { + display: block; + } + } + } +} + +// RTL pagination +html[dir="rtl"] main.people nav.pagination { + // Hebrew and Arabic reversed arrows + .prev::before { + margin: 0.1rem 0 0 spacing.$spacing-xs; + content: "\f0c4"; // phosphor caret-right icon + } + .next::after { + margin: 0.1rem spacing.$spacing-xs 0 0; + content: "\f0c3"; // phosphor caret-left icon + } +} diff --git a/sitemedia/scss/pages/_person.scss b/sitemedia/scss/pages/_person.scss index f9776f2bd..eeccf8917 100644 --- a/sitemedia/scss/pages/_person.scss +++ b/sitemedia/scss/pages/_person.scss @@ -8,20 +8,57 @@ @use "../base/spacing"; @use "../base/typography"; +// TODO: standardize typography here once all redesigns are implemented + +main.person * { + max-width: none; + @include breakpoints.for-tablet-landscape-up { + max-width: 56rem; // = 896px + } +} + main.person { // Header h1 { + width: 100%; z-index: 2; font-size: typography.$text-size-3xl; - margin: spacing.$spacing-xl spacing.$spacing-md spacing.$spacing-md; + margin: spacing.$spacing-lg 0 spacing.$spacing-md; + padding-left: spacing.$spacing-md; @include breakpoints.for-tablet-landscape-up { - margin: spacing.$spacing-2xl spacing.$spacing-md spacing.$spacing-lg; + width: auto; + margin: spacing.$spacing-lg spacing.$spacing-md spacing.$spacing-lg; font-size: typography.$text-size-3xl; + padding: 0; + } + } + // back to browse link + div.breadcrumbs-link-container { + width: 100%; + z-index: 2; + display: flex; + justify-content: flex-start; + align-items: center; + font-size: typography.$text-size-lg; + a { + margin-left: spacing.$spacing-md; + display: flex; + align-items: center; + gap: 0.5rem; + text-decoration: none; + color: var(--link-primary); + &::before { + font-family: "Phosphor" !important; + content: "\f03b"; // phosphor arrow-left icon + font-size: typography.$text-size-2xl; + } + &:hover { + color: var(--link-secondary); + } } } // Primary container .container { - max-width: 896px; display: flex; flex-direction: column; padding: 0 spacing.$spacing-md spacing.$spacing-md;