-
-
Notifications
You must be signed in to change notification settings - Fork 34
Show publication date on petition page #299
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
Conversation
…tion date at time of publish
fallen
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.
Thanks for the PR!
It's good overall, a few fixes are needed as well as a rebase.
Thanks and sorry for taking long to review it!
| <div class="presentation"> | ||
| {% if petition.publication_date and petition.show_publication_date %} | ||
| <div class="font-weight-light font-italic"> | ||
| Published on: {{ petition.publication_date|date:"M d, Y" }} |
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.
I would say this date format is specific to english speaking countries and others but do not work for France for instance.
I would say petition.publication_date|date:"SHORT_DATE_FORMAT" would be better, don't you think?
| class Migration(migrations.Migration): | ||
|
|
||
| dependencies = [ | ||
| ('petition', '0016_signature_uses_phone_number_field'), |
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.
Now there is a migration 0017 (https://github.com/pytition/Pytition/blob/master/pytition/petition/migrations/0017_petitiontemplate_paper_signatures_enabled.py)
Could you "rebase" your PR so that your migrations depend on this one please?
| @@ -0,0 +1,18 @@ | |||
| # Generated by Django 2.2.28 on 2022-12-16 05:04 | |||
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.
This migration should now depend on 0018 and be renamed as 0019
| <div class="presentation"> | ||
| {% if petition.publication_date and petition.show_publication_date %} | ||
| <div class="font-weight-light font-italic"> | ||
| Published on: {{ petition.publication_date|date:"M d, Y" }} |
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.
Please use text that can be translated via i18n :
{% trans "Published on: "%}
|
Thanks @fallen! I've updated the PR |
fallen
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.
That's all good!
Thanks a lot for the PR and sorry again for the latency in the review!
#294