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

Jse/optim/time ago component #5

Open
wants to merge 20 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 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
16 changes: 16 additions & 0 deletions doc/review/meme-feed-code-review.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# Meme feed code review report

## Identified Problems:

- The entire feed is loaded recursively on the home page.
- The components are not sufficiently broken down.
- The `index.tsx` file of the feed handles API calls, data manipulation, and display at the same time.
- All comments for each meme are loaded from the start.
- The publication time of memes and comments does not take into account the user's timezone.

## Solutions Provided:

- Breakdown of the feed into: `MemeListLayout` + `MemeList` (for data control) + `MemeCard` (for display).
- Added `useInView` and `useInfiniteQuery` to load `MemeCard` on scroll.
- Only the first page of comments is preloaded, the rest are loaded on scroll once the comments are opened.
- Added a temporary fix for timezones while awaiting a backend fix: `format(comment.createdAt + "Z")`.
299 changes: 200 additions & 99 deletions package-lock.json

Large diffs are not rendered by default.

8 changes: 7 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,19 @@
"@phosphor-icons/react": "^2.1.7",
"@tanstack/react-query": "^5.51.17",
"@tanstack/react-router": "^1.46.0",
"@types/js-cookie": "^3.0.6",
"axios": "^1.7.7",
"eventemitter3": "^5.0.1",
"framer-motion": "^11.3.19",
"js-cookie": "^3.0.5",
"jwt-decode": "^4.0.0",
"react": "^18.3.1",
"react-dom": "^18.3.1",
"react-dropzone": "^14.2.3",
"react-hook-form": "^7.52.1",
"timeago.js": "^4.0.2"
"react-intersection-observer": "^9.13.1",
"timeago.js": "^4.0.2",
"zod": "^3.23.8"
},
"devDependencies": {
"@tanstack/router-devtools": "^1.46.0",
Expand Down
77 changes: 56 additions & 21 deletions src/__tests__/routes/_authentication/index.test.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { screen, waitFor } from "@testing-library/react";
import { screen, waitFor, fireEvent } from "@testing-library/react";
import { ChakraProvider } from "@chakra-ui/react";
import { QueryClientProvider, QueryClient } from "@tanstack/react-query";
import { AuthenticationContext } from "../../../contexts/authentication";
Expand Down Expand Up @@ -37,42 +37,77 @@ describe("routes/_authentication/index", () => {

await waitFor(() => {
// We check that the right author's username is displayed
expect(screen.getByTestId("meme-author-dummy_meme_id_1")).toHaveTextContent('dummy_user_1');

expect(
screen.getByTestId("meme-author-dummy_meme_id_1")
).toHaveTextContent("dummy_user_1");

//click in comments
fireEvent.click(
screen.getByTestId("meme-comments-section-dummy_meme_id_1")
);

// We check that the right meme's picture is displayed
expect(screen.getByTestId("meme-picture-dummy_meme_id_1")).toHaveStyle({
'background-image': 'url("https://dummy.url/meme/1")',
"background-image": 'url("https://dummy.url/meme/1")',
});

// We check that the right texts are displayed at the right positions
const text1 = screen.getByTestId("meme-picture-dummy_meme_id_1-text-0");
const text2 = screen.getByTestId("meme-picture-dummy_meme_id_1-text-1");
expect(text1).toHaveTextContent('dummy text 1');
expect(text1).toHaveTextContent("dummy text 1");
expect(text1).toHaveStyle({
'top': '0px',
'left': '0px',
top: "0px",
left: "0px",
});
expect(text2).toHaveTextContent('dummy text 2');
expect(text2).toHaveTextContent("dummy text 2");
expect(text2).toHaveStyle({
'top': '100px',
'left': '100px',
top: "100px",
left: "100px",
});

// We check that the right description is displayed
expect(screen.getByTestId("meme-description-dummy_meme_id_1")).toHaveTextContent('dummy meme 1');

expect(
screen.getByTestId("meme-description-dummy_meme_id_1")
).toHaveTextContent("dummy meme 1");

// We check that the right number of comments is displayed
expect(screen.getByTestId("meme-comments-count-dummy_meme_id_1")).toHaveTextContent('3 comments');

expect(
screen.getByTestId("meme-comments-count-dummy_meme_id_1")
).toHaveTextContent("3 comments");

// We check that the right comments with the right authors are displayed
expect(screen.getByTestId("meme-comment-content-dummy_meme_id_1-dummy_comment_id_1")).toHaveTextContent('dummy comment 1');
expect(screen.getByTestId("meme-comment-author-dummy_meme_id_1-dummy_comment_id_1")).toHaveTextContent('dummy_user_1');
expect(
screen.getByTestId(
"meme-comment-content-dummy_meme_id_1-dummy_comment_id_1"
)
).toHaveTextContent("dummy comment 1");
expect(
screen.getByTestId(
"meme-comment-author-dummy_meme_id_1-dummy_comment_id_1"
)
).toHaveTextContent("dummy_user_1");

expect(
screen.getByTestId(
"meme-comment-content-dummy_meme_id_1-dummy_comment_id_2"
)
).toHaveTextContent("dummy comment 2");
expect(
screen.getByTestId(
"meme-comment-author-dummy_meme_id_1-dummy_comment_id_2"
)
).toHaveTextContent("dummy_user_2");

expect(screen.getByTestId("meme-comment-content-dummy_meme_id_1-dummy_comment_id_2")).toHaveTextContent('dummy comment 2');
expect(screen.getByTestId("meme-comment-author-dummy_meme_id_1-dummy_comment_id_2")).toHaveTextContent('dummy_user_2');

expect(screen.getByTestId("meme-comment-content-dummy_meme_id_1-dummy_comment_id_3")).toHaveTextContent('dummy comment 3');
expect(screen.getByTestId("meme-comment-author-dummy_meme_id_1-dummy_comment_id_3")).toHaveTextContent('dummy_user_3');
expect(
screen.getByTestId(
"meme-comment-content-dummy_meme_id_1-dummy_comment_id_3"
)
).toHaveTextContent("dummy comment 3");
expect(
screen.getByTestId(
"meme-comment-author-dummy_meme_id_1-dummy_comment_id_3"
)
).toHaveTextContent("dummy_user_3");
});
});
});
Expand Down
12 changes: 5 additions & 7 deletions src/__tests__/routes/login.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,11 @@ import { vi } from "vitest";
import { act, fireEvent, waitFor, screen } from "@testing-library/react";
import { renderWithRouter } from "../utils";
import { LoginPage } from "../../routes/login";
import {
AuthenticationContext,
AuthenticationState,
} from "../../contexts/authentication";
import { AuthenticationContext } from "../../contexts/authentication";
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
import { ChakraProvider } from "@chakra-ui/react";
import { ListenerFn, RouterEvents } from "@tanstack/react-router";
import { AuthenticationState } from "../../types/authentication-state";

type RenderLoginPageParams = {
authenticate?: (token: string) => void;
Expand Down Expand Up @@ -99,7 +97,7 @@ describe("routes/login", () => {

await waitFor(() => {
expect(
screen.getByText(/an unknown error occured, please try again later/i),
screen.getByText(/an unknown error occured, please try again later/i)
).toBeInTheDocument();
});
});
Expand All @@ -119,7 +117,7 @@ describe("routes/login", () => {
expect(onBeforeNavigateMock).toHaveBeenCalledWith(
expect.objectContaining({
toLocation: expect.objectContaining({ pathname: "/" }),
}),
})
);
});
});
Expand All @@ -140,7 +138,7 @@ describe("routes/login", () => {
expect(onBeforeNavigateMock).toHaveBeenCalledWith(
expect.objectContaining({
toLocation: expect.objectContaining({ pathname: "/profile" }),
}),
})
);
});
});
Expand Down
149 changes: 0 additions & 149 deletions src/api.ts

This file was deleted.

Loading