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

Jump to bottom

Add view partials for repeated content on post or topic writer #221

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Add view partials for repeated content on post or topic writer #221

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d35c029
Add view partials for repeated content on post or topic writer
carofun Feb 8, 2021
bb440c9
fixup! Add view partials for repeated content on post or topic writer
carofun Mar 19, 2021
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
22 changes: 2 additions & 20 deletions machina/templates/machina/forum/forum_list.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,16 +72,7 @@ <h3 class="m-0 card-title h5 text-dark">{% trans "Forums" %}</h3>
<div class="py-2 col-md-1 d-none d-md-block text-center forum-count">{{ node.posts_count }}</div>
<div class="py-2 col-md-2 col-sm-3 d-none d-sm-block forum-last-post">
{% if node.last_post %}
{% if node.last_post.poster %}
{% url 'forum_member:profile' node.last_post.poster_id as poster_url %}
{% blocktrans trimmed with poster_url=poster_url username=node.last_post.poster|forum_member_display_name %}
By: <a href="{{ poster_url }}">{{ username }}</a>
{% endblocktrans %}
{% else %}
{% blocktrans trimmed with poster_username=node.last_post.username %}
By: {{ poster_username }}
{% endblocktrans %}
{% endif %}
{% include "partials/posted_by.html" with item=node.last_post %}
&nbsp;<a href="{% url 'forum_conversation:topic' node.obj.slug node.obj.pk node.last_post.topic.slug node.last_post.topic.pk %}?post={{ node.last_post.pk }}#{{ node.last_post.pk }}"><i class="fas fa-arrow-circle-right "></i></a>
<br />
<small>{{ node.last_post.created }}</small>
Expand Down Expand Up @@ -145,16 +136,7 @@ <h3 class="m-0 card-title h5 text-dark">{% trans "Forums" %}</h3>
<div class="py-2 col-md-1 d-none d-md-block text-center forum-count">{{ node.posts_count }}</div>
<div class="py-2 col-md-2 col-sm-3 d-none d-sm-block forum-last-post">
{% if node.last_post %}
{% if node.last_post.poster_id %}
{% url 'forum_member:profile' node.last_post.poster_id as poster_url %}
{% blocktrans trimmed with poster_url=poster_url username=node.last_post.poster|forum_member_display_name %}
By: <a href="{{ poster_url }}">{{ username }}</a>
{% endblocktrans %}
{% else %}
{% blocktrans trimmed with poster_username=node.last_post.username %}
By: {{ poster_username }}
{% endblocktrans %}
{% endif %}
{% include "partials/posted_by.html" with item=node.last_post %}
&nbsp;<a href="{% url 'forum_conversation:topic' node.obj.slug node.obj.pk node.last_post.topic.slug node.last_post.topic.pk %}?post={{ node.last_post.pk }}#{{ node.last_post.pk }}"><i class="fas fa-arrow-circle-right "></i></a>
<br />
<small>{{ node.last_post.created }}</small>
Expand Down
15 changes: 4 additions & 11 deletions machina/templates/machina/forum_conversation/post_create.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,16 +46,7 @@ <h4 class="subject">{{ post.subject }}</h4>
<p><small class="text-muted">
{% spaceless %}
<i class="fa fa-clock-o"></i>&nbsp;
{% if post.poster %}
{% url 'forum_member:profile' post.poster_id as poster_url %}
{% blocktrans trimmed with poster_url=poster_url username=post.poster|forum_member_display_name creation_date=post.created %}
By: <a href="{{ poster_url }}">{{ username }}</a> on {{ creation_date }}
{% endblocktrans %}
{% else %}
{% blocktrans trimmed with poster_username=post.username creation_date=post.created %}
By: {{ poster_username }} on {{ creation_date }}
{% endblocktrans %}
{% endif %}
{% include "partials/posted_by.html" with item=post show_creation_date=True%}
{% endspaceless %}
</small></p>
<div class="post-content">
Expand All @@ -64,7 +55,9 @@ <h4 class="subject">{{ post.subject }}</h4>
{% include "forum_conversation/forum_attachments/attachments_detail.html" %}
</div>
<div class="col-md-2 post-sidebar">
<div class="username">{% if post.poster %}<a href="{% url 'forum_member:profile' post.poster_id %}"><b>{{ post.poster|forum_member_display_name }}</b></a>{% else %}<b>{{ post.username }}</b>{% endif %}</div>
<div class="username">
{% include "partials/posted_by.html" with item=post highlight_username=True show_by_prefix=True %}
</div>
</div>
</div>
{% endfor %}
Expand Down
17 changes: 5 additions & 12 deletions machina/templates/machina/forum_conversation/topic_detail.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,21 +45,14 @@ <h4 class="m-0 subject">
</h4>
{% endspaceless %}
<p>
{% block written_by %}
<small class="text-muted">
{% spaceless %}
<i class="fas fa-clock"></i>&nbsp;
{% if post.poster %}
{% url 'forum_member:profile' post.poster_id as poster_url %}
{% blocktrans trimmed with poster_url=poster_url username=post.poster|forum_member_display_name creation_date=post.created %}
By: <a href="{{ poster_url }}">{{ username }}</a> on {{ creation_date }}
{% endblocktrans %}
{% else %}
{% blocktrans trimmed with poster_username=post.username creation_date=post.created %}
By: {{ poster_username }} on {{ creation_date }}
{% endblocktrans %}
{% endif %}
{% include "partials/posted_by.html" with item=post show_creation_date=True%}
{% endspaceless %}
</small>
{% endblock %}
</p>
<div class="post-content">
{{ post.content.rendered }}
Expand All @@ -73,7 +66,7 @@ <h4 class="m-0 subject">
{% if post.updates_count %}
<div class="mt-4 edit-info">
<small class="text-muted">
<i class="fas fa-edit"></i>&nbsp;{% if post.updated_by %}{% trans "Last edited by:" %}&nbsp;<a href="{% url 'forum_member:profile' post.updated_by_id %}">{{ post.updated_by|forum_member_display_name }}</a>&nbsp;{% else %}{% trans "Updated" %}&nbsp;{% endif %}{% trans "on" %}&nbsp;{{ post.updated }}, {% blocktrans count counter=post.updates_count %}edited {{counter }} time in total.{% plural %}edited {{counter }} times in total.{% endblocktrans %}
<i class="fas fa-edit"></i>&nbsp;{% if post.updated_by %}{% trans "Last edited by:" %}&nbsp;{% include "partials/posted_by.html" with item=post.updated_by show_by_prefix=True%}&nbsp;{% else %}{% trans "Updated" %}&nbsp;{% endif %}{% trans "on" %}&nbsp;{{ post.updated }}, {% blocktrans count counter=post.updates_count %}edited {{counter }} time in total.{% plural %}edited {{counter }} times in total.{% endblocktrans %}
</small>
{% if post.update_reason %}
<br />
Expand All @@ -91,7 +84,7 @@ <h4 class="m-0 subject">
{% include "partials/avatar.html" with profile=post.poster.forum_profile show_placeholder=True %}
</a>
</div>
<div class="username"><a href="{% url 'forum_member:profile' post.poster_id %}"><b>{{ post.poster|forum_member_display_name }}</b></a></div>
<div class="username">{% include "partials/posted_by.html" with item=post highlight_username=True show_by_prefix=True %}</div>
<div class="posts-count text-muted"><b>{% trans "Posts:" %}</b>&nbsp;{{ post.poster.forum_profile.posts_count }}</div>
{% else %}
<div class="username"><b>{{ post.username }}</b></div>
Expand Down
22 changes: 2 additions & 20 deletions machina/templates/machina/forum_conversation/topic_list.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,16 +29,7 @@
<a href="{% url 'forum_conversation:topic' topic.forum.slug topic.forum.pk topic.slug topic.pk %}" class="topic-name-link">{{ topic.subject }}</a>{% if topic.is_locked %}&nbsp;<i class="fas fa-times-circle locked-indicator" title="{% trans 'This topic is locked' %}"></i>{% endif %}
<div>
<div class="topic-created">
{% if topic.poster %}
{% url 'forum_member:profile' topic.poster_id as poster_url %}
{% blocktrans trimmed with poster_url=poster_url username=topic.poster|forum_member_display_name creation_date=topic.created %}
By: <a href="{{ poster_url }}">{{ username }}</a> on {{ creation_date }}
{% endblocktrans %}
{% else %}
{% blocktrans trimmed with poster_username=topic.first_post.username creation_date=topic.created %}
By: {{ poster_username }} on {{ creation_date }}
{% endblocktrans %}
{% endif %}
{% include "partials/posted_by.html" with item=topic is_topic=True show_creation_date=True %}
</div>
</div>
</td>
Expand All @@ -49,16 +40,7 @@
<div class="py-2 col-md-1 d-none d-md-block text-center topic-count">{{ topic.views_count }}</div>
<div class="py-2 col-md-2 col-sm-3 d-none d-sm-block topic-last-post">
{% with last_post=topic.last_post %}
{% if last_post.poster %}
{% url 'forum_member:profile' last_post.poster_id as poster_url %}
{% blocktrans trimmed with poster_url=poster_url username=last_post.poster|forum_member_display_name %}
By: <a href="{{ poster_url }}">{{ username }}</a>
{% endblocktrans %}
{% else %}
{% blocktrans trimmed with poster_username=last_post.username %}
By: {{ poster_username }}
{% endblocktrans %}
{% endif %}
{% include "partials/posted_by.html" with item=last_post %}
&nbsp;<a href="{% url 'forum_conversation:topic' topic.forum.slug topic.forum.pk topic.slug topic.pk %}?post={{ last_post.pk }}#{{ last_post.pk }}"><i class="fas fa-arrow-circle-right "></i></a>
<br />
<small>{{ last_post.created }}</small>
Expand Down
7 changes: 2 additions & 5 deletions machina/templates/machina/forum_member/user_posts_list.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,7 @@ <h4 class="subject">
<small class="text-muted">
{% spaceless %}
<i class="fas fa-clock"></i>&nbsp;
{% url 'forum_member:profile' post.poster_id as poster_url %}
{% blocktrans trimmed with poster_url=poster_url username=post.poster|forum_member_display_name creation_date=post.created %}
By: <a href="{{ poster_url }}">{{ username }}</a> on {{ creation_date }}
{% endblocktrans %}
{% include "partials/posted_by.html" with item=post show_creation_date=True %}
{% endspaceless %}
</small>
</p>
Expand All @@ -63,7 +60,7 @@ <h4 class="subject">
{% include "partials/avatar.html" with profile=post.poster placeholder=False %}
</a>
</div>
<div class="username"><a href="{% url 'forum_member:profile' post.poster_id %}"><b>{{ post.poster|forum_member_display_name }}</b></a></div>
<div class="username">{% include "partials/posted_by.html" with item=post highlight_username=True show_by_prefix=True %}</div>
<div class="posts-count text-muted"><b>{% trans "Posts:" %}</b>&nbsp;{{ post.poster.forum_profile.posts_count }}</div>
</div>
</div>
Expand Down
26 changes: 4 additions & 22 deletions machina/templates/machina/forum_moderation/moderation_queue/detail.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,16 +81,7 @@ <h4 class="subject">{{ post.subject }}</h4>
<small class="text-muted">
{% spaceless %}
<i class="fas fa-clock"></i>&nbsp;
{% if post.poster %}
{% url 'forum_member:profile' post.poster_id as poster_url %}
{% blocktrans trimmed with poster_url=poster_url username=post.poster|forum_member_display_name creation_date=post.created %}
By: <a href="{{ poster_url }}">{{ username }}</a> on {{ creation_date }}
{% endblocktrans %}
{% else %}
{% blocktrans trimmed with poster_username=post.username creation_date=post.created %}
By: {{ poster_username }} on {{ creation_date }}
{% endblocktrans %}
{% endif %}
{% include "partials/posted_by.html" with item=post show_creation_date=True%}
{% endspaceless %}
</small>
</p>
Expand All @@ -113,7 +104,7 @@ <h4 class="subject">{{ post.subject }}</h4>
</a>
</div>
{% endif %}
<div class="username"><a href="{% url 'forum_member:profile' post.poster_id %}"><b>{{ post.poster|forum_member_display_name }}</b></a></div>
<div class="username">{% include "partials/posted_by.html" with item=post highlight_username=True show_by_prefix=True %}</div>
<div class="posts-count text-muted"><b>{% trans "Posts:" %}</b>&nbsp;{{ post.poster.forum_profile.posts_count }}</div>
{% else %}
<div class="username"><b>{{ post.username }}</b></div>
Expand All @@ -140,16 +131,7 @@ <h4 class="subject">{{ post.subject }}</h4>
<p><small class="text-muted">
{% spaceless %}
<i class="fas fa-clock"></i>&nbsp;
{% if post.poster %}
{% url 'forum_member:profile' post.poster_id as poster_url %}
{% blocktrans trimmed with poster_url=poster_url username=post.poster|forum_member_display_name creation_date=post.created %}
By: <a href="{{ poster_url }}">{{ username }}</a> on {{ creation_date }}
{% endblocktrans %}
{% else %}
{% blocktrans trimmed with poster_username=post.username creation_date=post.created %}
By: {{ poster_username }} on {{ creation_date }}
{% endblocktrans %}
{% endif %}
{% include "partials/posted_by.html" with item=post show_creation_date=True%}
{% endspaceless %}
</small></p>
<div class="post-content">
Expand All @@ -158,7 +140,7 @@ <h4 class="subject">{{ post.subject }}</h4>
{% include "forum_conversation/forum_attachments/attachments_detail.html" %}
</div>
<div class="col-md-2 post-sidebar">
<div class="username">{% if post.poster %}<a href="{% url 'forum_member:profile' post.poster_id %}"><b>{{ post.poster|forum_member_display_name }}</b></a>{% else %}<b>{{ post.username }}</b>{% endif %}</div>
<div class="username">{% include "partials/posted_by.html" with item=post highlight_username=True show_by_prefix=True %}</div>
</div>
</div>
{% endfor %}
Expand Down
13 changes: 3 additions & 10 deletions machina/templates/machina/forum_moderation/moderation_queue/list.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,16 +46,9 @@ <h3 class="m-0 card-title h5">{% trans "Moderation queue" %}</h3>
<a href="{% url 'forum_moderation:queued_post' post.pk %}" class="post-name-link">{{ post.subject }}</a>
<div>
<div class="post-created">
{% if post.poster %}
{% url 'forum_member:profile' post.poster_id as poster_url %}
{% blocktrans trimmed with poster_url=poster_url username=post.poster|forum_member_display_name creation_date=post.created %}
By: <a href="{{ poster_url }}">{{ username }}</a> on {{ creation_date }}
{% endblocktrans %}
{% else %}
{% blocktrans trimmed with poster_username=post.username creation_date=post.created %}
By: {{ poster_username }} on {{ creation_date }}
{% endblocktrans %}
{% endif %}
{% block written_by %}
{% include "partials/posted_by.html" with item=post show_creation_date=True %}
{% endblock %}
</div>
</div>
</td>
Expand Down
13 changes: 3 additions & 10 deletions machina/templates/machina/forum_search/search.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,16 +78,9 @@ <h3 class="m-0 card-title h5">
<a href="{% url 'forum_conversation:topic' result.forum_slug result.forum result.topic_slug result.topic %}" class="topic-name-link">{{ result.topic_subject }}</a>
<div>
<div class="post-created">
{% if result.poster %}
{% url 'forum_member:profile' result.poster as poster_url %}
{% blocktrans trimmed with poster_url=poster_url username=result.poster_name creation_date=result.created %}
By: <a href="{{ poster_url }}">{{ username }}</a> on {{ creation_date }}
{% endblocktrans %}
{% else %}
{% blocktrans trimmed with poster_username=result.poster_name creation_date=result.created %}
By: {{ poster_username }} on {{ creation_date }}
{% endblocktrans %}
{% endif %}
{% block written_by %}
{% include "partials/posted_by.html" with item=result show_creation_date=True is_search=True %}
{% endblock %}
</div>
</div>
</td>
Expand Down
59 changes: 59 additions & 0 deletions machina/templates/machina/partials/posted_by.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
{% load forum_member_tags %}
Copy link
Owner

Choose a reason for hiding this comment

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

I think we could probably improve how this partial works so that it - kindof - keep working like things were working previously.

For example why not:

  • have a partial that either works from a single post variable or topic variable (depending on whether you want to display the poster info for the post or the topic itself) - this would allow to avoid passing poster, username and creation_date systematically in each partial use (which would be less error prone in my opinion)
  • then in order to control whether the creation date has to be displayed, you could use an extra show_creation_date variable (True / False)
  • no_trans would be replaced by something more explicit like show_by_prefix
  • username_highlighted would be replaced by highlight_username (True / False) to improve consistency

What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, definitely, I think it's way simpler this way. I'll work around it and suggest it as a fix.
thank you for you feedback

Copy link
Contributor Author

@carofun carofun Mar 19, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I've commited with a fixup, I had forgotten to add a file with the search in the previous commit which was another case.
I don't know what to think of this new version. I had to make more cases to be able to read username as it is not specified anymore.
Tell me please what you think of it.

{% load i18n %}

{% if is_search %}
{% if item.poster %}
{% url 'forum_member:profile' item.poster as poster_url %}
{% blocktrans trimmed with username=item.poster_name creation_date=item.created %}
By: <a href="{{ poster_url }}">{{ username }}</a> on {{ creation_date }}
{% endblocktrans %}
{% else %}
{% blocktrans trimmed with poster_username=item.poster_name creation_date=item.created %}
By: {{ poster_username }} on {{ creation_date }}
{% endblocktrans %}
{% endif %}
{% else %}
Comment on lines +4 to +15
Copy link
Owner

Choose a reason for hiding this comment

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

The fact that we need to pass is_search=True in order to render this partial templates makes me think that it's probably equivalent to simply let the initial search template untouched. Otherwise would it be possible to make this partial template generic enough so that we don't have to use this is_search variable? If the partial is generic enough, it shouldn't make any assumption regarding the context in which it is used. 🤷‍♂️

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi I had a second look at it, IMO, we should pass through as we did before (before the fixup), the username, the creation date and poster now that we have 3 cases : topics, posts and search, this way the partial will be generic.
Before I go for it, I prefer to have your feedback.
Thank you

Copy link
Owner

Choose a reason for hiding this comment

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

We can try! Though I think the resulting partial should be as simple as possible and not contain too much logic.

{% if item.poster %}
{% url 'forum_member:profile' item.poster_id as profil_url %}
{% if show_by_prefix %}
<a href="{{profil_url }}">{% if highlight_username %}<b>{% endif %}{{ item.poster|forum_member_display_name }}{% if highlight_username %}</b>{% endif %}</a>
{% else %}
{% if show_creation_date %}
{% blocktrans trimmed with username=item.poster|forum_member_display_name creation_date=item.created %}
By: <a href="{{ profil_url }}">{{ username }}</a> on {{ creation_date }}
{% endblocktrans %}
{% else %}
{% blocktrans trimmed with username=item.poster|forum_member_display_name %}
By: <a href="{{ profil_url }}">{{ username }}</a>
{% endblocktrans %}
{% endif %}
{% endif %}
{% else %}
{% if show_by_prefix %}
{% if highlight_username %}<b>{% endif %}{{ username }}{% if highlight_username %}</b>{% endif %}
{% else %}
{% if show_creation_date %}
{% if is_topic %}
{% blocktrans trimmed with username=topic.first_post.username creation_date=item.created %}
By: {{ username }} on {{ creation_date }}
{% endblocktrans %}
{% else %}
{% blocktrans trimmed with username=item.username creation_date=item.created %}
By: {{ username }} on {{ creation_date }}
{% endblocktrans %}
{% endif %}
{% else %}
{% if is_topic %}
{% blocktrans trimmed with username=topic.first_post.username %}
By: {{ username }}
{% endblocktrans %}
{% else %}
{% blocktrans trimmed with username=item.username %}
By: {{ username }}
{% endblocktrans %}
{% endif %}
{% endif %}
{% endif %}
{% endif %}
{% endif %}