Skip to content
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

Documentation enhancements #31

Closed
shacker opened this issue Aug 31, 2017 · 4 comments
Closed

Documentation enhancements #31

shacker opened this issue Aug 31, 2017 · 4 comments

Comments

@shacker
Copy link

shacker commented Aug 31, 2017

I found it a bit challenging to get the static media wired up. Even though the docs suggesting seeing the upstream repos for more information, they weren't super helpful either.

So that staticfilesfinder can find the js assets, this must be added to INSTALLED_APPS:

'zxcvbn_password',

You refer to form.media, but that doesn't work because the example form in the docs doesn't have a Media: class. However, I think it's easier to just include JS in the template:

{% load static %}
...
<script src="{% static 'zxcvbn_password/js/zxcvbn.js' %}"></script>
<script src="{% static 'zxcvbn_password/js/password_strength.js' %}"></script>

With that done, it started working but the indicator bar never changed color. Had to add to my css:

.progress-bar-warning {
    background-color: yellow;
}

.progress-bar-danger {
    background-color: red;
}

.progress-bar-success {
    background-color: green;
}

Finally, a little guidance on processing a valid password would be helpful (since we don't see Django's set_password() all that often:

        if form.is_valid():
            user = request.user
            user.set_password(form.cleaned_data['password1'])
            user.save()

I'll do a PR if you approve of the idea.

@pawamoy
Copy link
Owner

pawamoy commented Sep 10, 2017

Thanks for your report. Yes a PR would be much appreciated :) Please separate code and documentation in different PRs of course. Just came back from holidays, lot of work to catch up ^^

@shacker
Copy link
Author

shacker commented Sep 13, 2017

Hi -

I actually ended up doing a more manual implementation of zxcvbn rather than using this library. So since I'm not using it, am probably not the best person to do the PR after all. Should be an easy doc tweak though. Thanks!

@pawamoy
Copy link
Owner

pawamoy commented Mar 7, 2018

So, to get back at this:

  • doc about adding zxcvbn_password in INSTALLED_APPS: done thanks to @beruic
  • note about upstream docs: I will remove this note since it does not seem to be helpful
  • note about form.media: I will update this note to be more clear: including {{ form.media }} in the JS block of your template will in fact include media from all form's fields and widgets. In this app, the only declared media is in the PasswordStrengthInput widget.
  • colors not working: indeed the JS code relies on jquery and bootstrap 😕 I will add a note about these requirements in the README.
  • guidance about Django's set_password: it's an example among many others, I personally don't use set_password. Usually you just User.objects.create(username=..., password=...) with username and password retrieved from the form cleaned data.

@pawamoy
Copy link
Owner

pawamoy commented Mar 7, 2018

Thanks @shacker for your feedback!

@pawamoy pawamoy closed this as completed Mar 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants