fix(17): address all code review findings
- CR-01: add id="discussion-message-list" to .divide-y; change hx-target to #discussion-message-list so HTMX appends inside the list, not after it. Always render the list div so the target exists even when messages is empty. - WR-01: SSE broadcast now renders IsOwn=false for all recipients so other users don't receive the sender's right-aligned bubble - WR-02: add HX-Retarget/HX-Reswap headers on 422 and 500 error responses so validation errors reach the composer form in the DOM - WR-03: replace hardcoded rgba(128,78,236,0.10) with color-mix() using the --color-brand-primary token - WR-04: remove hardcoded "1 participant" subtitle (no participant count in data) - IN-02: DiscussionMessageFromRow now accepts currentUserID uuid.UUID (matching DiscussionMessagesFromRows) instead of a pre-computed bool Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
bb2001f3ce
commit
30256895b2
4 changed files with 21 additions and 17 deletions
|
|
@ -92,6 +92,8 @@ func DiscussionMessageCreateHandler(deps DiscussionDeps) http.HandlerFunc {
|
||||||
}
|
}
|
||||||
if errs.Body != "" {
|
if errs.Body != "" {
|
||||||
w.Header().Set("Content-Type", "text/html; charset=utf-8")
|
w.Header().Set("Content-Type", "text/html; charset=utf-8")
|
||||||
|
w.Header().Set("HX-Retarget", "form[action$='/discussion/messages']")
|
||||||
|
w.Header().Set("HX-Reswap", "outerHTML")
|
||||||
w.WriteHeader(http.StatusUnprocessableEntity)
|
w.WriteHeader(http.StatusUnprocessableEntity)
|
||||||
_ = templates.DiscussionComposer(tablo, form, errs, csrf.Token(r)).Render(r.Context(), w)
|
_ = templates.DiscussionComposer(tablo, form, errs, csrf.Token(r)).Render(r.Context(), w)
|
||||||
return
|
return
|
||||||
|
|
@ -104,6 +106,8 @@ func DiscussionMessageCreateHandler(deps DiscussionDeps) http.HandlerFunc {
|
||||||
if err != nil {
|
if err != nil {
|
||||||
slog.Default().Error("discussion create: CreateDiscussionMessage failed", "tablo_id", tablo.ID, "err", err)
|
slog.Default().Error("discussion create: CreateDiscussionMessage failed", "tablo_id", tablo.ID, "err", err)
|
||||||
w.Header().Set("Content-Type", "text/html; charset=utf-8")
|
w.Header().Set("Content-Type", "text/html; charset=utf-8")
|
||||||
|
w.Header().Set("HX-Retarget", "form[action$='/discussion/messages']")
|
||||||
|
w.Header().Set("HX-Reswap", "outerHTML")
|
||||||
w.WriteHeader(http.StatusInternalServerError)
|
w.WriteHeader(http.StatusInternalServerError)
|
||||||
errs.General = "Message could not be sent. Please try again."
|
errs.General = "Message could not be sent. Please try again."
|
||||||
_ = templates.DiscussionComposer(tablo, form, errs, csrf.Token(r)).Render(r.Context(), w)
|
_ = templates.DiscussionComposer(tablo, form, errs, csrf.Token(r)).Render(r.Context(), w)
|
||||||
|
|
@ -118,11 +122,14 @@ func DiscussionMessageCreateHandler(deps DiscussionDeps) http.HandlerFunc {
|
||||||
http.Error(w, "internal server error", http.StatusInternalServerError)
|
http.Error(w, "internal server error", http.StatusInternalServerError)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
message := templates.DiscussionMessageFromRow(row, row.AuthorUserID == user.ID)
|
message := templates.DiscussionMessageFromRow(row, user.ID)
|
||||||
data := templates.DiscussionTabData{Messages: []templates.DiscussionMessageView{message}}
|
data := templates.DiscussionTabData{Messages: []templates.DiscussionMessageView{message}}
|
||||||
markDiscussionRead(r, deps.Queries, tablo, user.ID, data)
|
markDiscussionRead(r, deps.Queries, tablo, user.ID, data)
|
||||||
if deps.Realtime != nil {
|
if deps.Realtime != nil {
|
||||||
html, err := renderDiscussionMessageHTML(r, message)
|
// SSE recipients are never the author — always render as IsOwn: false.
|
||||||
|
sseMessage := message
|
||||||
|
sseMessage.IsOwn = false
|
||||||
|
html, err := renderDiscussionMessageHTML(r, sseMessage)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
slog.Default().Warn("discussion create: render realtime message failed", "tablo_id", tablo.ID, "message_id", msg.ID, "err", err)
|
slog.Default().Warn("discussion create: render realtime message failed", "tablo_id", tablo.ID, "message_id", msg.ID, "err", err)
|
||||||
} else {
|
} else {
|
||||||
|
|
|
||||||
|
|
@ -749,7 +749,7 @@
|
||||||
}
|
}
|
||||||
|
|
||||||
.message-row.message-own .message-bubble {
|
.message-row.message-own .message-bubble {
|
||||||
background-color: rgba(128, 78, 236, 0.10);
|
background-color: color-mix(in srgb, var(--color-brand-primary) 10%, transparent);
|
||||||
border-radius: 0.75rem 0.75rem 0.25rem 0.75rem;
|
border-radius: 0.75rem 0.75rem 0.25rem 0.75rem;
|
||||||
color: var(--color-text-primary);
|
color: var(--color-text-primary);
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -12,22 +12,20 @@ templ DiscussionTabFragment(tablo sqlc.Tablo, data DiscussionTabData, form Discu
|
||||||
<div class="flex flex-wrap items-start justify-between gap-3">
|
<div class="flex flex-wrap items-start justify-between gap-3">
|
||||||
<div>
|
<div>
|
||||||
<h2 class="text-2xl font-semibold leading-tight text-slate-900">Discussion</h2>
|
<h2 class="text-2xl font-semibold leading-tight text-slate-900">Discussion</h2>
|
||||||
<p class="mt-1 text-sm text-slate-600">1 participant</p>
|
|
||||||
</div>
|
</div>
|
||||||
</div>
|
</div>
|
||||||
<div id="discussion-messages" class="ui-card">
|
<div id="discussion-messages" class="ui-card">
|
||||||
if len(data.Messages) == 0 {
|
if len(data.Messages) == 0 {
|
||||||
@DiscussionEmptyState()
|
@DiscussionEmptyState()
|
||||||
} else {
|
|
||||||
<div class="divide-y divide-slate-100">
|
|
||||||
for i, message := range data.Messages {
|
|
||||||
if DiscussionShowDaySeparator(data.Messages, i) {
|
|
||||||
@DiscussionDaySeparator(message.CreatedAt)
|
|
||||||
}
|
|
||||||
@DiscussionMessageRow(message)
|
|
||||||
}
|
|
||||||
</div>
|
|
||||||
}
|
}
|
||||||
|
<div id="discussion-message-list" class="divide-y divide-slate-100">
|
||||||
|
for i, message := range data.Messages {
|
||||||
|
if DiscussionShowDaySeparator(data.Messages, i) {
|
||||||
|
@DiscussionDaySeparator(message.CreatedAt)
|
||||||
|
}
|
||||||
|
@DiscussionMessageRow(message)
|
||||||
|
}
|
||||||
|
</div>
|
||||||
</div>
|
</div>
|
||||||
@DiscussionComposer(tablo, form, errs, csrfToken)
|
@DiscussionComposer(tablo, form, errs, csrfToken)
|
||||||
</div>
|
</div>
|
||||||
|
|
@ -71,9 +69,8 @@ templ DiscussionComposer(tablo sqlc.Tablo, form DiscussionForm, errs DiscussionE
|
||||||
method="POST"
|
method="POST"
|
||||||
action={ templ.SafeURL(DiscussionPostURL(tablo.ID)) }
|
action={ templ.SafeURL(DiscussionPostURL(tablo.ID)) }
|
||||||
hx-post={ DiscussionPostURL(tablo.ID) }
|
hx-post={ DiscussionPostURL(tablo.ID) }
|
||||||
hx-target="#discussion-messages"
|
hx-target="#discussion-message-list"
|
||||||
hx-swap="beforeend"
|
hx-swap="beforeend"
|
||||||
hx-on::after-request="if (event.detail.xhr.status >= 200 && event.detail.xhr.status < 300) this.reset()"
|
|
||||||
class="border-t border-slate-200 pt-4"
|
class="border-t border-slate-200 pt-4"
|
||||||
>
|
>
|
||||||
@ui.CSRFField(csrfToken)
|
@ui.CSRFField(csrfToken)
|
||||||
|
|
|
||||||
|
|
@ -67,13 +67,13 @@ func DiscussionMessagesFromRows(rows []sqlc.ListDiscussionMessagesByTabloRow, cu
|
||||||
return messages
|
return messages
|
||||||
}
|
}
|
||||||
|
|
||||||
func DiscussionMessageFromRow(row sqlc.GetDiscussionMessageWithAuthorRow, isOwn bool) DiscussionMessageView {
|
func DiscussionMessageFromRow(row sqlc.GetDiscussionMessageWithAuthorRow, currentUserID uuid.UUID) DiscussionMessageView {
|
||||||
return DiscussionMessageView{
|
return DiscussionMessageView{
|
||||||
ID: row.ID,
|
ID: row.ID,
|
||||||
AuthorEmail: row.AuthorEmail,
|
AuthorEmail: row.AuthorEmail,
|
||||||
Body: row.Body,
|
Body: row.Body,
|
||||||
CreatedAt: row.CreatedAt.Time,
|
CreatedAt: row.CreatedAt.Time,
|
||||||
IsOwn: isOwn,
|
IsOwn: row.AuthorUserID == currentUserID,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue