Conversation
📝 WalkthroughWalkthroughAdds a new "Tags" feature: client React TagsPage with infinite loading and SCSS, a server Next.js Page that prefetches/hydrates tag data, i18n strings, a sidebar nav item, a new SDK infinite-query helper that filters community tags, a new isCommunity utility, and an SDK version bump. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Browser
participant NextServer as Next.js Server
participant QueryClient
participant HiveAPI as Hive RPC (CONFIG.hiveClient)
Note over Browser,NextServer: Server-side prefetch + hydrate
Browser->>NextServer: GET /tags
NextServer->>QueryClient: prefetchInfiniteQuery(getTrendingTagsWithStatsQueryOptions)
QueryClient->>HiveAPI: call("get_trending_tags", [afterTag="", limit])
HiveAPI-->>QueryClient: trending tags page (items)
QueryClient-->>NextServer: dehydrate(state)
NextServer-->>Browser: HTML + hydrated state
Note over Browser: Client interactions and pagination
Browser->>QueryClient: hydrate(state)
Browser->>Browser: render TagsPage (useInfiniteQuery)
Browser->>HiveAPI: call("get_trending_tags", [afterTag=lastTag, limit]) on "Load more"
HiveAPI-->>Browser: next page of tags
Browser-->>Browser: append rows, update UI
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (6)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
@packages/sdk/src/modules/posts/queries/get-trending-tags-with-stats-query-options.ts:
- Around line 17-18: Replace the infinite caching for trending tags by setting
staleTime to a finite interval (e.g., 5–10 minutes) instead of Infinity so
trending data refreshes periodically; update the object that currently sets
staleTime: Infinity (and leave refetchOnMount: true) to use a millisecond value
like 5 * 60 * 1000 (or 10 * 60 * 1000) to ensure automatic refresh while
retaining caching.
- Line 12: The current filter on tags uses a startsWith check
("!tag.name.startsWith('hive-')") which incorrectly excludes non-community tags;
replace that predicate with the shared utility check by importing isCommunity
and using .filter(tag => !isCommunity(tag.name)) so community detection matches
getTrendingTags/getAllTrendingTags; update the imports at the top of the module
to import isCommunity from the utils module where other consumers use it (same
module referenced by apps/web/src/api/hive.ts).
🧹 Nitpick comments (5)
packages/sdk/src/modules/posts/queries/get-trending-tags-with-stats-query-options.ts (1)
5-5: Consider a lower default limit.The default limit of 250 tags in the initial fetch may impact performance. Since this powers an infinite scroll UI, consider starting with a smaller page size (e.g., 50-100) to improve initial load time and user experience.
♻️ Suggested adjustment
-export function getTrendingTagsWithStatsQueryOptions(limit = 250) { +export function getTrendingTagsWithStatsQueryOptions(limit = 50) {apps/web/src/app/tags/_page.tsx (3)
64-64: Consider using a stable unique key.Using
indexin the key can cause React reconciliation issues if the tags list is reordered or items are inserted. Iftag.nameis guaranteed unique, use it alone as the key; otherwise, use a stable unique identifier from the tag object.♻️ Proposed fix
- <Tr key={`${tag.name}-${index}`}> + <Tr key={tag.name}>
73-80: Ensure consistent number formatting.The
commentsandtop_postsfields usetoLocaleString()for formatting, buttotal_payoutsdoes not. Iftotal_payoutsis a numeric value, apply the same formatting for consistency. If it's already a formatted currency string, consider adding a comment to clarify.♻️ Proposed fix if total_payouts is numeric
<Td className="border p-2"> - <span className="tag-metric">{tag.total_payouts}</span> + <span className="tag-metric">{typeof tag.total_payouts === 'number' ? tag.total_payouts.toLocaleString() : tag.total_payouts}</span> </Td>
46-85: Consider adding table accessibility attributes.For better accessibility, consider adding an
aria-labelor a<caption>element to describe the table's purpose for screen reader users.♻️ Proposed enhancement
- <Table full={true}> + <Table full={true} aria-label={i18next.t("tags-page.title")}>Or add a caption:
<Table full={true}> + <caption className="sr-only">{i18next.t("tags-page.title")}</caption> <thead>apps/web/src/app/tags/page.tsx (1)
9-11: Consider enhancing metadata for better SEO.The page metadata only includes a title. Consider adding a
descriptionand other SEO-relevant fields (e.g.,openGraph) to improve search engine visibility and social media sharing.♻️ Proposed enhancement
export const metadata: Metadata = { - title: "Tags" + title: "Trending Tags", + description: "Explore trending topics and tags on Ecency with statistics on comments, top posts, and total payouts." };
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
packages/sdk/dist/browser/index.d.tsis excluded by!**/dist/**packages/sdk/dist/browser/index.jsis excluded by!**/dist/**packages/sdk/dist/browser/index.js.mapis excluded by!**/dist/**,!**/*.mappackages/sdk/dist/node/index.cjsis excluded by!**/dist/**packages/sdk/dist/node/index.cjs.mapis excluded by!**/dist/**,!**/*.mappackages/sdk/dist/node/index.mjsis excluded by!**/dist/**packages/sdk/dist/node/index.mjs.mapis excluded by!**/dist/**,!**/*.map
📒 Files selected for processing (8)
apps/web/src/app/tags/_page.scssapps/web/src/app/tags/_page.tsxapps/web/src/app/tags/page.tsxapps/web/src/features/i18n/locales/en-US.jsonapps/web/src/features/shared/navbar/navbar-main-sidebar.tsxpackages/sdk/package.jsonpackages/sdk/src/modules/posts/queries/get-trending-tags-with-stats-query-options.tspackages/sdk/src/modules/posts/queries/index.ts
🔇 Additional comments (7)
packages/sdk/package.json (1)
4-4: LGTM! Version bump is appropriate.The patch version increment (1.4.0 → 1.4.1) correctly reflects the addition of a new exported API (
getTrendingTagsWithStatsQueryOptions) without breaking changes.apps/web/src/features/i18n/locales/en-US.json (1)
411-426: LGTM! Translation keys are well-structured.The new
tags-pagetranslation block provides comprehensive i18n support for the Tags page feature, including page metadata, column headers, error states, and load-more functionality.apps/web/src/features/shared/navbar/navbar-main-sidebar.tsx (1)
9-9: LGTM! Navigation integration is clean.The new "Trending Tags" menu item follows the existing patterns and is properly positioned in the navigation hierarchy. The i18n integration and icon selection are appropriate.
Also applies to: 119-124
packages/sdk/src/modules/posts/queries/index.ts (1)
2-2: LGTM! Export is properly structured.The new export statement correctly exposes the trending tags query options and maintains the file's export organization.
apps/web/src/app/tags/_page.scss (1)
1-115: LGTM! Well-structured styling with proper theming.The SCSS follows the project's theming conventions correctly, includes responsive design considerations, and provides a clean visual hierarchy for the tags page.
apps/web/src/app/tags/_page.tsx (2)
14-21: No action required—the function has sensible defaults.The
getTrendingTagsWithStatsQueryOptions()function has a defaultlimitparameter of 250, which is used for pagination. Calling it without arguments is correct and will use this reasonable default value.
23-23: The code is correct. Thedata.pagesstructure fromgetTrendingTagsWithStatsQueryOptions()is indeed an array of arrays (each page returnsTrendingTag[]), so.flat()properly flattens it to a single array of tags. TheuseInfiniteDataFlowutility elsewhere in the codebase uses the same pattern with.reduce()for identical logic, confirming this is the standard approach in the project.
…stats-query-options.ts Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
@packages/sdk/src/modules/posts/queries/get-trending-tags-with-stats-query-options.ts:
- Line 12: Replace the ad-hoc community filter that uses
!tag.name.startsWith("hive-") with the shared isCommunity utility: import
isCommunity and change the filter chain on tags (the expression around
tags.filter((tag) => tag.name !== "").filter((tag) =>
!tag.name.startsWith("hive-"))) to instead exclude community tags via
!isCommunity(tag.name) while preserving the non-empty name check; this ensures
tags like "hive-gaming" are not wrongly filtered out and matches other usages of
isCommunity across the codebase.
🧹 Nitpick comments (1)
packages/sdk/src/modules/posts/queries/get-trending-tags-with-stats-query-options.ts (1)
12-12: Consider combining filter calls.The two separate
.filter()calls can be combined into a single filter for slightly better performance.♻️ Combine filters
- tags.filter((tag) => tag.name !== "").filter((tag) => !tag.name.startsWith("hive-")) + tags.filter((tag) => tag.name !== "" && !tag.name.startsWith("hive-"))Note: This suggestion assumes the community filtering issue above is addressed first by using
isCommunity().
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/sdk/src/modules/posts/queries/get-trending-tags-with-stats-query-options.ts
🔇 Additional comments (3)
packages/sdk/src/modules/posts/queries/get-trending-tags-with-stats-query-options.ts (3)
1-3: Imports look good.All necessary imports are present and correctly referenced.
5-7: Function signature and query key are well-designed.The default limit of 250 is reasonable, and the query key properly includes the limit parameter for cache differentiation.
14-19: Pagination and cache configuration are correct.The pagination logic properly derives the next page parameter, and the
staleTimeof 5 minutes is a good balance for trending data - it ensures periodic refresh while benefiting from caching. This addresses the previous concern about usingInfinity.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @packages/sdk/src/modules/core/utils/is-community.ts:
- Around line 1-3: The isCommunity function's regex /^hive-\d+/ can produce
false positives (e.g., "hive-123abc"); update the pattern to enforce the end
anchor and use a boolean test: change the match call to use
/^hive-\d+$/.test(value) inside isCommunity so it only returns true for strings
that exactly match the hive-<digits> form.
🧹 Nitpick comments (1)
packages/sdk/src/modules/core/utils/is-community.ts (1)
1-3: Consider adding documentation and return type annotation.For better maintainability and developer experience, consider:
- Adding an explicit return type annotation (
: boolean)- Adding JSDoc to document the function's purpose and behavior
📝 Suggested enhancement
+/** + * Checks if a value is a Hive community identifier. + * Community identifiers follow the pattern "hive-" followed by digits. + * + * @param value - The value to check + * @returns true if the value is a community identifier, false otherwise + */ -export function isCommunity(value: unknown) { +export function isCommunity(value: unknown): boolean { return typeof value === "string" ? /^hive-\d+$/.test(value) : false; }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
packages/sdk/dist/browser/index.d.tsis excluded by!**/dist/**packages/sdk/dist/browser/index.jsis excluded by!**/dist/**packages/sdk/dist/browser/index.js.mapis excluded by!**/dist/**,!**/*.mappackages/sdk/dist/node/index.cjsis excluded by!**/dist/**packages/sdk/dist/node/index.cjs.mapis excluded by!**/dist/**,!**/*.mappackages/sdk/dist/node/index.mjsis excluded by!**/dist/**packages/sdk/dist/node/index.mjs.mapis excluded by!**/dist/**,!**/*.map
📒 Files selected for processing (3)
packages/sdk/src/modules/core/utils/index.tspackages/sdk/src/modules/core/utils/is-community.tspackages/sdk/src/modules/posts/queries/get-trending-tags-with-stats-query-options.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/sdk/src/modules/posts/queries/get-trending-tags-with-stats-query-options.ts
🧰 Additional context used
🧬 Code graph analysis (1)
packages/sdk/src/modules/core/utils/is-community.ts (1)
packages/sdk/dist/browser/index.d.ts (1)
isCommunity(2967-2967)
🔇 Additional comments (1)
packages/sdk/src/modules/core/utils/index.ts (1)
4-4: LGTM!The re-export follows the established pattern and properly exposes the new
isCommunityutility.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
1 participant
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.