From e79d920aeb00204232669b09dcd06a09f0a4605a Mon Sep 17 00:00:00 2001 From: Rotimi Best Date: Sun, 2 Aug 2026 23:23:18 +0100 Subject: [PATCH] fix(newsfeed): tighten card spacing and reposition pinned icon (#818) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(newsfeed): tighten card spacing and reposition pinned icon - Move pinned icon to top-left corner with a tilted rotation - Remove the extra pin row that added excess top padding - Reduce spacing between post content, reactions, and comments - Remove card box shadow Co-authored-by: Rotimi Best * fix(newsfeed): place pinned icon on card corner instead of avatar Co-authored-by: Rotimi Best * fix(newsfeed): stop dropdown menus locking body scroll bits-ui defaults DropdownMenu.Content's preventScroll to true, so every open ran a body scroll lock: a forced full-document style recalc and reflow, a horizontal shift by the scrollbar width, and pointer-events: none on the body. On the newsfeed page — a long list of cards each with a scrollable comment thread — that made the comment ellipsis menu feel sluggish to open. Menus don't need scroll locking; only modal layers do. Also tighten card spacing, use the primary color for the pinned icon, give the card menu trigger a pointer cursor, and switch the comment save and submit buttons to the loading and secondary variants. * refactor(newsfeed): store reply targets as data, not markup Replies used to prepend a presentational
— Tailwind classes, hardcoded English and a copied snippet of the parent — onto the comment body, and that whole string was persisted to course_newsfeed_comment.content. Editing a reply therefore showed raw markup in the textarea, the quoted snippet froze at write time, the label could never be localized, and two of its ui: utilities never compiled because they appeared only in dashboard code. Add a replyToCommentId column, distinct from parentId (the thread root), so a reply can name the comment it answers. The service verifies the target belongs to the same feed and the same thread; the paginated query joins it to return replyToAuthorFullname, so the label is rendered from live data by the new CommentTree.ReplyingTo component using the existing replying_to key. Rows written the old way still display their stored quote, and stripLegacyReplyQuote keeps that markup out of the edit textarea. * refactor(newsfeed): drop redundant comments on reply target plumbing * fix(newsfeed): address review feedback on pinned icon and quote stripping - The pin icon carried an unprefixed `text-muted-foreground` plus `ui:stroke-primary`, which is emitted nowhere because it appears only in dashboard code, so the outline fell back to the muted colour. Use `ui:text-primary` and let the stroke inherit currentColor. - Give the pinned state an accessible name: the icon is `aria-hidden`, and header.svelte no longer carries an indicator, so screen readers had nothing. - Filter feeds on `isPinned` alone. `is_pinned` is only the database column name; drizzle maps it to `isPinned` and the API never returns the snake_case form, so the `as any` fallback was dead and let the page and card disagree. - `ui:transition-color` is not a Tailwind utility and emitted nothing; the plural `ui:transition-colors` is what applies the transition. - Strip a legacy reply quote only when the blockquote carries the `reply-quote` class the old code always wrote, so a blockquote the author typed is kept. * docs(agents): forbid presentational markup in persisted content columns --------- --- AGENTS.md | 21 ++++++++++ apps/api/src/routes/course/newsfeed.ts | 4 +- apps/api/src/services/newsfeed/newsfeed.ts | 26 ++++++++++++- .../features/course/api/newsfeed.svelte.ts | 11 +++++- .../components/newsfeed/card/card.svelte | 20 ++++++++-- .../components/newsfeed/card/comments.svelte | 38 +++++++++++-------- .../components/newsfeed/card/header.svelte | 12 ++---- .../lib/features/course/pages/newsfeed.svelte | 17 ++++++--- .../course/utils/newsfeed-comment-utils.ts | 20 ++++++++++ packages/db/src/queries/newsfeed/newsfeed.ts | 14 ++++++- packages/db/src/schema.ts | 8 +++- .../dropdown-menu-content.svelte | 5 +++ .../comment-tree/comment-tree-input.svelte | 4 +- .../comment-tree-replying-to.svelte | 15 ++++++++ packages/ui/src/custom/comment-tree/index.ts | 5 ++- .../utils/src/validation/newsfeed/newsfeed.ts | 3 +- 16 files changed, 176 insertions(+), 47 deletions(-) create mode 100644 apps/dashboard/src/lib/features/course/utils/newsfeed-comment-utils.ts create mode 100644 packages/ui/src/custom/comment-tree/comment-tree-replying-to.svelte diff --git a/AGENTS.md b/AGENTS.md index eb8301b3..bfd2e7dd 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -58,6 +58,27 @@ Also run `pnpm format:check` (see Translation, Formatting, and Git Workflow abov org.limits = { students: toResourceUsage(studentsUsed, studentsLimit) }; ``` +## Persisted columns store data, not presentation + +A user-content column (`content`, `body`, `description`, …) stores what the author typed and nothing else. Check this **at the write site** — the component or service building the value — not at the schema. + +Never write into such a column: + +- **Presentational markup** — wrapper elements, Tailwind classes, inline styles. Render that in a component instead. +- **Localized strings** — copy baked in at write time can never be re-translated, and it freezes the author's locale for every future reader. +- **Copied values from another row** — a quoted snippet, an author name, a title. Store the relationship (a real FK column) and join or derive the display value at render time, so it stays correct when the source row is edited or deleted. + +```ts +// ❌ don't — markup, English copy and a copy of the parent all land in the column +const quote = `
Replying to @${target.fullname}"${target.snippet}"
`; +await createComment(`${quote}${text}`, parentId); + +// ✅ do — persist the relationship, render the label from live data +await createComment(text, parentId, target.commentId); +``` + +The tell that this went wrong: an edit form shows raw markup, because the markup *is* the content. + ## Creating a New Route ### Step 1: Validation Schema diff --git a/apps/api/src/routes/course/newsfeed.ts b/apps/api/src/routes/course/newsfeed.ts index 03079f37..a782ce37 100644 --- a/apps/api/src/routes/course/newsfeed.ts +++ b/apps/api/src/routes/course/newsfeed.ts @@ -152,14 +152,14 @@ export const newsfeedRouter = new Hono() const user = c.get('user')!; const courseId = c.req.param('courseId')!; const { feedId } = c.req.valid('param'); - const { content, parentId } = c.req.valid('json'); + const { content, parentId, replyToCommentId } = c.req.valid('json'); const groupMemberId = await getGroupMemberIdByCourseAndProfile(courseId, user.id); if (!groupMemberId) { return c.json({ success: false, error: 'User is not a member of this course' }, 403); } - const comment = await createNewsfeedCommentService(feedId, groupMemberId, content, parentId); + const comment = await createNewsfeedCommentService(feedId, groupMemberId, content, parentId, replyToCommentId); return c.json({ success: true, data: comment }, 201); } catch (error) { diff --git a/apps/api/src/services/newsfeed/newsfeed.ts b/apps/api/src/services/newsfeed/newsfeed.ts index f3a553e6..0c17d58a 100644 --- a/apps/api/src/services/newsfeed/newsfeed.ts +++ b/apps/api/src/services/newsfeed/newsfeed.ts @@ -265,13 +265,15 @@ export async function getNewsfeedCommentsService( * @param authorId Group member ID (author) * @param content Comment content * @param parentId Optional parent comment ID for replies + * @param replyToCommentId Optional comment being answered, within the same thread * @returns Created comment */ export async function createNewsfeedCommentService( feedId: string, authorId: string, content: string, - parentId?: number + parentId?: number, + replyToCommentId?: number ) { try { // Validate parentId when provided @@ -295,13 +297,33 @@ export async function createNewsfeedCommentService( } } + if (replyToCommentId !== undefined) { + const targetComment = await getNewsfeedCommentById(replyToCommentId); + + if (!targetComment) { + throw new AppError('Comment being replied to not found', ErrorCodes.COMMENT_NOT_FOUND, 404); + } + + if (targetComment.courseNewsfeedId !== feedId) { + throw new AppError('Comment being replied to does not belong to this feed', ErrorCodes.VALIDATION_ERROR, 400); + } + + const isThreadRoot = targetComment.id === parentId; + const isSiblingReply = targetComment.parentId === parentId; + + if (!isThreadRoot && !isSiblingReply) { + throw new AppError('Comment being replied to is not part of this thread', ErrorCodes.VALIDATION_ERROR, 400); + } + } + const sanitizedContent = sanitizeHtml(content); const commentData: TNewCourseNewsfeedComment = { courseNewsfeedId: feedId, authorId, content: sanitizedContent, - parentId: parentId ?? null + parentId: parentId ?? null, + replyToCommentId: replyToCommentId ?? null }; const comment = await createNewsfeedComment(commentData); diff --git a/apps/dashboard/src/lib/features/course/api/newsfeed.svelte.ts b/apps/dashboard/src/lib/features/course/api/newsfeed.svelte.ts index e446288f..71bf8d42 100644 --- a/apps/dashboard/src/lib/features/course/api/newsfeed.svelte.ts +++ b/apps/dashboard/src/lib/features/course/api/newsfeed.svelte.ts @@ -489,9 +489,15 @@ export class NewsfeedApi extends BaseApiWithErrors { feedId: string, content: string, author: { id: string; username: string; fullname: string; avatarUrl: string }, - parentId?: number + parentId?: number, + replyTo?: { commentId: number; authorFullname: string } ) { - const result = ZNewsfeedCommentCreate.safeParse({ courseNewsfeedId: feedId, content, parentId }); + const result = ZNewsfeedCommentCreate.safeParse({ + courseNewsfeedId: feedId, + content, + parentId, + replyToCommentId: replyTo?.commentId + }); if (!result.success) { this.errors = mapZodErrorsToTranslations(result.error, 'newsfeed'); return; @@ -514,6 +520,7 @@ export class NewsfeedApi extends BaseApiWithErrors { authorFullname: author.fullname, authorUsername: author.username, authorAvatarUrl: author.avatarUrl, + replyToAuthorFullname: replyTo?.authorFullname ?? null, replyCount: 0 }; diff --git a/apps/dashboard/src/lib/features/course/components/newsfeed/card/card.svelte b/apps/dashboard/src/lib/features/course/components/newsfeed/card/card.svelte index 7487d662..def2630b 100644 --- a/apps/dashboard/src/lib/features/course/components/newsfeed/card/card.svelte +++ b/apps/dashboard/src/lib/features/course/components/newsfeed/card/card.svelte @@ -2,9 +2,12 @@ import { onMount, tick } from 'svelte'; import type { Feed } from '$features/course/utils/types'; + import { t } from '$lib/utils/functions/translations'; import { newsfeedApi } from '$features/course/api'; import { getNewsfeedReactionCounts, getSelectedNewsfeedReactionType } from '@cio/ui/custom/newsfeed-reactions'; + import PinIcon from '@lucide/svelte/icons/pin'; + import DeleteConfirmation from './delete-confirmation.svelte'; import type { NewsfeedCommentsByFeedId } from '$features/course/api'; import Header from './header.svelte'; @@ -80,13 +83,22 @@
+ {#if feed.isPinned} +
{:else} + {#if reply.replyToAuthorFullname} + + {/if} -
- {#if feed.isPinned} -
- -
- {/if} - -
+
+
@@ -43,7 +37,7 @@ diff --git a/apps/dashboard/src/lib/features/course/pages/newsfeed.svelte b/apps/dashboard/src/lib/features/course/pages/newsfeed.svelte index 79443ce3..ec5037db 100644 --- a/apps/dashboard/src/lib/features/course/pages/newsfeed.svelte +++ b/apps/dashboard/src/lib/features/course/pages/newsfeed.svelte @@ -69,8 +69,13 @@ } }; - const addNewComment = async (comment: string, feedId: string, parentId?: number) => { - await newsfeedApi.createComment(courseId, feedId, comment, author, parentId); + const addNewComment = async ( + comment: string, + feedId: string, + parentId?: number, + replyTo?: { commentId: number; authorFullname: string } + ) => { + await newsfeedApi.createComment(courseId, feedId, comment, author, parentId, replyTo); if (!newsfeedApi.success) { return snackbar.error('snackbar.course.error.commenting_error'); @@ -97,11 +102,11 @@ }; const pinnedFeeds = $derived.by(() => { - return newsfeedApi.feeds.filter((feed) => Boolean((feed as any).isPinned || (feed as any).is_pinned)); + return newsfeedApi.feeds.filter((feed) => feed.isPinned); }); const unpinnedFeeds = $derived.by(() => { - return newsfeedApi.feeds.filter((feed) => !((feed as any).isPinned || (feed as any).is_pinned)); + return newsfeedApi.feeds.filter((feed) => !feed.isPinned); }); function getPageRoles(org: AccountOrg) { @@ -147,7 +152,7 @@ /> {:else} {#if pinnedFeeds.length > 0} -
+
{$t('course.navItem.news_feed.pinned') || 'Pinned'} @@ -172,7 +177,7 @@ {/if} {#if pinnedFeeds.length > 0 && unpinnedFeeds.length > 0} -
+
{$t('course.navItem.news_feed.other_posts')} diff --git a/apps/dashboard/src/lib/features/course/utils/newsfeed-comment-utils.ts b/apps/dashboard/src/lib/features/course/utils/newsfeed-comment-utils.ts new file mode 100644 index 00000000..eb6e112d --- /dev/null +++ b/apps/dashboard/src/lib/features/course/utils/newsfeed-comment-utils.ts @@ -0,0 +1,20 @@ +// Replies used to persist their "Replying to @someone" chrome as a leading +//
inside the comment body. New replies carry `replyToCommentId` +// instead, so these helpers exist only to read rows written the old way. +// Matches on the `reply-quote` class the old code always emitted, so a +// blockquote the author typed themselves is left alone. +const LEADING_REPLY_QUOTE = + /^\s*]*class=(?:"[^"]*\breply-quote\b[^"]*"|'[^']*\breply-quote\b[^']*')[\s\S]*?<\/blockquote>/i; + +const SNIPPET_MAX_LENGTH = 60; + +export function stripLegacyReplyQuote(content: string) { + return content.replace(LEADING_REPLY_QUOTE, '').trim(); +} + +export function getReplySnippet(content: string) { + const body = stripLegacyReplyQuote(content); + if (body.length <= SNIPPET_MAX_LENGTH) return body; + + return `${body.slice(0, SNIPPET_MAX_LENGTH)}...`; +} diff --git a/packages/db/src/queries/newsfeed/newsfeed.ts b/packages/db/src/queries/newsfeed/newsfeed.ts index 813b5f30..da8f927d 100644 --- a/packages/db/src/queries/newsfeed/newsfeed.ts +++ b/packages/db/src/queries/newsfeed/newsfeed.ts @@ -2,6 +2,7 @@ import * as schema from '@db/schema'; import type { TCourseNewsfeed, TCourseNewsfeedComment, TNewCourseNewsfeed, TNewCourseNewsfeedComment } from '@db/types'; import { and, db, desc, eq, isNull, lt, ne, sql } from '@db/drizzle'; +import { alias } from 'drizzle-orm/pg-core'; import { ROLE } from '@cio/utils/constants'; /** @@ -251,16 +252,21 @@ export async function getNewsfeedCommentsByFeedIdPaginated( .where(and(...baseWhereConditions)); const totalCount = Number(totalCountResult[0]?.count || 0); + const replyTarget = alias(schema.courseNewsfeedComment, 'reply_target'); + const replyTargetMember = alias(schema.groupmember, 'reply_target_member'); + const replyTargetProfile = alias(schema.profile, 'reply_target_profile'); + // Fetch comments with author profile & reply count const comments = await db .select({ comment: schema.courseNewsfeedComment, groupmember: schema.groupmember, profile: schema.profile, + replyToAuthorFullname: replyTargetProfile.fullname, replyCount: sql` COALESCE( - (SELECT COUNT(*)::int - FROM ${schema.courseNewsfeedComment} c2 + (SELECT COUNT(*)::int + FROM ${schema.courseNewsfeedComment} c2 WHERE c2.parent_id = ${schema.courseNewsfeedComment.id}), 0 ) @@ -269,6 +275,9 @@ export async function getNewsfeedCommentsByFeedIdPaginated( .from(schema.courseNewsfeedComment) .leftJoin(schema.groupmember, eq(schema.courseNewsfeedComment.authorId, schema.groupmember.id)) .leftJoin(schema.profile, eq(schema.groupmember.profileId, schema.profile.id)) + .leftJoin(replyTarget, eq(schema.courseNewsfeedComment.replyToCommentId, replyTarget.id)) + .leftJoin(replyTargetMember, eq(replyTarget.authorId, replyTargetMember.id)) + .leftJoin(replyTargetProfile, eq(replyTargetMember.profileId, replyTargetProfile.id)) .where(and(...whereConditions)) .orderBy(desc(schema.courseNewsfeedComment.createdAt)) .limit(limit + 1); @@ -285,6 +294,7 @@ export async function getNewsfeedCommentsByFeedIdPaginated( authorFullname: row.profile?.fullname || null, authorUsername: row.profile?.username || null, authorAvatarUrl: row.profile?.avatarUrl || null, + replyToAuthorFullname: row.replyToAuthorFullname || null, replyCount: Number(row.replyCount || 0) })), totalCount, diff --git a/packages/db/src/schema.ts b/packages/db/src/schema.ts index 447cecdf..d25f6216 100644 --- a/packages/db/src/schema.ts +++ b/packages/db/src/schema.ts @@ -1693,7 +1693,8 @@ export const courseNewsfeedComment = pgTable( cache: 1 }), courseNewsfeedId: uuid('course_newsfeed_id'), - parentId: bigint('parent_id', { mode: 'number' }) + parentId: bigint('parent_id', { mode: 'number' }), + replyToCommentId: bigint('reply_to_comment_id', { mode: 'number' }) }, (table) => [ foreignKey({ @@ -1711,6 +1712,11 @@ export const courseNewsfeedComment = pgTable( foreignColumns: [table.id], name: 'course_newsfeed_comment_parent_id_fkey' }).onDelete('cascade'), + foreignKey({ + columns: [table.replyToCommentId], + foreignColumns: [table.id], + name: 'course_newsfeed_comment_reply_to_comment_id_fkey' + }).onDelete('set null'), index('course_newsfeed_comment_parent_id_idx').on(table.parentId) ] ); diff --git a/packages/ui/src/base/dropdown-menu/dropdown-menu-content.svelte b/packages/ui/src/base/dropdown-menu/dropdown-menu-content.svelte index 723d6614..3e86e994 100644 --- a/packages/ui/src/base/dropdown-menu/dropdown-menu-content.svelte +++ b/packages/ui/src/base/dropdown-menu/dropdown-menu-content.svelte @@ -6,6 +6,10 @@ ref = $bindable(null), sideOffset = 4, portalProps, + // bits-ui defaults this to true, which locks body scroll on every open: + // a forced full-document style recalc + reflow, plus pointer-events: none. + // A menu doesn't need it; only modal layers do. + preventScroll = false, class: className, ...restProps }: DropdownMenuPrimitive.ContentProps & { @@ -18,6 +22,7 @@ bind:ref data-slot="dropdown-menu-content" {sideOffset} + {preventScroll} class={cn( 'ui:bg-popover ui:text-popover-foreground ui:data-[state=open]:animate-in ui:data-[state=closed]:animate-out ui:data-[state=closed]:fade-out-0 ui:data-[state=open]:fade-in-0 ui:data-[state=closed]:zoom-out-95 ui:data-[state=open]:zoom-in-95 ui:data-[side=bottom]:slide-in-from-top-2 ui:data-[side=left]:slide-in-from-right-2 ui:data-[side=right]:slide-in-from-left-2 ui:data-[side=top]:slide-in-from-bottom-2 ui:max-h-(--bits-dropdown-menu-content-available-height) ui:origin-(--bits-dropdown-menu-content-transform-origin) ui:z-50 ui:min-w-32 ui:overflow-y-auto ui:overflow-x-hidden ui:rounded-md ui:border ui:p-1 ui:shadow-md ui:outline-none', className diff --git a/packages/ui/src/custom/comment-tree/comment-tree-input.svelte b/packages/ui/src/custom/comment-tree/comment-tree-input.svelte index 2ce01bc0..acec84ee 100644 --- a/packages/ui/src/custom/comment-tree/comment-tree-input.svelte +++ b/packages/ui/src/custom/comment-tree/comment-tree-input.svelte @@ -56,7 +56,7 @@
{#if replyingToUser}