Skip to content

Conversation

@Deniisolo
Copy link

No description provided.

Deniisolo and others added 30 commits April 24, 2024 14:07
feat: i do typescript type
feat: I add formatMovie function and unit testing
Fix: unit testing and formatMovie
feat> add css to moviecard, movielist and app
feat: add unit testing for MovieCard
feat: add unit testing for MovieList
feat: add  update of get movies to pagination
feat: i start pagination component
Deniisolo and others added 27 commits May 15, 2024 15:30
feat: finish  pagination component
feat: add a unit testing for paginanation component
feat: add formatGenresToMap
feat: i start movieDetail component
feat: add  the  last component (movie detail)
feat: (readme)i change readme
<html lang="en">
<head>
<meta charset="UTF-8" />
<link rel="icon" type="image/svg+xml" href="/vite.svg" />

Choose a reason for hiding this comment

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

change the default icon

},
"dependencies": {
"dotenv": "^16.4.5",
"jsdom": "^24.1.0",

Choose a reason for hiding this comment

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

this is a dependence of develp

Comment on lines +7 to +12
<link rel="preconnect" href="https://fonts.googleapis.com" />
<link rel="preconnect" href="https://fonts.gstatic.com" crossorigin />
<link
href="https://fonts.googleapis.com/css2?family=Lexend:[email protected]&display=swap"
rel="stylesheet"
/>

Choose a reason for hiding this comment

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

Maybe you can move the fonts to style.css

<style> @import url('https://fonts.googleapis.com/css2?family=Roboto:ital,wght@0,100;0,300;0,400;0,500;0,700;0,900;1,100;1,300;1,400;1,500;1,700;1,900&display=swap'); </style>

"react-dom": "^18.2.0",
"react-loading": "^2.0.3",
"react-router-dom": "^6.23.0",
"vitest": "^1.6.0"

Choose a reason for hiding this comment

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

tienes 2 entornos de test

Comment on lines +3 to +4
import Home from "./components/Home";
import { MovieDetail } from "./components/MovieDetail";

Choose a reason for hiding this comment

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

Suggested change
import Home from "./components/Home";
import { MovieDetail } from "./components/MovieDetail";
import Home from "./components/Home";
import MovieDetail from "./components/MovieDetail";

consistency

Choose a reason for hiding this comment

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

All components with export default

];

setTimeout(() => {
expect(getByText(moviesExpected[0].title)).toBeInTheDocument();

Choose a reason for hiding this comment

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

Maybe try snapshot

id="seletFilter"
value={selectedOption?.value || ""}
onChange={(event) => {
const selectedOption = options.find(option => option.value === event.target.value);

Choose a reason for hiding this comment

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

useMemo

@@ -0,0 +1,10 @@
import React from 'react'

Choose a reason for hiding this comment

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

In the future try install eslint and prettier

Comment on lines +6 to +20
export function getMovieGenres(): Promise<[{ id: number; name: string }]> {
return fetch(`https://api.themoviedb.org/3/genre/movie/list?language=en`, {
method: "GET",
headers: {
accept: "application/json",
Authorization: `Bearer ${token}`,
},
})
.then((response) => response.json())
.then((genresResult) => {
return genresResult.genres;
})
.catch((error) => {
throw error;
});

Choose a reason for hiding this comment

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

async/ await

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.

2 participants