Conversation
hmpf
added
help wanted
hmpf
requested review from
elfjes,
johannaengland,
lunkwill42,
podliashanyk and
stveit
Test results 11 files 1 320 suites 49m 57s ⏱️ Results for commit d149c12. ♻️ This comment has been updated with latest results. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1361 +/- ##
==========================================
+ Coverage 78.74% 78.78% +0.03%
==========================================
Files 142 143 +1
Lines 5801 5811 +10
==========================================
+ Hits 4568 4578 +10
Misses 1233 1233 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
elfjes
left a comment
elfjes
left a comment
There was a problem hiding this comment.
Interesting. I don't have any big gripes with this other then maybe I like the current styling of the login "button". It would be nice if we could reuse the style of the current opener
perhaps something like this?
class Modal:
opener: str = "_base_form_modal_button.html"
...| @@ -1,3 +1,4 @@ | |||
| {% include logout_modal.template_name with modal=logout_modal %} | |||
There was a problem hiding this comment.
this looks weird. would be much better if it was possible to just write {{ logout_modal }}
There was a problem hiding this comment.
That's my ultimate goal but I've been struggling to make the csrf token/context outside of the modal itself available to the template. Therefore I'm working to finish everything else while doing the occasional web search for clues.
There was a problem hiding this comment.
How does django forms do this?
There was a problem hiding this comment.
The form way is what I've been doing so far, which is why I inherit from RenderableMixin. Just doing that doesn't make csrf_token available...
| "argus.htmx.user.context_processors.logout_context", | ||
| ], |
There was a problem hiding this comment.
Nice, I like how the context processor is automatically added through appconfig
| """ | ||
| How to use: | ||
|
|
||
| Append the "context_processors" list for the TEMPLATES-backend | ||
| ``django.template.backends.django.DjangoTemplates`` with the full dotted path. | ||
|
|
||
| See django settings for ``TEMPLATES``. | ||
| """ |
There was a problem hiding this comment.
Do we need something to indicate that if you're using appconfig.py, the context processors are already added?
There was a problem hiding this comment.
Hmm.. this is copypasted from another context processor so there would be more places to change. Separate PR I think.
github-project-automation
bot
moved this from ❓ Ready for review
to ♻ Changes requested
in Argus development, public
hmpf
force-pushed
the
modal-logout
branch
2 times, most recently
from
83e9ee0 to
3f0f310
Compare
|
|
I agree with @podliashanyk. Logging back into Argus is not that difficult, so even if one accidentally would log out, it's fixed very easily |
5 participants
Scope and purpose
Fixes #1352
This is an attempt at making a component. The goal is that the same template works everywhere and that any dialog-specific changes are in python.
Currently this adds a log out button in addition to the link, since we need to figure out what things should look like.
Should the button be moved up to be next to the username? Should it look like a button?
How to fix the dialog so that it has no scroll bar and is centered?
Contributor Checklist
Every pull request should have this checklist filled out, no matter how small it is.
More information about contributing to Argus can be found in the
Development docs.