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.

Sign up for GitHub

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

Jump to bottom

Add new Navigation for editor #1163

Merged
merged 4 commits into from
Oct 21, 2024
Merged

Add new Navigation for editor #1163

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2304cf2
Add new navbar for editor
NiallJoeMaher Oct 20, 2024
2294f7f
Update article test
NiallJoeMaher Oct 20, 2024
3d55083
Update Links from Next Link
NiallJoeMaher Oct 20, 2024
cdc2c69
Remove unused code
NiallJoeMaher Oct 20, 2024
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
16 changes: 0 additions & 16 deletions app/(app)/create/[[...paramsArr]]/page.tsx
View file
Open in desktop

This file was deleted.

23 changes: 7 additions & 16 deletions app/(app)/layout.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,22 +79,13 @@ export default async function RootLayout({

return (
<>
<ProgressBar />
<AuthProvider>
<ThemeProvider>
<TRPCReactProvider headers={headers()}>
<PromptProvider>
<Nav
session={session}
algoliaSearchConfig={algoliaSearchConfig}
username={userData?.username || null}
/>
{children}
<Footer />
</PromptProvider>
</TRPCReactProvider>
</ThemeProvider>
</AuthProvider>
<Nav
session={session}
algoliaSearchConfig={algoliaSearchConfig}
username={userData?.username || null}
/>
{children}
<Footer />
</>
);
}
10 changes: 5 additions & 5 deletions app/(app)/my-posts/_client.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import { useSearchParams } from "next/navigation";
import { api } from "@/server/trpc/react";
import { Tabs } from "@/components/Tabs";
import { PromptDialog } from "@/components/PromptService";
import { PostStatus, getPostStatus } from "@/utils/post";
import { status, getPostStatus } from "@/utils/post";

function classNames(...classes: string[]) {
return classes.filter(Boolean).join(" ");
Expand Down Expand Up @@ -135,7 +135,7 @@ const MyPosts = () => {
}) => {
const postStatus = published
? getPostStatus(new Date(published))
: PostStatus.DRAFT;
: status.DRAFT;
return (
<article
className="mb-4 border border-neutral-300 bg-white p-4 dark:bg-neutral-900"
Expand All @@ -158,11 +158,11 @@ const MyPosts = () => {
</p>
<div className="flex items-center">
<div className="flex-grow">
{published && postStatus === PostStatus.SCHEDULED ? (
{published && postStatus === status.SCHEDULED ? (
<>
{renderDate("Scheduled to publish on ", published)}
</>
) : published && postStatus === PostStatus.PUBLISHED ? (
) : published && postStatus === status.PUBLISHED ? (
<>
{/*If updatedAt is greater than published by more than on minutes show updated at else show published
as on updating published updatedAt is automatically updated and is greater than published*/}
Expand All @@ -174,7 +174,7 @@ const MyPosts = () => {
<>{renderDate("Published on ", published)}</>
)}
</>
) : postStatus === PostStatus.DRAFT ? (
) : postStatus === status.DRAFT ? (
<>{renderDate("Last updated on ", updatedAt)}</>
) : null}
</div>
Expand Down
108 changes: 53 additions & 55 deletions ...(app)/create/[[...paramsArr]]/_client.tsx → ...itor)/create/[[...paramsArr]]/_client.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,16 +22,23 @@ import { useMarkdownHotkeys } from "@/markdoc/editor/hotkeys/hotkeys.markdoc";
import { useMarkdownShortcuts } from "@/markdoc/editor/shortcuts/shortcuts.markdoc";
import { markdocComponents } from "@/markdoc/components";
import { config } from "@/markdoc/config";
import { useParams, useRouter } from "next/navigation";
import { notFound, useParams, useRouter } from "next/navigation";
import { usePrompt } from "@/components/PromptService";
import { Switch } from "@/components/Switch/Switch";
import copy from "copy-to-clipboard";
import { PostStatus, getPostStatus, isValidScheduleTime } from "@/utils/post";
import {
type PostStatus,
getPostStatus,
isValidScheduleTime,
status,
} from "@/utils/post";
import { ImageUp, LoaderCircle } from "lucide-react";
import { uploadFile } from "@/utils/s3helpers";
import { getUploadUrl } from "@/app/actions/getUploadUrl";
import EditorNav from "./navigation";
import { type Session } from "next-auth";

const Create = () => {
const Create = ({ session }: { session: Session | null }) => {
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

Pass 'session' prop to 'EditorNav' component

The session prop is now passed to the Create component and then forwarded to the EditorNav component. Ensure that this prop is necessary and used appropriately within EditorNav. If EditorNav can access the session internally (e.g., via a context or a hook), consider removing the prop to simplify the component interface.

const params = useParams();
const router = useRouter();

Expand All @@ -52,6 +59,7 @@ const Create = () => {
const [uploadStatus, setUploadStatus] = useState<
"loading" | "error" | "success" | "default"
>("default");
const [postStatus, setPostStatus] = useState<PostStatus | null>(null);
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

Initialize 'postStatus' with consistent default value

The postStatus state is initialized with null. Since postStatus is used to determine the UI state, consider initializing it with a default status, such as status.DRAFT, to avoid potential null checks elsewhere in the code.


const { unsavedChanges: _unsaved, setUnsavedChanges: _setUnsaved } =
usePrompt();
Expand Down Expand Up @@ -186,16 +194,10 @@ const Create = () => {
toast.error("Error saving");
}

if (isSuccess) {
toast.success("Saved");
}

if (draftFetchError) {
toast.error(
"Something went wrong fetching your draft, refresh your page or you may lose data",
);
notFound();
}
}, [draftFetchError, isError, isSuccess]);
}, [draftFetchError, isError]);
Comment on lines +198 to +200
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Consider alternative error handling for draft fetch failures.

While using notFound() for draft fetch errors improves the UX by avoiding error toasts, it might confuse users who expect to edit their draft. Consider displaying an error message or redirecting to a dedicated error page that explains the issue more clearly.


useEffect(() => {
if (shouldRefetch) {
Expand Down Expand Up @@ -227,7 +229,6 @@ const Create = () => {
await create({ ...formData });
} else {
await save({ ...formData, id: postId });
toast.success("Saved");
setSavedTime(
new Date().toLocaleString(undefined, {
dateStyle: "medium",
Expand All @@ -243,9 +244,9 @@ const Create = () => {
saveStatus === "loading" ||
dataStatus === "loading";

const postStatus = data?.published
const currentPostStatus = data?.published
? getPostStatus(new Date(data.published))
: PostStatus.DRAFT;
: status.DRAFT;
Comment on lines +247 to +249
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Reconsider setting postStatus to null based on content length.

While the changes improve post status management, setting postStatus to null when the combined length of title and body is less than 5 might lead to unintended UI states. Instead, consider using a separate state or validation function to handle content length requirements without altering the post status.

Also applies to: 345-356


const onSubmit = async (inputData: SavePostInput) => {
// validate markdoc syntax
Expand All @@ -264,7 +265,7 @@ const Create = () => {

await savePost();

if (postStatus === PostStatus.PUBLISHED) {
if (currentPostStatus === status.PUBLISHED) {
if (data) {
router.push(`/articles/${data.slug}`);
}
Expand Down Expand Up @@ -341,10 +342,21 @@ const Create = () => {
published: published ? published : undefined,
});
setIsPostScheduled(published ? new Date(published) > new Date() : false);
setPostStatus(
published ? getPostStatus(new Date(published)) : status.DRAFT,
);
Comment on lines +345 to +347
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

Set 'postStatus' consistently after data fetch

After fetching the post data, postStatus is updated based on published. Ensure that this logic aligns with how postStatus is used elsewhere, and that it doesn't conflict with currentPostStatus. Consolidating these statuses might improve code clarity.

}, [data]);

useEffect(() => {
if (postStatus !== PostStatus.DRAFT) return;
if ((title + body).length < 5) {
setPostStatus(null);
} else if (postStatus === null) {
setPostStatus(status.DRAFT);
}
}, [title, body]);
Comment on lines +351 to +356
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Avoid setting 'postStatus' to null based on content length

Setting postStatus to null when the combined length of title and body is less than 5 might lead to unintended UI states. Instead, consider using a separate state or validation function to handle content length requirements without altering the post status.


useEffect(() => {
if (currentPostStatus !== status.DRAFT) return;
if ((title + body).length < 5) return;
if (debouncedValue === (data?.title || "") + data?.body) return;
if (allowUpdate) savePost();
Expand Down Expand Up @@ -374,8 +386,21 @@ const Create = () => {
}
}, [publishStatus, publishData, isPostScheduled, router]);

const handlePublish = () => {
if (isDisabled) return;
setOpen(true);
};
Comment on lines +389 to +392
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

Simplify nested ternary operators for button label.

The nested ternary operators used to determine the button label make the code difficult to read and maintain. Consider extracting this logic into a separate function or variable to improve readability.

Apply this refactor:

-                            {currentPostStatus === status.PUBLISHED
-                              ? "Save changes"
-                              : currentPostStatus === status.DRAFT
-                                ? isPostScheduled
-                                  ? "Schedule"
-                                  : "Publish now"
-                                : currentPostStatus === status.SCHEDULED
-                                  ? isPostScheduled
-                                    ? "Update schedule"
-                                    : "Publish now"
-                                  : "Publish"}
+                            {getButtonLabel(currentPostStatus, isPostScheduled)}

Define the getButtonLabel function:

const getButtonLabel = (postStatus, isPostScheduled) => {
  if (postStatus === status.PUBLISHED) {
    return "Save changes";
  } else if (postStatus === status.DRAFT) {
    return isPostScheduled ? "Schedule" : "Publish now";
  } else if (postStatus === status.SCHEDULED) {
    return isPostScheduled ? "Update schedule" : "Publish now";
  } else {
    return "Publish";
  }
};

Also applies to: 577-616


return (
<>
<EditorNav
session={session}
username={session?.user?.name || null}
postStatus={postStatus}
unsavedChanges={unsavedChanges}
onPublish={handlePublish}
isDisabled={isDisabled}
/>
<form onSubmit={handleSubmit(onSubmit)}>
<Transition show={open} as={Fragment}>
<div className="old-input fixed bottom-0 left-0 top-0 z-50 h-screen w-full overflow-y-scroll bg-white dark:bg-black">
Expand Down Expand Up @@ -549,7 +574,7 @@ const Create = () => {
</Disclosure>
</div>
<div className="mt-4 flex w-full justify-end sm:col-span-12 sm:mt-0">
{postStatus === PostStatus.DRAFT && (
{currentPostStatus === status.DRAFT && (
<button
type="button"
disabled={isDisabled}
Expand Down Expand Up @@ -578,14 +603,17 @@ const Create = () => {
</>
) : (
<>
{postStatus === PostStatus.DRAFT &&
(isPostScheduled ? "Schedule" : "Publish now")}
{postStatus === PostStatus.SCHEDULED &&
(isPostScheduled
? "Save changes"
: "Publish now")}
{postStatus === PostStatus.PUBLISHED &&
"Save changes"}
{currentPostStatus === status.PUBLISHED
? "Save changes"
: currentPostStatus === status.DRAFT
? isPostScheduled
? "Schedule"
: "Publish now"
: currentPostStatus === status.SCHEDULED
? isPostScheduled
? "Update schedule"
: "Publish now"
: "Publish"}
Comment on lines +606 to +616
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

Simplify nested ternary operators for button label

The nested ternary operators used to determine the button label make the code difficult to read and maintain. Consider extracting this logic into a separate function or variable to improve readability.

Apply this refactor:

-                                {currentPostStatus === status.PUBLISHED
-                                  ? "Save changes"
-                                  : currentPostStatus === status.DRAFT
-                                    ? isPostScheduled
-                                      ? "Schedule"
-                                      : "Publish now"
-                                    : currentPostStatus === status.SCHEDULED
-                                      ? isPostScheduled
-                                        ? "Update schedule"
-                                        : "Publish now"
-                                      : "Publish"}
+                                {getButtonLabel(currentPostStatus, isPostScheduled)}

Define the getButtonLabel function:

const getButtonLabel = (postStatus, isPostScheduled) => {
  if (postStatus === status.PUBLISHED) {
    return "Save changes";
  } else if (postStatus === status.DRAFT) {
    return isPostScheduled ? "Schedule" : "Publish now";
  } else if (postStatus === status.SCHEDULED) {
    return isPostScheduled ? "Update schedule" : "Publish now";
  } else {
    return "Publish";
  }
};

</>
)}
</button>
Expand Down Expand Up @@ -743,36 +771,6 @@ const Create = () => {
inputRef={textareaRef}
/>
</div>

<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)}
>
Next
</button>
</div>
</div>
</div>
)}
</div>
Expand Down
Loading
Loading