[openstreetmap/openstreetmap-website] On-site notifications (PR #7030)

[Minh Nguyễn]

@1ec5 commented on this pull request.

Yay, I’m excited to use this even before extra polish, instead of missing e-mail notifications that get lost in my inbox!

> +    <%= t(
+          ".notification_header_html",
+          :changeset_comment_link => link_to(
+            t(".changeset_comment"),
+            changeset_path(
+              notification.changeset,
+              :anchor => "c#{notification.comment_id}"
+            )
+          ),
+          :commenter_name_link => link_to(
+            notification.commenter.display_name,
+            notification.commenter
+          ),
+          :time_ago => friendly_date_ago(notification.timestamp)
+        ) %>

> All other notification types.

As we add more notification types, would there be any opportunity to factor out some common elements to avoid boilerplate? This code to generate the header seems like a candidate for that, except that some notification types probably won’t have anything analogous to a commenter or a page to link to. 

Since the intention is to communicate all the information that’s currently going out in e-mails, what if we add a consistent heading that’s based on the subject line of those e-mails? For simplicity, it could remain as plain text. Then, these details specific to changeset comments could be a separate line below it. Or maybe the code to generate a rich text heading could live alongside the existing code that generates the plain text subject line.

> @@ -0,0 +1,7 @@
+<% content_for :heading do %>
+  <h1><%= t ".title" %></h1>
+<% end %>
+
+<% @notifications.each do |notification| %>
+  <%= render :partial => notification.to_partial_path, :object => notification, :as => :notification %>
+<% end %>

This would be a good opportunity to link to the notification preferences in #7001, depending on which PR lands first.

> +            t(".changeset_comment"),
+            changeset_path(
+              notification.changeset,
+              :anchor => "c#{notification.comment_id}"
+            )
+          ),
+          :commenter_name_link => link_to(
+            notification.commenter.display_name,
+            notification.commenter
+          ),
+          :time_ago => friendly_date_ago(notification.timestamp)
+        ) %>
+  </p>
+  <blockquote class="mx-2 fst-italic">
+    <p class="text-truncate"><%= notification.comment_body.truncate(200, :separator => " ") %></p>
+  </blockquote>

Should we add some kind of action button below or to the right of the notification, like an “Open Comment” button that does the same thing as the “Changeset Comment” link currently does? It would be a nice place to put a Mark as Read/Unread button once we have that functionality, similar to what we have in the Inbox.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/7030?email_source=notifications&email_token=AAK2OLNRFLICADGOJHJS75L4XJE3FA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTIMJWGQZTENZRG43KM4TFMFZW63VKON2WE43DOJUWEZLEUVSXMZLOOS6XA4S7OJSXM2LFO5PW433UNFTGSY3BORUW63TTL5RWY2LDNM#pullrequestreview-4164327176
You are receiving this because you are subscribed to this thread.

Message ID: <openstreetmap/openstreetmap-website/pull/7030/review/4164327176 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20260423/37e1682a/attachment.htm>