Skip to content

POC: Replace foundation modals with HTMX templates and utilities #3454

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: master
Choose a base branch
from

Conversation

Simrayz
Copy link
Contributor

@Simrayz Simrayz commented Aug 19, 2025

Scope and purpose

Implements part of #3449, part of meta #2972.

This PR replaces modals used in the Add/Remove Patch modals in the SeedDB tool. It is an inital POC of how modals could be implemented with reusable utilities.

Add a description of what this PR does and why it is needed. If a linked ticket(s) fully
cover it, you can omit this.

The implementation takes a "functional approach", where the rendering is controlled and configured in the views. I tried using a "dialog" element, but it is very heavily styled and modifed by foundation css and js. As such, I opted for a role="dialog" for now.

Closing of the modal/updating is handled by utilities, where hx-swap-oob is used to update other parts of the UI together with the main response.

This pull request

  • Adds new reusable HTMX modal utilities:
    • render_modal: Renders a given template inside an HTMX modal
    • resolve_modal: Resolves modal by closing it and returning an optional template (which can be replaced with hx-target attributes)
    • render_modal_alert: Adds a given error message to the alert section of the modal.
  • Replaces the javascript magic of seeddb_add_patch.js with htmx enabled views for showing modals and updating the DOM after successful saving of patches.
  • Adds tests for the adding and removal of tests, and fixes a bug where the view crashes if a cable ID is not selected.

Contributor Checklist

Every pull request should have this checklist filled out, no matter how small it is.
More information about contributing to NAV can be found in the
Hacker's guide to NAV.

  • Added a changelog fragment for towncrier
  • Added/amended tests for new/changed code
  • Added/changed documentation
  • Linted/formatted the code with ruff, easiest by using pre-commit
  • The first line of the commit message continues the sentence "If applied, this commit will ...", starts with a capital letter, does not end with punctuation and is 50 characters or less long. See https://cbea.ms/git-commit/
  • This pull request is based on the correct upstream branch: For a patch/bugfix affecting the latest stable version, it should be based on that version's branch (<major>.<minor>.x). For a new feature or other additions, it should be based on master.
  • If applicable: Created new issues if this PR does not fix the issue completely/there is further work to be done
  • If this results in changes in the UI: Added screenshots of the before and after
  • If this adds a new Python source code file: Added the boilerplate header to that file

Copy link

github-actions bot commented Aug 19, 2025

Test results

   12 files     12 suites   12m 22s ⏱️
2 268 tests 2 268 ✅ 0 💤 0 ❌
6 369 runs  6 369 ✅ 0 💤 0 ❌

Results for commit d76cea3.

♻️ This comment has been updated with latest results.

Copy link

codecov bot commented Aug 19, 2025

Codecov Report

❌ Patch coverage is 97.56098% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 60.88%. Comparing base (d7ef7c3) to head (d76cea3).
⚠️ Report is 36 commits behind head on master.

Files with missing lines Patch % Lines
python/nav/web/seeddb/page/patch/__init__.py 95.45% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3454      +/-   ##
==========================================
+ Coverage   60.83%   60.88%   +0.04%     
==========================================
  Files         608      606       -2     
  Lines       44273    44301      +28     
  Branches       48       43       -5     
==========================================
+ Hits        26934    26972      +38     
+ Misses      17327    17319       -8     
+ Partials       12       10       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Simrayz Simrayz force-pushed the poc/replace-foundation-js-modals branch from ef0ba41 to d76cea3 Compare August 19, 2025 14:29
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant