-
Notifications
You must be signed in to change notification settings - Fork 0
FREYA-1640: New SLU wastewater dashboard #56
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
base: main
Are you sure you want to change the base?
Conversation
0343be9 to
5bfe0d5
Compare
Ziip-dev
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given #60 , maybe we can already leverage it here so we have a solid baseline implementation that can serve as an example for other htmx stuff to come?
Yes, I intend to modify this. Will make new commit 👍🏽 |
Ziip-dev
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I didn't see your answer while I was doing this (and I felt bad about giving you extra work on this one 😅) so here are some examples and guidelines if that can help:
| from core.settings.base import env # noqa: E402, F401 | ||
|
|
||
|
|
||
| class SLUsync(LoginRequiredMixin, View): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then we can use trigger_client_event and HX-Trigger to simplify the data sync by moving the logic server-side, something like:
from django_htmx.http import trigger_client_event
class SLUsync(LoginRequiredMixin, View):
def post(self, request, *args, **kwargs):
...
response = HttpResponseClientRefresh() # or HttpResponse("OK")
trigger_client_event(response, "slu-sync:done", {"And the template can listen for htmx:afterOnLoad (or your slu-sync:done if you prefer) to avoid custom button toggling JS:
<button hx-post="{% url 'dashboards:slu_data_sync' %}" hx-trigger="click" hx-indicator=".spinner">
Sync SLU data
</button>
<div id="sync-status"></div>
<script>
document.body.addEventListener('slu-sync:done', (event) => {
document.querySelector('#sync-status').textContent =
`Synced ${event.detail.rows} records`;
});
</script>There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to simplify the data sync by moving the logic server-side
I couldn’t see what logic the suggested code moves to the server side. Also I think for this particular case,trigger_client_event doesn’t give any additional benefit but just another way of doing things.
If we don't wanna have any html code in the python view file, then I would just have something like you suggested for the template, but only differences being I would listen for htmx:afterRequest instead of custom trigger.
P.S: In general, I am curious how the HttpResponseClientRefresh and trigger_client_event combination will work. Will it refresh the page first ? In that case the previous response cycle wouldn’t exist anymore to trigger the custom event right ? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So the goal here was just to stop emitting HTML strings from the view and stop juggling button text in JS.
After syncing we can keep returning HttpResponse("OK") but use something like trigger_client_event(response, "slu-sync:done", ...) so the template just listens for that event and updates the sync status (this would replace setButtonStatus in data_sync.html and the <span> response in slu_ww.py`)
HttpResponseClientRefresh is to order htmx to fully reload the page yes ('combination' as in A or B, not A and B working together, sorry 😅 )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So the goal here was just to stop emitting HTML strings from the view and stop juggling button text in JS. After syncing we can keep returning
HttpResponse("OK")but use something liketrigger_client_event(response, "slu-sync:done", ...)so the template just listens for that event and updates the sync status (this would replacesetButtonStatusindata_sync.htmland the<span>response in slu_ww.py`)
Yes, that was my understanding and for that I think we can just use already existing check points (like afterRequest, beforeSwap, etc) in the template directly instead of a custom trigger and trigger_client_event (in view) :)
HttpResponseClientRefreshis to orderhtmxto fully reload the page yes ('combination' as in A or B, not A and B working together, sorry 😅 )
Haha ok, now that makes sense 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok that's fine
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Every SLU template currently re-imports the CDN script and adds global event listeners for validation (overview.html and analysis.html both append <script src="https://cdn...">). If we leverage django-htmx custom middleware, we can:
- Load
htmxonce incore/templates/base.html(to discuss, but if we end up usinghtmxcapabilities more and more throughout the pages I think it's worth it) - Use
request.htmx.boostedin the views tohx-boostthe tab navigation container inbase.htmlso tab clicks fetch only the content area:
<div hx-boost="true" hx-target="main" hx-push-url="true">There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Load
htmxonce incore/templates/base.html(to discuss, but if we end up usinghtmxcapabilities more and more throughout the pages I think it's worth it)
I thought about this too, not just for this but for plotly js too. Like having it in the main base.html but with a condition check like
{% if load_htmx %} {% htmx_script %} {% endif %}
{% if load_plotlyjs %} {% include "load_plotlyjs.html" %} {% endif %}
Then the above variables can be set in the respective views to include these libraries.
- Use
request.htmx.boostedin the views tohx-boostthe tab navigation container inbase.htmlso tab clicks fetch only the content area:<div hx-boost="true" hx-target="main" hx-push-url="true">
Hmm I am not a fan of using htmx to navigate through the pages considering the overhead (just loading some libraries which are mostly cached by browsers) we would potentially save. I also prefer not to give an illusion of a single-page-application while it is not. I would prefer to use htmx only to update parts of the page and not the whole main content unless it is necessary 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
{% if load_htmx %} {% htmx_script %} {% endif %}
{% if load_plotlyjs %} {% include "load_plotlyjs.html" %} {% endif %}
That's cool! Then we would be able to load it like context["load_htmx"] = True ?
Well the dashboard is a part of the page technically ^^ But I get your point, I'm curious why though, if you don't mind explaining, because I would genuinely be like 'user click on a button so let's just reload the dashboard area and call it a day without having different endpoints for different sections of the dashboard'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
{% if load_htmx %} {% htmx_script %} {% endif %} {% if load_plotlyjs %} {% include "load_plotlyjs.html" %} {% endif %}That's cool! Then we would be able to load it like
context["load_htmx"] = True?
Exactly, thats what I was thinking.
Well the dashboard is a part of the page technically ^^
Yes, but each tab is its own page (i.e. everything except header menu bar and footer changes). Which is basically the whole page.
But I get your point, I'm curious why though, if you don't mind explaining, because I would genuinely be like 'user click on a button so let's just reload the dashboard area and call it a day without having different endpoints for different sections of the dashboard'
As mentioned above, each tab is its own page and some of those pages/tabs (Overview and the pathogens pages/tabs) have graphs.
Currently, we use htmx to only update these graphs based and selected inputs/filters and use the native link (a) navigation to change tabs. So we know for a given url if it is a normal request then we need load the whole page and it is a htmx request we need to update the plot.
If we use htmx to also navigate the tabs, that would mean a htmx request could either load the whole page or just the plot. That means the view should check where the request was trigged from for the htmx request. I feel like we wont be gaining much, but just making the view little complex than needed. Also in future if something changes, it could also get more complex.
Thus in general, I would like to avoid loading the whole page using the htmx as much as possible. To htmx is to load parts of the page not the whole page 🙂
EDIT: I might have found an use case where boost navigation could be more appropriate, but in different page/app (publications). Lets see 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I wouldn't mind using those separate pages (the tabs) as template partials and load them in place but that's just me :)
Otherwise, fair point! I don't see any strong argument for or against it so it's just as good
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I might change my mind on what I said earlier or I might believe it even stronger depending on my working experience with it (hx-boost) :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes we will also be learning how to use htmx correctly, so time and experience will tell :)
5bfe0d5 to
c4f417c
Compare
📋 Summary
This PR adds completely new SLU wastewater dashboard combining all active wastewater surveillance under single dashboard. This new layout was proposed by the researchers.
🛠️ Changes Made
templates/dashboards/slu_wwviews/slu_ww.pyvisualisation/slu_ww.py🔍 Notes for Reviewers
The rendered content is almost final (maybe except some styling and minor tweaks), how it is rendered is implemented with respect to the MVP. After deciding on the dashboards architecture in next phase, relevant modifications will be made.
To test this dashboard locally, kindly to the following steps.
.envfilelocalhost:8000/dashboards/wastewater-slu/data-syncand pressSync SLU databutton✅ Checklist
FREYA-XXXX: Clear and short description🔗 Jira Issue
Closes: FREYA-1640