-
Notifications
You must be signed in to change notification settings - Fork 94
Add delete and archive buttons in messages #1996
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
base: master
Are you sure you want to change the base?
Conversation
Bomme
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.
Hi @5aola,
many thanks for the PR! Sorry, that it took so long to get it reviewed.
I left a few comments. Let me know if you have any questions!
templates/messages/message.html
Outdated
| href="" | ||
| data-action-value="a"> | ||
| {% if message.is_archived %}{% bw_icon "box-remove" %}{% else %}{% bw_icon "box-add" %}{% endif %} | ||
| </i>{% if message.is_archived %}Unarchive{% else %}Archive{% endif %}</a>{% endif %} |
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.
Closing tag </i> matches nothing
templates/messages/message.html
Outdated
| class="h-spacing-left-2 btn-inverse no-hover message-action" | ||
| href="" | ||
| data-action-value="dm"> | ||
| {% bw_icon "trash" %}</i>Delete</a> |
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.
Closing tag </i> matches nothing
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 change is a bit hacky and it took me a while to understand why this works.
Maybe it's better to add a form to templates/messages/message.html?
templates/messages/message.html
Outdated
| </div> | ||
| <div class="col-12"> | ||
| <div class="v-spacing-top-5 text-center"> | ||
| <div class="v-spacing-top-5 text-center" data-message-id="{{ message.id }}"> |
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.
Maybe better add a form
templates/messages/message.html
Outdated
| {% endblock %} | ||
|
|
||
| {% block extrabody %} | ||
| <script src="{% static 'bw-frontend/dist/messages.js' %}"></script> |
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 think we should not rely on messages.js here.
A simple form / POST --> to messages-change-state might be cleaner.
See comment above (line 51)
|
Thanks for the feedback. My initial thinking was just to recreate a very similar behavior to the other delete and archive buttons at display_message.html, but it makes sense to just use forms POST call to trigger the original |
Issue(s)
#172
Description
Added buttons to delete and archive the message when it's in detailed view.
The features of the buttons are kept original. If the message is deleted, it redirects to the messages list page.