Skip to content

Add border to non federated notifications for admins and moderators #1432

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

Draft
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

TheVillageGuy
Copy link
Contributor

Added new_signup block

TheVillageGuy and others added 11 commits December 14, 2024 18:09
Add border to non federated notifications for admins and moderators

This allows admins/moderators (when they receive notifications in the future) to quickly see if there has been any activity by local users on their instance
Title shows entry as local and author
remove bordering from here
Saves a lot of var checking and gives more potential for future visual improvements in the notifications
@melroy89
Copy link
Member

melroy89 commented Feb 6, 2025

What are those random ><div> doing in the _blocks.html.twig??

{{ component('user_inline', {user: notification.comment.user}) }} {{ 'replied_to_your_comment'|trans|lower }} - <a
href="{{ entry_comment_view_url(notification.comment) }}#{{ get_url_fragment(notification.comment) }}">{{ notification.comment.shortTitle }}</a>
{% endblock entry_comment_reply_notification %}

{% block entry_comment_deleted_notification %}
><div>
Copy link
Member

Choose a reason for hiding this comment

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

I honestly thing that this is just bad practice... It might do the job but it is not easy to maintain or to read...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, though I don't see another way of achieving it without reorganizing the entire notifications code

Copy link
Member

Choose a reason for hiding this comment

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

You can set the class in an if in the front.html.twig inside of the notifications folder. As far as I can tell you just want to do it for specific notification types right? You have the notification.type variable there to check that

Copy link
Contributor Author

@TheVillageGuy TheVillageGuy Feb 10, 2025

Choose a reason for hiding this comment

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

So creating a new class for every kind of notification, I guess that would work. The benefit of having the logic in the blocks is that in the future we can do more than just a coloured border, like adding a symbol for different situations, I'm not sure what idea I had on that about though, I think indicating when something is done by a local user, not just locally.

@TheVillageGuy TheVillageGuy marked this pull request as draft February 11, 2025 14:58
Copy link
Contributor

This PR is stale because it has been open 40 days with no activity.

@github-actions github-actions bot added the Stale Inactivity for too long label Mar 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Stale Inactivity for too long
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants