Skip to content
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

feat: Ensure the article appears exactly as it will when published #1102 #1153

Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 43 additions & 62 deletions app/(app)/alpha/new/[[...postIdArr]]/_client.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import {
} from "@headlessui/react";
import { ChevronUpIcon } from "@heroicons/react/20/solid";
import Editor from "@/components/editor/editor";
import RenderPost from "@/components/editor/editor/RenderPost";
import useCreatePage from "@/hooks/useCreatePage";
import { usePrompt } from "@/components/PromptService";

Expand Down Expand Up @@ -214,69 +213,51 @@ const Create = () => {
<div className="z-60 absolute bottom-0 left-0 right-0 top-0 bg-black opacity-25" />
</div>
)}
<div className="bg-black">
<div className="mx-auto w-full max-w-7xl flex-grow text-black lg:flex xl:px-8">
{/* Left sidebar & main wrapper */}
<div className="min-w-0 flex-1 xl:flex">
<div className="lg:min-w-0 lg:flex-1">
<div className="h-full min-h-[40rem] px-4 py-0 sm:px-6 lg:px-4 lg:py-6">
{/* Start main area*/}
<div className="relative h-full">
<div className="bg-neutral-900 text-white shadow-xl">
<div className="bg-neutral-900 px-4 py-6 sm:p-6 lg:pb-8">
{!body && (
<Controller
name="body"
control={control}
render={({ field }) => (
<Editor {...field} initialValue={"{}"} />
)}
/>
)}
{body && body.length > 0 && (
<Controller
name="body"
control={control}
render={({ field }) => (
<Editor {...field} initialValue={body} />
)}
/>
)}
<div className="mx-auto break-words px-2 pb-4 sm:px-4 md:max-w-3xl">
<div className="prose mx-auto max-w-3xl dark:prose-invert lg:prose-lg">
{!body && (
<Controller
name="body"
control={control}
render={({ field }) => (
<Editor {...field} initialValue={"{}"} />
)}
/>
)}
{body && body.length > 0 && (
<Controller
name="body"
control={control}
render={({ field }) => (
<Editor {...field} initialValue={body} />
)}
/>
)}

<div className="flex items-center justify-between">
<>
{saveStatus === "loading" && <p>Auto-saving...</p>}
{saveStatus === "error" && savedTime && (
<p className="text-xs text-red-600 lg:text-sm">
{`Error saving, last saved: ${savedTime.toString()}`}
</p>
)}
{saveStatus === "success" && savedTime && (
<p className="text-xs text-neutral-400 lg:text-sm">
{`Saved: ${savedTime.toString()}`}
</p>
)}
</>
<div />

<div className="flex">
<button
type="button"
disabled={isDisabled}
className="ml-5 inline-flex justify-center bg-gradient-to-r from-orange-400 to-pink-600 px-4 py-2 text-sm font-medium text-white shadow-sm hover:from-orange-300 hover:to-pink-500 focus:outline-none focus:ring-2 focus:ring-pink-300 focus:ring-offset-2 disabled:opacity-50"
onClick={() => setOpen(true)}
>
{!data?.published && "Publish"}
{data?.published && "Save Changes"}
</button>
</div>
</div>
</div>
</div>
</div>
{/* End main area */}
</div>
<div className="flex items-center justify-between">
<div>
{saveStatus === "loading" && (<p className="text-xs lg:text-sm">Auto-saving...</p>)}
{saveStatus === "error" && savedTime && (
<p className="text-xs text-red-600 lg:text-sm">
Error saving, last saved: {savedTime.toString()}
</p>
)}
{saveStatus === "success" && savedTime && (
<p className="text-xs text-neutral-400 lg:text-sm">
Saved: {savedTime.toString()}
</p>
)}
</div>

<button
type="button"
disabled={isDisabled}
className="ml-5 inline-flex justify-center bg-gradient-to-r from-orange-400 to-pink-600 px-4 py-2 text-sm font-medium text-white shadow-sm hover:from-orange-300 hover:to-pink-500 focus:outline-none focus:ring-2 focus:ring-pink-300 focus:ring-offset-2 disabled:opacity-50"
onClick={() => setOpen(true)}
aria-label={data?.published ? "Save changes to document" : "Publish document"}
>
{data?.published ? "Save Changes" : "Publish"}
</button>
</div>
</div>
</div>
Expand Down
53 changes: 46 additions & 7 deletions app/(app)/articles/[slug]/page.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import React from "react";
import type { RenderableTreeNode } from "@markdoc/markdoc";
import Markdoc from "@markdoc/markdoc";
import Link from "next/link";
import BioBar from "@/components/BioBar/BioBar";
Expand All @@ -13,6 +14,10 @@ import ArticleAdminPanel from "@/components/ArticleAdminPanel/ArticleAdminPanel"
import { type Metadata } from "next";
import { getPost } from "@/server/lib/posts";
import { getCamelCaseFromLower } from "@/utils/utils";
import { generateHTML } from "@tiptap/html";
import { TiptapExtensions } from "@/components/editor/editor/extensions";
import DOMPurify from 'isomorphic-dompurify';
import type { JSONContent } from '@tiptap/core';

type Props = { params: { slug: string } };

Expand Down Expand Up @@ -57,6 +62,20 @@ export async function generateMetadata({ params }: Props): Promise<Metadata> {
};
}

const parseJSON = (str: string): JSONContent | null => {
try {
return JSON.parse(str);
} catch (e) {
return null;
}
};

const renderSanitizedTiptapContent = (jsonContent: JSONContent) => {
const rawHtml = generateHTML(jsonContent, [...TiptapExtensions]);
// Sanitize the HTML
return DOMPurify.sanitize(rawHtml);
};
Comment on lines +73 to +77
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider memoizing the TiptapExtensions array

The renderSanitizedTiptapContent function creates a new array spread of TiptapExtensions on every call, which could impact performance for large articles.

+const tiptapExtensions = [...TiptapExtensions];
+
 const renderSanitizedTiptapContent = (jsonContent: JSONContent) => {
-  const rawHtml = generateHTML(jsonContent, [...TiptapExtensions]);
+  const rawHtml = generateHTML(jsonContent, tiptapExtensions);
   // Sanitize the HTML
   return DOMPurify.sanitize(rawHtml);
 };
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const renderSanitizedTiptapContent = (jsonContent: JSONContent) => {
const rawHtml = generateHTML(jsonContent, [...TiptapExtensions]);
// Sanitize the HTML
return DOMPurify.sanitize(rawHtml);
};
const tiptapExtensions = [...TiptapExtensions];
const renderSanitizedTiptapContent = (jsonContent: JSONContent) => {
const rawHtml = generateHTML(jsonContent, tiptapExtensions);
// Sanitize the HTML
return DOMPurify.sanitize(rawHtml);
};


const ArticlePage = async ({ params }: Props) => {
const session = await getServerAuthSession();
const { slug } = params;
Expand All @@ -66,11 +85,25 @@ const ArticlePage = async ({ params }: Props) => {
const post = await getPost({ slug });

if (!post) {
notFound();
return notFound();
}

const parsedBody = parseJSON(post.body);
const isTiptapContent = parsedBody?.type === "doc";

let renderedContent: string | RenderableTreeNode;
dineshsutihar marked this conversation as resolved.
Show resolved Hide resolved

if (isTiptapContent && parsedBody) {
const jsonContent = parsedBody;
renderedContent = renderSanitizedTiptapContent(jsonContent);
} else {
const ast = Markdoc.parse(post.body);
const transformedContent = Markdoc.transform(ast, config);
renderedContent = Markdoc.renderers.react(transformedContent, React, {
components: markdocComponents,
}) as unknown as string;
Comment on lines +91 to +104
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 22, 2024

Choose a reason for hiding this comment

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

⚠️ Potential issue

LGTM: Improved content type detection and rendering

The new logic effectively detects and renders both Tiptap and Markdoc content, aligning with the PR objectives.

Remove the unnecessary type assertion in the Markdoc rendering path:

- renderedContent = Markdoc.renderers.react(transformedContent, React, {
-   components: markdocComponents,
- }) as unknown as string;
+ renderedContent = Markdoc.renderers.react(transformedContent, React, {
+   components: markdocComponents,
+ });

This change eliminates the incorrect casting of a React element to string, improving type safety.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const parsedBody = parseJSON(post.body);
const isTiptapContent = parsedBody?.type === "doc";
let renderedContent: string | RenderableTreeNode;
if (isTiptapContent && parsedBody) {
const jsonContent = parsedBody;
renderedContent = renderSanitizedTiptapContent(jsonContent);
} else {
const ast = Markdoc.parse(post.body);
const transformedContent = Markdoc.transform(ast, config);
renderedContent = Markdoc.renderers.react(transformedContent, React, {
components: markdocComponents,
}) as unknown as string;
const parsedBody = parseJSON(post.body);
const isTiptapContent = parsedBody?.type === "doc";
let renderedContent: string | RenderableTreeNode;
if (isTiptapContent && parsedBody) {
const jsonContent = parsedBody;
renderedContent = renderSanitizedTiptapContent(jsonContent);
} else {
const ast = Markdoc.parse(post.body);
const transformedContent = Markdoc.transform(ast, config);
renderedContent = Markdoc.renderers.react(transformedContent, React, {
components: markdocComponents,
});

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Keep the type assertion because renderedContent needs to be treated as a string for use in dangerouslySetInnerHTML, and removing it could lead to runtime issues.

Copy link
Contributor

Choose a reason for hiding this comment

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

@dineshsutihar, understood. Thanks for the clarification. Keeping the type assertion ensures correct behavior when using dangerouslySetInnerHTML.

🐰🤔


✏️ Learnings added
Learnt from: dineshsutihar
PR: codu-code/codu#1153
File: app/(app)/articles/[slug]/page.tsx:91-104
Timestamp: 2024-10-22T08:43:13.045Z
Learning: In `app/(app)/articles/[slug]/page.tsx`, when rendering content, the `renderedContent` needs to be treated as a string using a type assertion because it's used with `dangerouslySetInnerHTML`, and removing the type assertion could lead to runtime issues.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

}

const ast = Markdoc.parse(post.body);
const content = Markdoc.transform(ast, config);

return (
<>
Expand All @@ -83,10 +116,16 @@ const ArticlePage = async ({ params }: Props) => {
/>
<div className="mx-auto break-words px-2 pb-4 sm:px-4 md:max-w-3xl">
<article className="prose mx-auto max-w-3xl dark:prose-invert lg:prose-lg">
<h1>{post.title}</h1>
{Markdoc.renderers.react(content, React, {
components: markdocComponents,
})}
{!isTiptapContent && (<h1>{post.title}</h1>)}

{isTiptapContent ? (
<div
dangerouslySetInnerHTML={{ __html: renderedContent }}
dineshsutihar marked this conversation as resolved.
Show resolved Hide resolved
className="tiptap-content"
/>
) : (
<div>{renderedContent}</div>
)}
</article>
{post.tags.length > 0 && (
<section className="flex flex-wrap gap-3">
Expand Down
3 changes: 2 additions & 1 deletion components/editor/editor/RenderPost.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import React from "react";
import { TiptapExtensions } from "./extensions";
import { EditorContent, useEditor } from "@tiptap/react";
import SlashCommand from "./extensions/slash-command";

interface RenderPostProps {
json: string;
Expand All @@ -11,7 +12,7 @@ const RenderPost = ({ json }: RenderPostProps) => {

const editor = useEditor({
editable: false,
extensions: [...TiptapExtensions],
extensions: [...TiptapExtensions, SlashCommand],
content,
});

Expand Down
1 change: 0 additions & 1 deletion components/editor/editor/extensions/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,6 @@ export const TiptapExtensions = [
return "type / to see a list of formatting features";
},
}),
SlashCommand,
TextStyle,
Link.configure({
HTMLAttributes: {
Expand Down
Loading
Loading