Add Htmx destinations page#970
Conversation
Test results 10 files 1 040 suites 35m 43s ⏱️ Results for commit 993ba78. ♻️ This comment has been updated with latest results. |
stveit
force-pushed
the
htmx-destinations
branch
from
84bedb4 to
94581d3
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #970 +/- ##
==========================================
- Coverage 82.50% 81.19% -1.31%
==========================================
Files 138 140 +2
Lines 4944 5074 +130
==========================================
+ Hits 4079 4120 +41
- Misses 865 954 +89 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
stveit
force-pushed
the
htmx-destinations
branch
5 times, most recently
from
301a331 to
c1e3d53
Compare
stveit
force-pushed
the
htmx-destinations
branch
2 times, most recently
from
b97d9e0 to
7f55846
Compare
It's that stupid, is it? sigh |
hmpf
requested review from
hmpf,
johannaengland,
lunkwill42 and
podliashanyk
and removed request for
hmpf
hmpf
left a comment
hmpf
left a comment
There was a problem hiding this comment.
Manually tested, seems to work just fine. @podliashanyk?
Pretty much copied from the destinations API, needs some changes so it wont just crash to 500 when it raises a ValidationError This commit doesnt make it show in the frontend, the button for deleting a destination will be grouped with the forms for updating destinations
Only update the elements that needs to be updated.
stveit
force-pushed
the
htmx-destinations
branch
from
7f55846 to
993ba78
Compare
|
Yup. All input fields on destinations page pass the test "click on a label and see the input field being activated". So all labels ARE correctly programatically connected to corresponding input fields |
There was a problem hiding this comment.
Great job! 👏 The page works and looks very well. See some non-critical suggestions/polishes/bugs below (sorted by priority). All of them belong to separate PRs.
- Bug: when creating an email and an sms destination with the same name I get error "Name must be unique per media".
- Error text should be red.
- Applies to collapse elements, as well as the create form: let's add vertical gaps between them, also let's add padding/spacing on each side of them, and let's center their contents.
- See HTMX suggestions below. All of them are polishes to make swaps more granular.
| <div class="flex w-full h-fit items-center"> | ||
| {% include "htmx/destination/_edit_form.html" %} | ||
| {% include "htmx/destination/_delete_form.html" %} | ||
| </div> |
There was a problem hiding this comment.
HTMX suggestion: If we put id on the parent div (the one with class .collapse-content) and put a highlighted div (lines 7-10) into a separate file, then we can return that file in create and update endpoints. This logic combined with using hx-swap='beforeend' would make more granular swaps on create and update actions.
| <form hx-post="{% url 'htmx:htmx-delete' form.instance.id %}" | ||
| hx-trigger="submit" | ||
| hx-target="#form-list"> |
There was a problem hiding this comment.
HTMX suggestion: If we put id on each form-div, we could use it as a value in hx-target here. This combined with hx-swap='delete' would lead to a more granular swap on delete action.
5 participants
Closes #1001
Replaces Uninett/argus-htmx-frontend#205
Things that need to be done:
Sonarcloud complains about
A form label must be associated with a control., but the form field is inside the label so explicitly stating the control element shouldnt be necessary.