Skip to content

Proof-of-concept: Introduce HTMX-based replacement for QuickSelect component in Maintenance tool #2883

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 2 commits into
base: feature/htmx
Choose a base branch
from

Conversation

lunkwill42
Copy link
Member

This is related to both #2639 and #2251 .

It's a very crude start to a PoC that can potentially replace the b0rked QuickSelect component used in the Maintenance tool. The QuickSelect component is also used by the IP Device History tool, and could be replaced also there once complete.

Quick explanation, for now: The QuickSelect component is pre-populated with all netboxes, locations, rooms, services, etc. from NAV, and is consequently output into the resulting HTML of the maintenance task edit form. From that HTML, QuickSelect's JavaScript counterpart allows the user to filter form values from this potentially giant list of stuff. This is exactly why the page load times can grow very long in big NAV installs, as mentioned in #2251. It will grow even bigger if we add an option to put interfaces on maintenance, which is a feature that has been suggested to us.

This PR intends to demonstrate how to use HTMX to dynamically look up maintenance components by having the backend do the searching and return fully formed HTML with the results. The potential downside to this approach is that there will be no initial scrollable list of all components: The user must enter at least a single letter search query to see any selectable components. This might still be preferable to loading "everything" from the database into an HTML fragment (this is equivalent to the API strategy that refuses to return all the millions of ARP entries when no arguments are given to the endpoint)

Copy link

github-actions bot commented Apr 26, 2024

Test results

       8 files         8 suites   4m 18s ⏱️
3 321 tests 3 321 ✔️ 0 💤 0
4 940 runs  4 940 ✔️ 0 💤 0

Results for commit f123d85.

♻️ This comment has been updated with latest results.

Copy link
Contributor

@hmpf hmpf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There ought to be a naming convention for html templates that do not contain {% extends ..%}, that is: html fragments changing parts of a page, not an entire page.

Some possible naming conventions:

  • Prefix with _ (single underline)
  • End with -frag.html, not .html
  • End with -partial.html, not.html

@@ -32,6 +32,7 @@
re_path(r'^active/$', views.active, name='maintenance-active'),
re_path(r'^planned/$', views.planned, name='maintenance-planned'),
re_path(r'^historic/$', views.historic, name='maintenance-historic'),
re_path(r'^search/$', views.component_search, name='maintenance-crazy-shit'),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why stick with re_path? path is easier to read.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could ask you the same. I used the same pattern as the surrounding lines. They were all last modified by you as part of some Django upgrade, I believe :)

@lunkwill42
Copy link
Member Author

There ought to be a naming convention for html templates that do not contain {% extends ..%}, that is: html fragments changing parts of a page, not an entire page.

Since NAV never started out as a Django project, there has never been an explicit agreed-upon convention for this in NAV. I suspect a lot of this is pretty unused in NAV to distinguish fragments today, except for one subsystem: ipdevinfo uses the *-frag.html convention, so perhaps we should continue using that going forward (but a decision should be made and documented in the "hacking" section of the docs, I guess)

@podliashanyk
Copy link
Contributor

Some possible naming conventions:

  • Prefix with _ (single underline)
  • End with -frag.html, not .html
  • End with -partial.html, not.html

I don't have any particularly strong opinion on this one. But here are my thoughts:

  • I slightly prefer the prefix with _ (single underline) option since it is easy to quickly visually grasp what template is what when looking at the file structure, f.e.:
    Screenshot 2024-09-27 at 08 44 00
  • Between -frag.html and -partial.html options I slightly prefer the former because of brevity

The differences are minuscule IMO and are more a case of personal preferences. Unless we discover some more "technical" reasons to choose one over the other..

@podliashanyk
Copy link
Contributor

Discovered another naming variant used in NAV:

  • partials (no special prefix or ending in name) placed in /fragments directory, see python/nav/web/templates/seeddb/fragments

@lunkwill42
Copy link
Member Author

The potential downside to this approach is that there will be no initial scrollable list of all components: The user must enter at least a single letter search query to see any selectable components. This might still be preferable to loading "everything" from the database into an HTML fragment

We might also consider some sort of "infinite-scroll" option that only loads a few items at a time, but reloads with new results when a filter has been entered. E.g. https://htmx.org/examples/infinite-scroll/

Copy link
Contributor

@podliashanyk podliashanyk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code seems good so far, although I didn't run it yet. Will come back to it later.

django_htmx dependency seems redundant so far, do you intend to use it in this PR?

Also we should document or mention someplace what version of HTMX is vendored.

@podliashanyk
Copy link
Contributor

The potential downside to this approach is that there will be no initial scrollable list of all components: The user must enter at least a single letter search query to see any selectable components. This might still be preferable to loading "everything" from the database into an HTML fragment

We might also consider some sort of "infinite-scroll" option that only loads a few items at a time, but reloads with new results when a filter has been entered. E.g. https://htmx.org/examples/infinite-scroll/

Loading all items, but limiting it with infinite scroll sounds like a good solution here. Then we also preserve the current functionality where user can browse all the items without entering at least 1 character. At the same time we will not overload our result list by default for cases when the list has many items.

@podliashanyk
Copy link
Contributor

When it comes to naming convention for partials, we now have a poll #3368

@lunkwill42 lunkwill42 force-pushed the poc/maintenance-htmx branch from f123d85 to 61af20b Compare June 12, 2025 14:48
@lunkwill42
Copy link
Member Author

Just rebased this on the latest master and squashed fixup commits, to get this up to snuff. Since we are already looking into other HTMX features like #3385, the two commits that introduce HTMX and Django-HTMX should probably be moved to a separate PR that others can be based off (and possibly the vendored in HTMX library should be replaced with a newer version)

@lunkwill42 lunkwill42 added the nonews No news fragment is necessary for this PR (e.g. refactoring, cleanups, workflow/development changes) label Jun 12, 2025
@lunkwill42 lunkwill42 force-pushed the poc/maintenance-htmx branch from 61af20b to 925d05f Compare June 12, 2025 15:13
@lunkwill42 lunkwill42 force-pushed the poc/maintenance-htmx branch from 925d05f to 9832f3c Compare June 12, 2025 15:20
@lunkwill42 lunkwill42 changed the base branch from master to feature/htmx June 12, 2025 15:20
@lunkwill42
Copy link
Member Author

Cleaned up once more and moved HTMX introduction to a separate draft PR #3386

@lunkwill42 lunkwill42 force-pushed the poc/maintenance-htmx branch from 534fb53 to 148c1e1 Compare June 13, 2025 09:34
Copy link

@podliashanyk
Copy link
Contributor

TODO: actually replace the old search component with the new one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
htmx nonews No news fragment is necessary for this PR (e.g. refactoring, cleanups, workflow/development changes)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants