UI bug triage

.gitignore

@@ -15,6 +15,11 @@ staticfiles/*
 customizations/**/*.css
 customizations/**/*.js
 
+### Vite's temporary compiled copy of vite.config.mjs, created on every
+### `vite build`/`vite dev` run and normally deleted on clean exit. A build
+### that errors out (missing dep, etc.) can leave one behind.
+vite.config.mjs.timestamp-*.mjs
+
 ### python gitignores auto-generated by github
 
 # Byte-compiled / optimized / DLL files

apps/cms/wagtail_hooks.py

@@ -8,5 +8,10 @@
 # Register a custom css file for the wagtail admin.
 @hooks.register("insert_global_admin_css", order=100)
 def global_admin_css():
-    """Add /static/css/wagtail.css."""
-    return format_html('<link rel="stylesheet" href="{}">', static("css/wagtail.css"))
+    """Add /static/css/wagtail_admin.css.
+
+    Named wagtail_admin.css (not wagtail.css) to avoid confusion with the
+    unrelated components/wagtail.scss, which styles rich-text embeds on the
+    public-facing site rather than admin widgets.
+    """
+    return format_html('<link rel="stylesheet" href="{}">', static("css/wagtail_admin.css"))

apps/readux/views.py

@@ -224,20 +224,17 @@ def get_context_data(self, **kwargs):
             .count()
         )
 
-        user_annotation_index = UserAnnotation.objects.all()
-
-        user_annotation_index = user_annotation_index.filter(
-            canvas__manifest__label=manifest.label
-        )
-
-        user_annotation_index = user_annotation_index.filter(
-            owner_id=self.request.user.id
-        ).distinct()
-
+        # Filter by manifest id (not label — labels aren't guaranteed unique,
+        # so filtering by label risks mixing in annotations from a different
+        # manifest that happens to share a title). Grouping happens via
+        # .values().annotate(Count(...)); an upstream .distinct() on the
+        # un-grouped queryset is unnecessary here (the GROUP BY already
+        # collapses to one row per canvas__position) and was masking/dropping
+        # canvases from the index in practice, so it's been removed.
         user_annotation_index = (
-            user_annotation_index.values(
-                "canvas__position", "canvas__manifest__label", "canvas__pid"
-            )
+            UserAnnotation.objects.filter(canvas__manifest__id=manifest.id)
+            .filter(owner_id=self.request.user.id)
+            .values("canvas__position", "canvas__manifest__label", "canvas__pid")
             .annotate(Count("canvas__position"))
             .order_by("canvas__position")
         )

apps/static/css/components/search.scss

@@ -236,6 +236,17 @@ form#search-form {
   background-color: $rx-color-faded-mint !important;
 }
 
+// Scoped to the date range filter only (its selectize dropdowns render inline
+// within #date-range-filter, since no dropdownParent is set in search.js) —
+// the start/end year selects are single-select, so "selected" here means the
+// currently chosen year, not a multi-select tag like the collection/author/
+// language facets above. Use the same dark blue as those facet chips instead
+// of selectize's default light blue, to match the rest of the design.
+#date-range-filter .selectize-dropdown .option.selected {
+  background-color: $rx-color-midnight-blue !important;
+  color: $color-white !important;
+}
+
 .selectize-control.plugin-clear_button .clear {
   height: 85%;
   top: -3px !important;

apps/static/css/readux.scss

@@ -65,7 +65,7 @@ main { flex-grow: 1; }
   width: 100%;
   height: 80vh;
   min-height: 500px;
-  background-color: rgba($rx-color-midnight-blue, 0.8);
+  background-color: rgba($rx-color-midnight-blue, 0.85);
 }
 
 .content {
@@ -82,7 +82,11 @@ main { flex-grow: 1; }
 /* Text & headings */
 .paragraph { color: $rx-color-midnight-blue; line-height: normal; }
 .title { color: $rx-color-midnight-blue; font-size: x-large; font-weight: bold; }
-.hero { padding-top: 15vh; padding-bottom: 2rem; color: $color-white; background-color: rgba($rx-color-midnight-blue, 0.85); }
+/* No background-color here: .hero sits on top of .overlay (see above), which
+   already provides the scrim over the hero image. Giving .hero its own
+   background would double-stack with .overlay and make the hero area visibly
+   more opaque than the surrounding .home-nav / .overlay-only regions. */
+.hero { padding-top: 15vh; padding-bottom: 2rem; color: $color-white; }
 .uk-container h2 { margin-bottom: 0.35rem;}
 
 /* Utilities */

apps/static/js/components/VolumeAnnotations.vue

@@ -69,13 +69,34 @@ export default {
     // navigation, where added/deleted can be spuriously true because the
     // counts of two different pages get compared. Track the current canvas
     // and only treat add/delete as real when the canvas hasn't changed.
+    //
+    // The annotator (ecds-annotator) also dispatches a synthetic RESET
+    // "canvasswitch" — {canvas: "all", annotationsOnPage: 0, ...} — before
+    // the real per-canvas event for whatever canvas is loading. That's not
+    // a timing race to be debounced away; it's a deliberate, deterministic
+    // placeholder event that never represents a real page's annotation
+    // count. Treating it like any other canvas (as the old code did) let it
+    // poison _prevPageCount with 0, so the next real event for the actual
+    // canvas got diffed against that bogus 0 and double-counted an
+    // annotation that was already included in the server-rendered total.
+    // Fix: ignore canvas === "all" outright rather than guess a settle time.
+    const RESET_CANVAS = "all";
     this._currentCanvas = null;
     this._prevPageCount = null;
+
     this._onCanvasSwitch = (event) => {
       const detail = event && event.detail ? event.detail : {};
-      const sameCanvas = detail.canvas && detail.canvas === this._currentCanvas;
       const newPageCount = typeof detail.annotationsOnPage === "number" ? detail.annotationsOnPage : null;
 
+      if (!detail.canvas || detail.canvas === RESET_CANVAS) {
+        // Synthetic reset event: carries no real per-canvas count. Ignore
+        // it entirely — don't touch localPageCount, _currentCanvas, or
+        // _prevPageCount from it.
+        return;
+      }
+
+      const sameCanvas = detail.canvas === this._currentCanvas;
+
       if (sameCanvas && newPageCount !== null && this._prevPageCount !== null) {
         const delta = newPageCount - this._prevPageCount;
         if (delta > 0) {
@@ -117,12 +138,23 @@ export default {
         }
       }
 
-      if (detail.canvas) {
-        if (detail.canvas !== this._currentCanvas) {
-          this._prevPageCount = newPageCount;
-        }
-        this._currentCanvas = detail.canvas;
+      if (detail.canvas !== this._currentCanvas) {
+        // First sighting of this canvas (or a switch to it). Don't trust
+        // this event's own annotationsOnPage as the baseline — it can be a
+        // transient/incomplete read fired before the annotator's async
+        // fetch for this canvas has resolved, and a later event for the
+        // SAME canvas with the real count would then get diffed against
+        // that bogus baseline and double-count an annotation that's already
+        // included in the server-rendered totals. annotationData (seeded
+        // from the server-rendered json_data) already has the authoritative
+        // per-canvas count for anything with existing annotations, so use
+        // that as the baseline instead. Only fall back to this event's
+        // value for a canvas annotationData has never heard of (a page with
+        // zero annotations, where 0 is correct either way).
+        const known = this.annotationData.find((a) => a.canvas__pid === detail.canvas);
+        this._prevPageCount = known ? known.canvas__position__count : newPageCount;
       }
+      this._currentCanvas = detail.canvas;
     };
 
     window.addEventListener("canvasswitch", this._onCanvasSwitch);

apps/templates/base.html

@@ -82,6 +82,31 @@
     <meta name="twitter:site" content="@emorycds" />
     <meta name="twitter:creator" content="@emorycds" />
     <link rel="icon" href="{% static 'images/favicons/favicon.ico' %}" />
+    <style>
+      /* Critical CSS to prevent a flash of unstyled nav on slow connections.
+         The top nav's submenus (e.g. "About") are rendered inline in the raw
+         HTML via {% main_menu add_sub_menus_inline=True %} so the dropdown
+         works even without JS — but that means they're hidden/positioned as
+         an overlay purely by the external UIkit/readux stylesheets below. On
+         a slow connection those stylesheets can still be loading when the
+         browser's render-blocking timeout expires and it paints anyway, so
+         the submenu briefly renders in normal document flow: a wide-open
+         plain list sitting under the nav instead of a hidden dropdown. This
+         duplicates just enough of UIkit's own navbar/dropdown rules, inline
+         and dependency-free, to avoid that flash until the full stylesheets
+         arrive. */
+      [uk-navbar], .uk-navbar-left, .uk-navbar-right, .uk-navbar-nav {
+        display: flex;
+        align-items: center;
+        list-style: none;
+        margin: 0;
+        padding: 0;
+      }
+      .uk-navbar-dropdown {
+        display: none;
+        position: absolute;
+      }
+    </style>
     {% block css %}
       <link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/uikit/3.17.0/css/uikit.min.css" />
       <link type="text/css" href="{% sass_src 'css/readux.scss' %}" rel="stylesheet" />

apps/templates/page.html

@@ -13,7 +13,6 @@
 {% endblock nav %}
 
 {% block content %}
-{{ json_data|json_script:"context" }}
 {% block inner %}
 
 <!-- uiAction search -->
@@ -76,6 +75,21 @@
 {% endif %}
 {% endblock viewer %}
 
+{% block javascript %}
+{% comment %}
+Rendered here (outside #v-readux, before main.js loads) rather than inside
+the content block: Vue 3 strips <script> tags when it compiles an in-DOM
+template, so a json_script tag placed anywhere inside the #v-readux mount
+target never survives to be read by VolumeAnnotations.vue's mounted() hook —
+document.getElementById("context") comes back null every time, even though
+the server renders this tag correctly on every request. Keeping it outside
+Vue's mount target (and before main.js, so it's already in the DOM by the
+time Vue mounts) is what makes it actually readable.
+{% endcomment %}
+{{ json_data|json_script:"context" }}
+{{ block.super }}
+{% endblock javascript %}
+
 {% block extra_javascript %}
 <script type="text/javascript">
 window.onload = function() {

apps/templates/search_results.html

@@ -132,7 +132,14 @@
                                 <select id="id_end_year" name="end_date" aria-label="Filter by end year published"></select>
                             </div>
                         </div>
-                        <p class="date-range-bce-note">
+                        {% comment %}
+                        Element stays in the DOM (just visually hidden) even when there's nothing
+                        BCE-dated in the current results, rather than being removed with {% if %} —
+                        search.js toggles an "is-disabled" class on this element by selector with
+                        no null check, so removing it outright would throw when the date filter is
+                        switched on/off.
+                        {% endcomment %}
+                        <p class="date-range-bce-note"{% if not date_range_has_bce %} hidden{% endif %}>
                             Volumes dated before year 1 CE (BCE) can't be shown individually here — they're grouped under the earliest year in the list above{% if date_range_has_bce %}, labeled "1 or earlier"{% endif %}.
                         </p>
                         <button
@@ -141,7 +148,12 @@
                             class="date-range-reset-link"
                             hidden
                         >Reset Date Range</button>
-                        <div class="date-range-filter-label uk-flex uk-flex-middle date-range-undated-toggle">
+                        {% comment %}
+                        Same reasoning as the BCE note above: kept in the DOM and just hidden when
+                        there's nothing for it to control, since search.js reads this checkbox
+                        unconditionally (id_include_undated.checked, etc.) on every page load.
+                        {% endcomment %}
+                        <div class="date-range-filter-label uk-flex uk-flex-middle date-range-undated-toggle"{% if not undated_volume_count %} hidden{% endif %}>
                             <input type="checkbox" name="include_undated" id="id_include_undated" class="uk-checkbox uk-margin-small-right"/>
                             <label for="id_include_undated">
                                 {{ form.include_undated.label }}