Skip to content

feat: add user activity tab and responsive navbar#48

Merged
ujsquared merged 6 commits into
p-society:mainfrom
burgerphilic18:bug-fix-v1
Apr 14, 2026
Merged

feat: add user activity tab and responsive navbar#48
ujsquared merged 6 commits into
p-society:mainfrom
burgerphilic18:bug-fix-v1

Conversation

@burgerphilic18

Copy link
Copy Markdown
Contributor
  • Integrated the activity feed into the profile page beside user threads.
image
  • Added responsiveness to the header with a mobile drawer menu.
image
  • Fixed the profile dropdown routing links and updated button styling for consistency.
  • Fixed the profile settings cancel action to redirect properly instead of logging the user out.
  • Added Cloudflare R2 environment variable in .env.example

Signed-off-by: Spandan Hota <spandanhota2005@outlook.com>
Signed-off-by: Spandan Hota <spandanhota2005@outlook.com>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a user activity feed on the profile page, supported by a new server-side endpoint and API integration. It also significantly refactors the header to include a mobile-friendly drawer navigation and updates various UI components for a more consistent design. Key feedback includes validating the userId parameter, adjusting the activity query limits to ensure a full feed, addressing potential dark mode contrast issues with hardcoded colors, and utilizing Tailwind's responsive utilities instead of custom CSS classes.

{ preHandler: optionalAuth },
async (request, reply) => {
try {
const { userId } = request.params as { userId: string };

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The userId parameter is extracted from request.params without validation. To maintain consistency with other routes in this file (e.g., line 22) and ensure data integrity, consider using a validation schema (e.g., Zod) to parse and validate the userId format before using it in database queries.

.from(threads)
.where(eq(threads.createdBy, userId))
.orderBy(desc(threads.createdAt))
.limit(5);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The limit for individual activity types (threads, posts, likes) is set to 5. When these are merged and sliced to 15 (line 299), the resulting feed might be incomplete if a user's activity is heavily weighted toward one type. For example, a user with many posts but no threads would only see 5 items. Consider increasing the limit for each query (lines 262, 276, and 290) to 15 to ensure the feed is always populated with the most recent items across all categories.

Suggested change
.limit(5);
.limit(15);

<DropdownMenuItem asChild>
<Link
to="/my-threads"
to={user.username ? `/profile/${user.username}` : "/my/profile"}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The "My Threads" link points to the same destination as "My Profile" (line 66). This redundancy is also present in the mobile header drawer. If the intention is to show the user's threads, consider linking to the profile page with a query parameter or hash to activate the threads tab (e.g., /profile/${user.username}#threads), or remove the duplicate entry to simplify the menu.

Comment on lines +146 to +149
<div className={`p-2 border-2 border-border ${
item.type === 'thread' ? 'bg-blue-100' :
item.type === 'post' ? 'bg-green-100' : 'bg-red-100'
}`}>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Hardcoding light background colors like bg-blue-100 and bg-green-100 may lead to poor contrast and visibility issues in dark mode. Since the application supports multiple themes, it is recommended to use theme-aware utility classes or CSS variables (e.g., bg-primary/10 or bg-secondary) that adapt to the current theme context.

Comment thread apps/web/src/index.css
Comment on lines +357 to +375
.nav-desktop {
display: none;
}

@media (min-width: 768px) {
.nav-desktop {
display: flex !important;
}
}

.nav-mobile {
display: flex;
}

@media (min-width: 768px) {
.nav-mobile {
display: none !important;
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Defining custom CSS classes like .nav-desktop and .nav-mobile with !important overrides for responsive visibility is less maintainable than using Tailwind's built-in responsive utilities. It is more idiomatic to use standard Tailwind classes like hidden md:flex and flex md:hidden directly on the elements in the Header component.

@ujsquared
ujsquared merged commit d827886 into p-society:main Apr 14, 2026
0 of 2 checks passed
Signed-off-by: Spandan Hota <spandanhota2005@outlook.com>
Signed-off-by: Spandan Hota <spandanhota2005@outlook.com>
Signed-off-by: Spandan Hota <spandanhota2005@outlook.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants