-
Notifications
You must be signed in to change notification settings - Fork 2.4k
[WEB-4297] feat: changelog improvements #7242
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: preview
Are you sure you want to change the base?
Conversation
WalkthroughThis update introduces a new changelog feature, including type definitions, configuration constants, a service layer for fetching changelog data, and a React component for rendering rich text with custom inline blocks. The product updates modal is refactored to fetch and render changelog entries dynamically, and a new rich text rendering dependency is added. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ProductUpdatesModal
participant ChangelogService
participant API
participant RichTextNode
User->>ProductUpdatesModal: Opens modal
ProductUpdatesModal->>ChangelogService: fetchChangelog(slug, limit, page)
ChangelogService->>API: GET /api/{slug}-releases?limit=...&page=...
API-->>ChangelogService: Returns changelog data
ChangelogService-->>ProductUpdatesModal: ChangelogPaginationData
ProductUpdatesModal->>RichTextNode: Render description for each changelog item
RichTextNode-->>ProductUpdatesModal: Rendered rich text
ProductUpdatesModal-->>User: Displays changelog entries
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
🧹 Nitpick comments (3)
packages/types/src/changelog.d.ts (1)
4-4
: Consider using a more specific date type.The
date
field as string is acceptable, but consider using ISO 8601 format or a more specific type for better type safety and consistency.export interface ChangelogItem { id: string; title: string; - date: string; + date: string; // ISO 8601 format: YYYY-MM-DDTHH:mm:ss.sssZ summary: string; description: any; published: boolean; }web/core/components/global/product-updates/jsxConverter.tsx (2)
137-258
: Consider performance optimization for DOM manipulation.The extensive DOM manipulation in useEffect could impact performance, especially with multiple changelog entries. Consider moving some styling to CSS classes or using CSS-in-JS solutions.
Here's an example of how to move some styles to CSS classes:
/* Add to your CSS */ .changelog-content h1 { margin-top: 35px; margin-bottom: 15px; } .changelog-content h2 { margin-top: 35px; margin-bottom: 20px; } .changelog-content h3 { margin-top: 20px; margin-bottom: 18px; } .changelog-content ul { list-style-type: disc; list-style-position: outside; padding-left: 20px; margin: 10px 0; } .changelog-content ol { list-style-type: decimal; list-style-position: outside; padding-left: 20px; margin: 10px 0; }Then simplify the useEffect:
return ( <div ref={containerRef} - // style={{letterSpacing: "-0.020em" }} + className="changelog-content" >
76-87
: Extract color configuration to constants for maintainability.The hardcoded color classes and styles should be extracted to constants for better maintainability and consistency.
+ const COLOR_THEMES = { + Red: { + container: "border border-[#DD4167] dark:border-[#4C182C] dark:bg-[#4C182C]/40 bg-[#DD4167]/40", + icon: "text-[#DD4167]" + }, + Yellow: { + container: "border border-[#D4A72C66] dark:border-[#BF8700] bg-[#FFF8C5] dark:bg-[#332E1B]", + icon: "text-[#9A6700]" + }, + Blue: { + container: "border border-[#3f76ff] dark:border-[#224f6a] bg-[#d9efff] dark:bg-[#1e2934]", + icon: "text-[#3f76ff] dark:text-[#4d9ed0]" + }, + Green: { + container: "border border-[#5CD3B5] dark:border-[#235645] bg-[#D3F9E7] dark:bg-[#1E2B2A]", + icon: "text-[#208779] dark:text-[#A8F3D0]" + } + } as const; - const colorClasses = { - Red: "border border-[#DD4167] dark:border-[#4C182C] dark:bg-[#4C182C]/40 bg-[#DD4167]/40", - Yellow: "border border-[#D4A72C66] dark:border-[#BF8700] bg-[#FFF8C5] dark:bg-[#332E1B]", - Blue: "border border-[#3f76ff] dark:border-[#224f6a] bg-[#d9efff] dark:bg-[#1e2934]", - Green: "border border-[#5CD3B5] dark:border-[#235645] bg-[#D3F9E7] dark:bg-[#1E2B2A]", - }; - const iconColorClasses = { - Red: "text-[#DD4167]", - Yellow: "text-[#9A6700]", - Blue: "text-[#3f76ff] dark:text-[#4d9ed0]", - Green: "text-[#208779] dark:text-[#A8F3D0]", - }; + const theme = COLOR_THEMES[fields.color] ?? COLOR_THEMES.Yellow;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (8)
packages/constants/src/changelog.ts
(1 hunks)packages/constants/src/index.ts
(1 hunks)packages/types/src/changelog.d.ts
(1 hunks)packages/types/src/index.d.ts
(1 hunks)web/core/components/global/product-updates/jsxConverter.tsx
(1 hunks)web/core/components/global/product-updates/modal.tsx
(1 hunks)web/core/services/changelog.service.ts
(1 hunks)web/package.json
(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
web/core/components/global/product-updates/jsxConverter.tsx
[error] 52-52: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 151-151: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (python)
🔇 Additional comments (5)
web/package.json (1)
24-24
: LGTM: Appropriate dependency for rich text rendering.The
@payloadcms/richtext-lexical
package addition aligns well with the changelog improvements objective and supports the rich text rendering functionality.packages/constants/src/changelog.ts (1)
1-9
: LGTM: Well-structured configuration constants.The environment variable handling with fallback is appropriate. The hardcoded values in
ChangelogConfig
appear to be sensible defaults for pagination and the specific cloud slug.packages/constants/src/index.ts (1)
38-38
: LGTM: Standard module re-export.The export follows the established pattern and makes the changelog constants available through the package's public API.
packages/types/src/index.d.ts (1)
48-48
: LGTM: Standard type module re-export.The export follows the established pattern and makes the changelog types available through the package's public API.
web/core/components/global/product-updates/modal.tsx (1)
47-48
: LGTM: Proper filtering of published content.Good security practice to filter for published content only, preventing unpublished/draft content from being displayed to end users.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
🧹 Nitpick comments (6)
packages/constants/src/changelog.ts (1)
5-9
: Consider making pagination configuration more flexible.The hardcoded pagination values (
limit: 5, page: 1
) might be too restrictive. Consider making these configurable through environment variables or allowing them to be overridden at runtime.export const ChangelogConfig = { slug: "cloud", - limit: 5, - page: 1, + limit: parseInt(process.env.NEXT_PUBLIC_CHANGELOG_LIMIT || "5", 10), + page: parseInt(process.env.NEXT_PUBLIC_CHANGELOG_PAGE || "1", 10), };web/core/components/global/product-updates/jsxConverter.tsx (5)
52-52
: Use optional chaining for better readability.The static analysis tool correctly identifies that this can be simplified using optional chaining.
- if (node.value && node.value.url) { + if (node.value?.url) {
151-151
: Use optional chaining for better readability.The static analysis tool correctly identifies that this can be simplified using optional chaining.
- if (node.nodeValue && node.nodeValue.includes("\u00A0")) { + if (node.nodeValue?.includes("\u00A0")) {
137-258
: Consider performance optimization for DOM manipulation.The
useEffect
performs extensive DOM manipulation on every render, which could impact performance. Consider:
- Memoizing the DOM operations
- Moving styling to CSS classes instead of inline styles
- Using CSS-in-JS or styled-components for better maintainability
This approach would improve both performance and maintainability. The current implementation directly manipulates the DOM after React renders, which can cause layout thrashing and is harder to maintain.
172-186
: Extract heading styles to CSS classes.The inline styling for headings should be moved to CSS classes for better maintainability and performance.
switch (htmlHeading.tagName) { case "H1": - htmlHeading.style.marginTop = "35px"; - htmlHeading.style.marginBottom = "15px"; + htmlHeading.classList.add("mt-[35px]", "mb-[15px]"); break; case "H2": - htmlHeading.style.marginTop = "35px"; - htmlHeading.style.marginBottom = "20px"; + htmlHeading.classList.add("mt-[35px]", "mb-[20px]"); break; case "H3": - htmlHeading.style.marginTop = "20px"; - htmlHeading.style.marginBottom = "18px"; + htmlHeading.classList.add("mt-[20px]", "mb-[18px]"); break; }
131-131
: Remove commented code or implement missing functionality.The commented line suggests incomplete implementation. Either implement the video converter or remove the commented code.
...CalloutJSXConverter, ...inLineBlockConverter, - // ...videoJSXConverter,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (8)
packages/constants/src/changelog.ts
(1 hunks)packages/constants/src/index.ts
(1 hunks)packages/types/src/changelog.d.ts
(1 hunks)packages/types/src/index.d.ts
(1 hunks)web/core/components/global/product-updates/jsxConverter.tsx
(1 hunks)web/core/components/global/product-updates/modal.tsx
(1 hunks)web/core/services/changelog.service.ts
(1 hunks)web/package.json
(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
web/core/components/global/product-updates/jsxConverter.tsx
[error] 52-52: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 151-151: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (5)
web/package.json (1)
24-24
: Verify the new dependency version and security status.The addition of
@payloadcms/richtext-lexical
supports the new rich text rendering functionality. Please ensure this version is current and free from known vulnerabilities.#!/bin/bash # Description: Check the latest version and security advisories for @payloadcms/richtext-lexical # Check latest version on npm curl -s https://registry.npmjs.org/@payloadcms/richtext-lexical | jq '.["dist-tags"].latest' # Check for security advisories via GitHub API gh api graphql -f query=' { securityVulnerabilities(first: 5, ecosystem: NPM, package: "@payloadcms/richtext-lexical") { nodes { advisory { summary severity publishedAt } vulnerableVersionRange firstPatchedVersion { identifier } } } }'packages/constants/src/changelog.ts (1)
2-2
: Consider security implications of the hardcoded fallback URL.The fallback URL
"https://preview.payload.plane.town"
appears to be a preview/development endpoint. Ensure this is appropriate for production environments and consider if it should be configurable or fail gracefully when the environment variable is not set.Do you want me to generate a more robust configuration that validates the environment variable or fails gracefully in production?
packages/constants/src/index.ts (1)
38-38
: LGTM!The export addition follows the established pattern and properly exposes the changelog constants for use throughout the codebase.
packages/types/src/index.d.ts (1)
48-48
: LGTM!The export addition follows the established pattern and properly exposes the changelog types for use throughout the codebase.
web/core/components/global/product-updates/modal.tsx (1)
58-58
: ```shell
#!/bin/bashExtract the remainder of RichTextNode implementation to inspect rendering
sed -n '200,300p' web/core/components/global/product-updates/jsxConverter.tsx
</details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/constants/src/changelog.ts (1)
5-9
: Consider adding TypeScript interface for better type safety.The configuration object would benefit from explicit typing to improve maintainability and catch potential issues at compile time.
+interface IChangelogConfig { + slug: string; + limit: number; + page: number; +} + -export const ChangelogConfig = { +export const ChangelogConfig: IChangelogConfig = { slug: "community", limit: 5, page: 1, };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/constants/src/changelog.ts
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (python)
🔇 Additional comments (1)
packages/constants/src/changelog.ts (1)
2-2
: Verify the hardcoded fallback URL for production environments.The hardcoded fallback URL points to a preview domain. Ensure this is appropriate for all deployment environments, especially production.
Consider adding environment-specific validation or using a more explicit configuration approach:
-export const PAYLOAD_BASE_URL = process.env.NEXT_PUBLIC_PAYLOAD_BASE_URL || "https://preview.payload.plane.town"; +export const PAYLOAD_BASE_URL = process.env.NEXT_PUBLIC_PAYLOAD_BASE_URL || (() => { + if (process.env.NODE_ENV === 'production') { + throw new Error('NEXT_PUBLIC_PAYLOAD_BASE_URL must be set in production'); + } + return "https://preview.payload.plane.town"; +})();
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
web/core/components/global/product-updates/root.tsx (1)
12-12
: Consider improving the empty state user experience.The empty state message could be more user-friendly and informative.
- if (!data || data?.docs?.length <= 0) return <div className="text-center container my-[30vh]">No data available</div>; + if (!data || data?.docs?.length <= 0) return <div className="text-center container my-[30vh]">No changelog entries found</div>;web/core/components/global/product-updates/content-item.tsx (1)
19-24
: Consider adding accessibility attributes for better screen reader support.The megaphone icon and header structure could benefit from proper ARIA attributes.
<div className="flex items-center gap-2 py-2 sticky top-0 z-10 bg-custom-background-100"> - <span className="size-8 rounded-full border flex items-center justify-center"> - <Megaphone className="size-6" /> + <span className="size-8 rounded-full border flex items-center justify-center" aria-hidden="true"> + <Megaphone className="size-6" /> </span> - <span className="text-neutral-text-primary text-xl font-bold">{contentItem.title}</span> + <h2 className="text-neutral-text-primary text-xl font-bold">{contentItem.title}</h2> </div>Using semantic
h2
element for the title improves document structure and accessibility.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
web/core/components/global/product-updates/content-item.tsx
(1 hunks)web/core/components/global/product-updates/error.tsx
(1 hunks)web/core/components/global/product-updates/loader.tsx
(1 hunks)web/core/components/global/product-updates/modal.tsx
(1 hunks)web/core/components/global/product-updates/root.tsx
(1 hunks)web/core/services/changelog.service.ts
(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- web/core/components/global/product-updates/loader.tsx
- web/core/components/global/product-updates/error.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- web/core/components/global/product-updates/modal.tsx
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (python)
🔇 Additional comments (3)
web/core/services/changelog.service.ts (1)
1-28
: Excellent improvements addressing previous review feedback!The service implementation has been significantly improved and addresses all the previous review concerns:
- ✅ Uses
CMS_BASE_URL
as requested- ✅ Proper error handling with throwing instead of swallowing errors
- ✅ Type consistency maintained with proper error responses
- ✅ Clean API design with config object parameter
The error handling flow is now correct - checking for missing data and rethrowing caught errors for upstream handling.
web/core/components/global/product-updates/root.tsx (1)
9-21
: Clean component implementation with good defensive programming.The component properly handles edge cases and follows React best practices with proper key usage in the map operation.
web/core/components/global/product-updates/content-item.tsx (1)
12-28
: Well-structured component with proper conditional rendering.The component follows React best practices with early returns, proper prop destructuring, and clean JSX composition. The integration with RichTextNode provides good separation of concerns.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
web/core/components/global/product-updates/loader.tsx (1)
1-17
: Well-implemented skeleton loader with room for accessibility and responsive improvements.The component is clean and follows React best practices. The layout structure with header and body sections effectively simulates changelog content loading.
Consider these enhancements:
- Accessibility: Add ARIA attributes to help screen readers understand the loading state
- Responsive design: The fixed pixel widths (500px, 300px) may not work well on mobile devices
export const ChangeLogLoader = () => ( - <Loader className="flex flex-col gap-3 h-full w-full "> + <Loader className="flex flex-col gap-3 h-full w-full" aria-label="Loading changelog content"> {/* header */} <div className="flex items-center gap-2 mb-3"> - <Loader.Item height="44px" width="500px" /> + <Loader.Item height="44px" width="min(500px, 100%)" /> </div> {/* body */} - <Loader.Item height="36px" width="300px" /> + <Loader.Item height="36px" width="min(300px, 75%)" /> <Loader.Item height="26px" width="60%" /> <Loader.Item height="26px" width="100%" /> <Loader.Item height="26px" width="75%" /> <Loader.Item height="26px" width="80%" /> </Loader> );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
web/core/components/global/product-updates/loader.tsx
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (javascript)
Description
This PR includes improvement for changelog modal.
Type of Change
References
[WEB-4297]
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Enhancements
Chores