Refresh file list when there are queued snapshots (#697)

* add destroy hook

* refresh details modal in interval

* refactor to refresh assets list

* disable create snapshot button when there is a pending snapshot

bookmarks/frontend/behaviors/bookmark-page.js

@@ -1,8 +1,8 @@
-import { registerBehavior } from "./index";
+import { Behavior, registerBehavior } from "./index";
 
-class BookmarkItem {
+class BookmarkItem extends Behavior {
   constructor(element) {
-    this.element = element;
+    super(element);
 
     // Toggle notes
     const notesToggle = element.querySelector(".toggle-notes");
@@ -13,9 +13,11 @@ class BookmarkItem {
     // Add tooltip to title if it is truncated
     const titleAnchor = element.querySelector(".title > a");
     const titleSpan = titleAnchor.querySelector("span");
-    if (titleSpan.offsetWidth > titleAnchor.offsetWidth) {
-      titleAnchor.dataset.tooltip = titleSpan.textContent;
-    }
+    requestAnimationFrame(() => {
+      if (titleSpan.offsetWidth > titleAnchor.offsetWidth) {
+        titleAnchor.dataset.tooltip = titleSpan.textContent;
+      }
+    });
   }
 
   onToggleNotes(event) {

bookmarks/frontend/behaviors/bulk-edit.js

@@ -1,8 +1,9 @@
-import { registerBehavior } from "./index";
+import { Behavior, registerBehavior } from "./index";
 
-class BulkEdit {
+class BulkEdit extends Behavior {
   constructor(element) {
-    this.element = element;
+    super(element);
+
     this.active = false;
 
     this.onToggleActive = this.onToggleActive.bind(this);

bookmarks/frontend/behaviors/confirm-button.js

@@ -1,25 +1,29 @@
-import { registerBehavior } from "./index";
+import { Behavior, registerBehavior } from "./index";
 
-class ConfirmButtonBehavior {
+class ConfirmButtonBehavior extends Behavior {
   constructor(element) {
-    const button = element;
-    button.dataset.type = button.type;
-    button.dataset.name = button.name;
-    button.dataset.value = button.value;
-    button.removeAttribute("type");
-    button.removeAttribute("name");
-    button.removeAttribute("value");
-    button.addEventListener("click", this.onClick.bind(this));
-    this.button = button;
+    super(element);
+    element.dataset.type = element.type;
+    element.dataset.name = element.name;
+    element.dataset.value = element.value;
+    element.removeAttribute("type");
+    element.removeAttribute("name");
+    element.removeAttribute("value");
+    element.addEventListener("click", this.onClick.bind(this));
+  }
+
+  destroy() {
+    Behavior.interacting = false;
   }
 
   onClick(event) {
     event.preventDefault();
+    Behavior.interacting = true;
 
     const container = document.createElement("span");
     container.className = "confirmation";
 
-    const icon = this.button.getAttribute("ld-confirm-icon");
+    const icon = this.element.getAttribute("ld-confirm-icon");
     if (icon) {
       const iconElement = document.createElementNS(
         "http://www.w3.org/2000/svg",
@@ -31,27 +35,27 @@ class ConfirmButtonBehavior {
       container.append(iconElement);
     }
 
-    const question = this.button.getAttribute("ld-confirm-question");
+    const question = this.element.getAttribute("ld-confirm-question");
     if (question) {
       const questionElement = document.createElement("span");
       questionElement.innerText = question;
       container.append(question);
     }
 
-    const buttonClasses = Array.from(this.button.classList.values())
+    const buttonClasses = Array.from(this.element.classList.values())
       .filter((cls) => cls.startsWith("btn"))
       .join(" ");
 
-    const cancelButton = document.createElement(this.button.nodeName);
+    const cancelButton = document.createElement(this.element.nodeName);
     cancelButton.type = "button";
     cancelButton.innerText = question ? "No" : "Cancel";
     cancelButton.className = `${buttonClasses} mr-1`;
     cancelButton.addEventListener("click", this.reset.bind(this));
 
-    const confirmButton = document.createElement(this.button.nodeName);
-    confirmButton.type = this.button.dataset.type;
-    confirmButton.name = this.button.dataset.name;
-    confirmButton.value = this.button.dataset.value;
+    const confirmButton = document.createElement(this.element.nodeName);
+    confirmButton.type = this.element.dataset.type;
+    confirmButton.name = this.element.dataset.name;
+    confirmButton.value = this.element.dataset.value;
     confirmButton.innerText = question ? "Yes" : "Confirm";
     confirmButton.className = buttonClasses;
     confirmButton.addEventListener("click", this.reset.bind(this));
@@ -59,14 +63,15 @@ class ConfirmButtonBehavior {
     container.append(cancelButton, confirmButton);
     this.container = container;
 
-    this.button.before(container);
-    this.button.classList.add("d-none");
+    this.element.before(container);
+    this.element.classList.add("d-none");
   }
 
   reset() {
     setTimeout(() => {
+      Behavior.interacting = false;
       this.container.remove();
-      this.button.classList.remove("d-none");
+      this.element.classList.remove("d-none");
     });
   }
 }

bookmarks/frontend/behaviors/dropdown.js

@@ -1,8 +1,8 @@
-import { registerBehavior } from "./index";
+import { Behavior, registerBehavior } from "./index";
 
-class DropdownBehavior {
+class DropdownBehavior extends Behavior {
   constructor(element) {
-    this.element = element;
+    super(element);
     this.opened = false;
     this.onOutsideClick = this.onOutsideClick.bind(this);
 

bookmarks/frontend/behaviors/fetch.js

@@ -1,15 +1,31 @@
-import { fireEvents, registerBehavior, swap } from "./index";
+import { Behavior, fireEvents, registerBehavior, swap } from "./index";
 
-class FetchBehavior {
+class FetchBehavior extends Behavior {
   constructor(element) {
-    this.element = element;
-    const eventName = element.getAttribute("ld-on");
+    super(element);
 
-    element.addEventListener(eventName, this.onFetch.bind(this));
+    const eventName = element.getAttribute("ld-on");
+    const interval = parseInt(element.getAttribute("ld-interval")) * 1000;
+
+    this.onFetch = this.onFetch.bind(this);
+    this.onInterval = this.onInterval.bind(this);
+
+    element.addEventListener(eventName, this.onFetch);
+    if (interval) {
+      this.intervalId = setInterval(this.onInterval, interval);
+    }
   }
 
-  async onFetch(event) {
-    event.preventDefault();
+  destroy() {
+    if (this.intervalId) {
+      clearInterval(this.intervalId);
+    }
+  }
+
+  async onFetch(maybeEvent) {
+    if (maybeEvent) {
+      maybeEvent.preventDefault();
+    }
     const url = this.element.getAttribute("ld-fetch");
     const html = await fetch(url).then((response) => response.text());
 
@@ -20,6 +36,13 @@ class FetchBehavior {
     const events = this.element.getAttribute("ld-fire");
     fireEvents(events);
   }
+
+  onInterval() {
+    if (Behavior.interacting) {
+      return;
+    }
+    this.onFetch();
+  }
 }
 
 registerBehavior("ld-fetch", FetchBehavior);

bookmarks/frontend/behaviors/form.js

@@ -1,8 +1,9 @@
-import { fireEvents, registerBehavior } from "./index";
+import { Behavior, fireEvents, registerBehavior } from "./index";
 
-class FormBehavior {
+class FormBehavior extends Behavior {
   constructor(element) {
-    this.element = element;
+    super(element);
+
     element.addEventListener("submit", this.onSubmit.bind(this));
   }
 
@@ -28,8 +29,10 @@ class FormBehavior {
   }
 }
 
-class AutoSubmitBehavior {
+class AutoSubmitBehavior extends Behavior {
   constructor(element) {
+    super(element);
+
     element.addEventListener("change", () => {
       const form = element.closest("form");
       form.dispatchEvent(new Event("submit", { cancelable: true }));

bookmarks/frontend/behaviors/global-shortcuts.js

@@ -1,7 +1,9 @@
-import { registerBehavior } from "./index";
+import { Behavior, registerBehavior } from "./index";
+
+class GlobalShortcuts extends Behavior {
+  constructor(element) {
+    super(element);
 
-class GlobalShortcuts {
-  constructor() {
     document.addEventListener("keydown", this.onKeyDown.bind(this));
   }
 

bookmarks/frontend/behaviors/index.js

@@ -1,4 +1,35 @@
 const behaviorRegistry = {};
+const debug = false;
+
+const mutationObserver = new MutationObserver((mutations) => {
+  mutations.forEach((mutation) => {
+    mutation.removedNodes.forEach((node) => {
+      if (node instanceof HTMLElement && !node.isConnected) {
+        destroyBehaviors(node);
+      }
+    });
+    mutation.addedNodes.forEach((node) => {
+      if (node instanceof HTMLElement && node.isConnected) {
+        applyBehaviors(node);
+      }
+    });
+  });
+});
+
+mutationObserver.observe(document.body, {
+  childList: true,
+  subtree: true,
+});
+
+export class Behavior {
+  constructor(element) {
+    this.element = element;
+  }
+
+  destroy() {}
+}
+
+Behavior.interacting = false;
 
 export function registerBehavior(name, behavior) {
   behaviorRegistry[name] = behavior;
@@ -33,6 +64,34 @@ export function applyBehaviors(container, behaviorNames = null) {
 
       const behaviorInstance = new behavior(element);
       element.__behaviors.push(behaviorInstance);
+      if (debug) {
+        console.log(
+          `[Behavior] ${behaviorInstance.constructor.name} initialized`,
+        );
+      }
+    });
+  });
+}
+
+export function destroyBehaviors(element) {
+  const behaviorNames = Object.keys(behaviorRegistry);
+
+  behaviorNames.forEach((behaviorName) => {
+    const elements = Array.from(element.querySelectorAll(`[${behaviorName}]`));
+    elements.push(element);
+
+    elements.forEach((element) => {
+      if (!element.__behaviors) {
+        return;
+      }
+
+      element.__behaviors.forEach((behavior) => {
+        behavior.destroy();
+        if (debug) {
+          console.log(`[Behavior] ${behavior.constructor.name} destroyed`);
+        }
+      });
+      delete element.__behaviors;
     });
   });
 }
@@ -63,10 +122,11 @@ export function swap(element, html, options) {
       break;
     case "innerHTML":
     default:
-      targetElement.innerHTML = "";
+      Array.from(targetElement.children).forEach((child) => {
+        child.remove();
+      });
       targetElement.append(...contents);
   }
-  contents.forEach((content) => applyBehaviors(content));
 }
 
 export function fireEvents(events) {

bookmarks/frontend/behaviors/modal.js

@@ -1,8 +1,8 @@
-import { registerBehavior } from "./index";
+import { Behavior, registerBehavior } from "./index";
 
-class ModalBehavior {
+class ModalBehavior extends Behavior {
   constructor(element) {
-    this.element = element;
+    super(element);
 
     const modalOverlay = element.querySelector(".modal-overlay");
     const closeButton = element.querySelector("button.close");

bookmarks/frontend/behaviors/tag-autocomplete.js

@@ -1,9 +1,10 @@
-import { registerBehavior } from "./index";
+import { Behavior, registerBehavior } from "./index";
 import TagAutoCompleteComponent from "../components/TagAutocomplete.svelte";
 import { ApiClient } from "../api";
 
-class TagAutocomplete {
+class TagAutocomplete extends Behavior {
   constructor(element) {
+    super(element);
     const wrapper = document.createElement("div");
     const apiBaseUrl = document.documentElement.dataset.apiBaseUrl || "";
     const apiClient = new ApiClient(apiBaseUrl);

bookmarks/templates/bookmarks/details/assets.html

@@ -1,37 +1,44 @@
-{% if details.assets %}
-  <div class="assets">
-    {% for asset in details.assets %}
-      <div class="asset" data-asset-id="{{ asset.id }}">
-        <div class="asset-icon {{ asset.icon_classes }}">
-          {% include 'bookmarks/details/asset_icon.html' %}
-        </div>
-        <div class="asset-text truncate {{ asset.text_classes }}">
+<div {% if details.has_pending_assets %}
+  ld-fetch="{% url 'bookmarks:details_assets' details.bookmark.id %}"
+  ld-interval="5" ld-target="self|outerHTML"
+{% endif %}>
+  {% if details.assets %}
+    <div class="assets">
+      {% for asset in details.assets %}
+        <div class="asset" data-asset-id="{{ asset.id }}">
+          <div class="asset-icon {{ asset.icon_classes }}">
+            {% include 'bookmarks/details/asset_icon.html' %}
+          </div>
+          <div class="asset-text truncate {{ asset.text_classes }}">
         <span>
           {{ asset.display_name }}
           {% if asset.status == 'pending' %}(queued){% endif %}
           {% if asset.status == 'failure' %}(failed){% endif %}
         </span>
-          {% if asset.file_size %}
-            <span class="filesize">{{ asset.file_size|filesizeformat }}</span>
-          {% endif %}
+            {% if asset.file_size %}
+              <span class="filesize">{{ asset.file_size|filesizeformat }}</span>
+            {% endif %}
+          </div>
+          <div class="asset-actions">
+            {% if asset.file %}
+              <a class="btn btn-link" href="{% url 'bookmarks:assets.view' asset.id %}" target="_blank">View</a>
+            {% endif %}
+            {% if details.is_editable %}
+              <button ld-confirm-button type="submit" name="remove_asset" value="{{ asset.id }}" class="btn btn-link">
+                Remove
+              </button>
+            {% endif %}
+          </div>
         </div>
-        <div class="asset-actions">
-          {% if asset.file %}
-            <a class="btn btn-link" href="{% url 'bookmarks:assets.view' asset.id %}" target="_blank">View</a>
-          {% endif %}
-          {% if details.is_editable %}
-            <button ld-confirm-button type="submit" name="remove_asset" value="{{ asset.id }}" class="btn btn-link">
-              Remove
-            </button>
-          {% endif %}
-        </div>
-      </div>
-    {% endfor %}
-  </div>
-{% endif %}
+      {% endfor %}
+    </div>
+  {% endif %}
 
-{% if details.is_editable %}
-  <div class="assets-actions">
-    <button type="submit" name="create_snapshot" class="btn btn-link">Create HTML snapshot</button>
-  </div>
-{% endif %}
+  {% if details.is_editable %}
+    <div class="assets-actions">
+      <button type="submit" name="create_snapshot" class="btn btn-link"
+              {% if details.has_pending_assets %}disabled{% endif %}>Create HTML snapshot
+      </button>
+    </div>
+  {% endif %}
+</div>

bookmarks/tests/test_bookmark_details_modal.py

@@ -1,3 +1,4 @@
+import re
 from unittest.mock import patch
 
 from django.test import TestCase, override_settings
@@ -105,6 +106,12 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin
     def test_access_with_sharing(self):
         self.details_route_sharing_access_test(self.get_view_name(), True)
 
+    def test_assets_access(self):
+        self.details_route_access_test("bookmarks:details_assets", True)
+
+    def test_assets_access_with_sharing(self):
+        self.details_route_sharing_access_test("bookmarks:details_assets", True)
+
     def test_displays_title(self):
         # with title
         bookmark = self.setup_bookmark(title="Test title")
@@ -753,6 +760,27 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin
         self.assertEqual(response.status_code, 404)
         self.assertTrue(BookmarkAsset.objects.filter(id=asset.id).exists())
 
+    @override_settings(LD_ENABLE_SNAPSHOTS=True)
+    def test_assets_refresh_when_having_pending_asset(self):
+        bookmark = self.setup_bookmark()
+        asset = self.setup_asset(bookmark, status=BookmarkAsset.STATUS_COMPLETE)
+        fetch_url = reverse("bookmarks:details_assets", args=[bookmark.id])
+
+        # no pending asset
+        soup = self.get_details(bookmark)
+        files_section = self.find_section(soup, "Files")
+        assets_wrapper = files_section.find("div", {"ld-fetch": fetch_url})
+        self.assertIsNone(assets_wrapper)
+
+        # with pending asset
+        asset.status = BookmarkAsset.STATUS_PENDING
+        asset.save()
+
+        soup = self.get_details(bookmark)
+        files_section = self.find_section(soup, "Files")
+        assets_wrapper = files_section.find("div", {"ld-fetch": fetch_url})
+        self.assertIsNotNone(assets_wrapper)
+
     @override_settings(LD_ENABLE_SNAPSHOTS=True)
     def test_create_snapshot(self):
         with patch.object(
@@ -765,3 +793,27 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin
             self.assertEqual(response.status_code, 302)
 
             self.assertEqual(bookmark.bookmarkasset_set.count(), 1)
+
+    @override_settings(LD_ENABLE_SNAPSHOTS=True)
+    def test_create_snapshot_is_disabled_when_having_pending_asset(self):
+        bookmark = self.setup_bookmark()
+        asset = self.setup_asset(bookmark, status=BookmarkAsset.STATUS_COMPLETE)
+
+        # no pending asset
+        soup = self.get_details(bookmark)
+        files_section = self.find_section(soup, "Files")
+        create_button = files_section.find(
+            "button", string=re.compile("Create HTML snapshot")
+        )
+        self.assertFalse(create_button.has_attr("disabled"))
+
+        # with pending asset
+        asset.status = BookmarkAsset.STATUS_PENDING
+        asset.save()
+
+        soup = self.get_details(bookmark)
+        files_section = self.find_section(soup, "Files")
+        create_button = files_section.find(
+            "button", string=re.compile("Create HTML snapshot")
+        )
+        self.assertTrue(create_button.has_attr("disabled"))

bookmarks/urls.py

@@ -44,6 +44,11 @@ urlpatterns = [
         views.bookmarks.details_modal,
         name="details_modal",
     ),
+    path(
+        "bookmarks/<int:bookmark_id>/details_assets",
+        views.bookmarks.details_assets,
+        name="details_assets",
+    ),
     # Assets
     path(
         "assets/<int:asset_id>",

bookmarks/views/bookmarks.py

@@ -172,6 +172,10 @@ def details_modal(request, bookmark_id: int):
     return _details(request, bookmark_id, "bookmarks/details_modal.html")
 
 
+def details_assets(request, bookmark_id: int):
+    return _details(request, bookmark_id, "bookmarks/details/assets.html")
+
+
 def convert_tag_string(tag_string: str):
     # Tag strings coming from inputs are space-separated, however services.bookmarks functions expect comma-separated
     # strings

bookmarks/views/partials/contexts.py

@@ -390,3 +390,6 @@ class BookmarkDetailsContext:
         self.assets = [
             BookmarkAssetItem(asset) for asset in bookmark.bookmarkasset_set.all()
         ]
+        self.has_pending_assets = any(
+            asset.status == BookmarkAsset.STATUS_PENDING for asset in self.assets
+        )

web-types.json

@@ -26,6 +26,20 @@
             "required": false
           }
         },
+        {
+          "name": "ld-select",
+          "description": "The content element(s) to select from the fetched content, for example `#main-content`",
+          "value": {
+            "required": false
+          }
+        },
+        {
+          "name": "ld-interval",
+          "description": "Automatically fetches the content of the given URL at the given interval, in seconds",
+          "value": {
+            "required": false
+          }
+        },
         {
           "name": "ld-fire",
           "description": "Fires one or more events once a behavior, such as ld-fetch or ld-form, is finished",