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.

Sign up for GitHub

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

allow dates to be specified as a year only #921

Closed
wants to merge 1 commit into from
Closed

allow dates to be specified as a year only #921

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions bookwyrm/forms.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,15 @@ class Meta:
"subject_places", # TODO
"connector",
]
def __init__(self, data=None, *args, **kwargs):
#print(data)
if data and data.get('first_published_year_only', None):
mutable = data._mutable
data._mutable = True
data['first_published_date'] += '-01-01'
data['published_date'] += '-01-01'
data._mutable = mutable
super().__init__(data, *args, **kwargs)


class AuthorForm(CustomForm):
Expand Down
24 changes: 24 additions & 0 deletions bookwyrm/migrations/0062_auto_20210408_0032.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# Generated by Django 3.1.6 on 2021-04-08 00:32

import bookwyrm.models.fields
from django.db import migrations


class Migration(migrations.Migration):

dependencies = [
('bookwyrm', '0061_auto_20210402_1435'),
]

operations = [
migrations.AddField(
model_name='book',
name='first_published_year_only',
field=bookwyrm.models.fields.BooleanField(default=False),
),
migrations.AddField(
model_name='book',
name='published_year_only',
field=bookwyrm.models.fields.BooleanField(default=False),
),
]
2 changes: 2 additions & 0 deletions bookwyrm/models/book.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,9 @@ class Book(BookDataModel):
upload_to="covers/", blank=True, null=True, alt_field="alt_text"
)
first_published_date = fields.DateTimeField(blank=True, null=True)
first_published_year_only = fields.BooleanField(default=False)
published_date = fields.DateTimeField(blank=True, null=True)
published_year_only = fields.BooleanField(default=False)

objects = InheritanceManager()

Expand Down
55 changes: 51 additions & 4 deletions bookwyrm/templates/book/edit_book.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,45 @@

{% block title %}{% if book %}{% blocktrans with book_title=book.title %}Edit "{{ book_title }}"{% endblocktrans %}{% else %}{% trans "Add Book" %}{% endif %}{% endblock %}

{% block scripts %}
<script defer>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Depending on where this PR is gonna go, I can help you with JS if you’re seeking assistance; just ping me if you wish.

Just two things for the sake of sharing:

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i appreciate that! i'll definitely ping if something comes up. i used an inline script just for development, but i didn't know that about defer, thank you!

document.getElementById("id_first_published_year_only")
.addEventListener(
"input",
setToYearOnly(
"id_first_published_date",
"label_first_published_date",
"label_first_published_year_only"),
{passive: true});
document.getElementById("id_published_year_only")
.addEventListener(
"input",
setToYearOnly(
"id_published_date",
"label_published_date",
"label_published_year_only"),
{passive: true});
function setToYearOnly(date_input_id, date_label_id, year_label_id) {
function setFirstPublishedToYearOnly(event) {
let first_published_date_input = document.getElementById(date_input_id);
let first_published_date_label = document.getElementById(date_label_id);
let first_published_year_label = document.getElementById(year_label_id);
// potential HACK: older ie browsers require a different method call: https://gist.github.com/tzi/2953155#file-important-js-L5
if (event.target.checked) {
first_published_date_label.setAttribute("style", "display: none !important");
first_published_year_label.setAttribute("style", "display: block");
first_published_date_input.type = "number";
} else {
first_published_date_label.setAttribute("style", "display: block");
first_published_year_label.setAttribute("style", "display: none !important");
first_published_date_input.type = "date";
}
Comment on lines +30 to +39
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your comment raises an interesting question: Does BookWyrm want to define a set of supported browser for a full-feature experience?

If yes, which ones? (That would definitely go into a .browserslistrc file. :) )

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my personal projects are always "the browsers supported are whatever browsers support the features i need without polyfills and work-arounds". that's definitely not accessible enough for a project with more reach and dev power though (imo)

i recently read an article (that i've lost, sadly) talking about how few people actually use internet explorer. my personal opinion is that ie is end-of-life for several years now, bookwyrm's potential userbase probably has no overlap with any of the people using ie, and the added size of polyfills and work-arounds is more of an accessibility issue than one or two people being unable to use certain js features

as for non-ie browsers to be supported, i'd have to defer to you! i think the js here makes it pretty clear that i'm not a front-end person :p

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I completely second the progressive enhancement approach.

My question was more about a full-feature experience, where feature meant UX/UI features more than the content provided. I hope the intention is clearer. :)

I won’t regret not caring about IE specifically as long as the content is available to the few people using it.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i definitely understand what you mean now! i think it might be a good idea to explicitly list several target browsers, so we can make sure we're usable with more than just the main two or three

}
return setFirstPublishedToYearOnly;
}
</script>
{% endblock %}

{% block content %}
<header class="block">
<h1 class="title level-left">
Expand Down Expand Up @@ -129,16 +168,24 @@ <h2 class="title is-4">{% trans "Metadata" %}</h2>
{% endfor %}

<p class="mb-2">
<label class="label" for="id_first_published_date">{% trans "First published date:" %}</label>
<input type="date" name="first_published_date" class="input" id="id_first_published_date"{% if form.first_published_date.value %} value="{{ form.first_published_date.value|date:'Y-m-d' }}"{% endif %}>
<label class="label" for="id_first_published_date" id="label_first_published_date">{% trans "First published date:" %}</label>
<label class="label" for="id_first_published_date" id="label_first_published_year_only" style="display: none;">{% trans "First published year:" %}</label>
<label class="label is-inline-block" for="id_first_published_year_only">{% trans "Year only:" %}</label>
<input type="checkbox" name="first_published_year_only" class="checkbox" id="id_first_published_year_only"{% if form.first_published_year_only.value %} checked{% endif %}>
<input
{% if form.first_published_year_only.value %}type="number"{% else %}type="date"{% endif %} name="first_published_date" class="input" id="id_first_published_date"{% if form.first_published_year_only.value and form.first_published_date.value%} value="{{ form.first_published_date.value|date:'Y' }}"{% elif form.first_published_date.value %} value="{{ form.first_published_date.value|date:'Y-m-d' }}"{% endif %}>
</p>
{% for error in form.first_published_date.errors %}
<p class="help is-danger">{{ error | escape }}</p>
{% endfor %}

<p class="mb-2">
<label class="label" for="id_published_date">{% trans "Published date:" %}</label>
<input type="date" name="published_date" class="input" id="id_published_date"{% if form.published_date.value %} value="{{ form.published_date.value|date:'Y-m-d'}}"{% endif %}>
<label class="label" for="id_published_date" id="label_published_date">{% trans "Published date:" %}</label>
<label class="label" for="id_published_date" id="label_published_year_only" style="display: none;">{% trans "Published year:" %}</label>
<label class="label is-inline-block" for="id_published_year_only">{% trans "Year only:" %}</label>
<input type="checkbox" name="published_year_only" class="checkbox" id="id_published_year_only"{% if form.published_year_only.value %} checked{% endif %}>
<input
{% if form.published_year_only.value %}type="number"{% else %}type="date"{% endif %} name="published_date" class="input" id="id_published_date"{% if form.published_year_only.value and form.published_date.value%} value="{{ form.published_date.value|date:'Y' }}"{% elif form.published_date.value %} value="{{ form.published_date.value|date:'Y-m-d' }}"{% endif %}>
</p>
{% for error in form.published_date.errors %}
<p class="help is-danger">{{ error | escape }}</p>
Expand Down
14 changes: 11 additions & 3 deletions bookwyrm/templates/book/publisher_info.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,18 @@
{% endif %}
<p>
{% if book.published_date and book.publishers %}
{% blocktrans with date=book.published_date|date:'M jS Y' publisher=book.publishers|join:', ' %}Published {{ date }} by {{ publisher }}.{% endblocktrans %}
{% if book.published_year_only %}
{% blocktrans with date=book.published_date|date:'Y' publisher=book.publishers|join:', ' %}Published {{ date }} by {{ publisher }}.{% endblocktrans %}
{% else %}
{% blocktrans with date=book.published_date|date:'M jS Y' publisher=book.publishers|join:', ' %}Published {{ date }} by {{ publisher }}.{% endblocktrans %}
{% endif %}
{% elif book.published_date %}
{% blocktrans with date=book.published_date|date:'M jS Y' %}Published {{ date }}{% endblocktrans %}
{% if book.published_year_only %}
{% blocktrans with date=book.published_date|date:'Y' %}Published {{ date }}{% endblocktrans %}
{% else %}
{% blocktrans with date=book.published_date|date:'M jS Y' %}Published {{ date }}{% endblocktrans %}
{% endif %}
{% elif book.publishers %}
{% blocktrans with publisher=book.publishers|join:', ' %}Published by {{ publisher }}.{% endblocktrans %}
{% blocktrans with publisher=book.publishers|join:', ' %}Published by {{ publisher }}.{% endblocktrans %}
{% endif %}
</p>
4 changes: 3 additions & 1 deletion bookwyrm/views/books.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ def post(self, request, book_id=None):

# either of the above cases requires additional confirmation
if add_author or not book:
# creting a book or adding an author to a book needs another step
# creating a book or adding an author to a book needs another step
data["confirm_mode"] = True
# this isn't preserved because it isn't part of the form obj
data["remove_authors"] = request.POST.getlist("remove_authors")
Expand All @@ -182,6 +182,8 @@ def post(self, request, book_id=None):
formcopy["first_published_date"]
)
except (MultiValueDictKeyError, ValueError):
# either form didn't have the data, or strptime failed
# let the template handle it
pass
try:
formcopy["published_date"] = dateparse(formcopy["published_date"])
Expand Down