Skip to content

Conversation

@fordelord
Copy link
Contributor

No description provided.

@netlify
Copy link

netlify bot commented Dec 8, 2022

Deploy Preview for alex-code-blog ready!

Name Link
🔨 Latest commit 732869a
🔍 Latest deploy log https://app.netlify.com/sites/alex-code-blog/deploys/6463376a6da23100073d9c04
😎 Deploy Preview https://deploy-preview-56--alex-code-blog.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@fordelord fordelord changed the title first-commit Editor-mode-code Dec 8, 2022
@1alexvash 1alexvash marked this pull request as draft December 23, 2022 22:39
@1alexvash 1alexvash changed the title Editor-mode-code Editor-mode-code | [Paused] Dec 23, 2022
@1alexvash 1alexvash changed the title Editor-mode-code | [Paused] Editor mode Jan 9, 2023
@1alexvash 1alexvash marked this pull request as ready for review January 9, 2023 14:33
const docs = slugs
.map((slug) => getPostDocumentBySlug(slug))
.filter((post: PostDocument) => {
if (isPostADraft(post) || isPostInTheFuture(post)) {
Copy link
Owner

Choose a reason for hiding this comment

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

We repeat those lines of code a lot

isPostADraft(post) || isPostInTheFuture(post)

Create a combined helper isUpcomingPost

@1alexvash 1alexvash marked this pull request as draft January 23, 2023 07:06
@fordelord fordelord marked this pull request as ready for review January 30, 2023 21:31
const [isFirstRender, setIsFirstRender] = useState(true);

const handleClick = () => {
if (admin === true) {
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
if (admin === true) {
if (admin) {

It's a same thing

imageWrapper?.setAttribute("admin", "");
editIcon?.setAttribute("admin", "");
setIsFirstRender(false);
}
Copy link
Owner

Choose a reason for hiding this comment

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

Add a break line down below, for the consistency

editIcon?.setAttribute("admin", "");
setIsFirstRender(false);
}
if (!admin && isFirstRender === false) {
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
if (!admin && isFirstRender === false) {
if (!admin && !isFirstRender) {

Use the same syntax for both of them

}, [admin, isFirstRender]);

return (
<section className="intro-section">
Copy link
Owner

Choose a reason for hiding this comment

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

We can do MUI in here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#82 - refactor of MUI component. I think it will be easier for you to check this refactor if it will be like another PR.

? "grayscale(50%)"
: "none",

filter: isUpcomingPost(post) ? "grayscale(50%)" : "none",
Copy link
Owner

Choose a reason for hiding this comment

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

Love this utility function

Comment on lines +48 to +61

export function getUpcomingPosts(): PostDocumentWithoutContent[] {
const slugs = fs.readdirSync(documentsDirectory);
const docs = slugs
.map((slug) => getPostDocumentBySlug(slug))
.filter((post: PostDocument) => isUpcomingPost(post))
.sort((post1, post2) => (post1.date > post2.date ? -1 : 1))
.map((post) => {
const { content, ...postWithoutContent } = post;
return postWithoutContent;
});

return JSONSerialize(docs);
}
Copy link
Owner

Choose a reason for hiding this comment

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

Tina CMS is fetching posts somewhat differently, so it might cause some merge conflicts

I might merge Tina first, and only then you're going to update this utility method

pages/_app.tsx Outdated
Comment on lines 8 to 9
import AdminProvider from "@/components/AdminProvider";
import ThemeProvider from "@/components/MUI/ThemeProvider";
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
import AdminProvider from "@/components/AdminProvider";
import ThemeProvider from "@/components/MUI/ThemeProvider";
import AdminProvider from "components/AdminProvider";
import ThemeProvider from "components/MUI/ThemeProvider";

It's a same thing, is there any reason we're using @ symbol?

pages/index.tsx Outdated
<section className="simple-section">
<div className="container">
{admin && <UpcomingPosts posts={upcomingPosts} />}
{/* TODO: Implement tags count for the admin user */}
Copy link
Owner

Choose a reason for hiding this comment

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

I think you've done this TODO item

const posts = getUpcomingPosts();

res.status(200).json(posts);
}
Copy link
Owner

Choose a reason for hiding this comment

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

Tina GraphQL query might look a bit differently, so just heads up

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, waiting for merge of Tina and after will be fixing that

styles/main.scss Outdated
}

.intro-avatar .image img {
.intro-avatar .image-avatar:after {
Copy link
Owner

Choose a reason for hiding this comment

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

We won't need it with MUI, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yea. I add the PR with refactor of Intro component to mui. I think it will be easier for you to check this refactor if it will be like another PR. #82

@fordelord fordelord marked this pull request as draft March 6, 2023 09:08
@fordelord fordelord marked this pull request as ready for review March 6, 2023 09:25
@fordelord fordelord requested a review from 1alexvash March 13, 2023 19:29
@ntorbinskiy ntorbinskiy deleted the editor-mode-code- branch August 1, 2023 07:44
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.

4 participants