FR | image files #2
Conversation
| @@ -0,0 +1,118 @@ | |||
| import styled from "styled-components"; | |||
There was a problem hiding this comment.
There is some } missing in this file. Please check it.
| @@ -0,0 +1,141 @@ | |||
| import React, { useState } from "react"; | |||
There was a problem hiding this comment.
Refactor this component to smaller components and separate logic from presentation.
| @@ -0,0 +1,74 @@ | |||
| import React from "react"; | |||
There was a problem hiding this comment.
Adjust the modal styling to always be in the center.
src/Asset/SvgImg.js
Outdated
| @@ -0,0 +1,16 @@ | |||
| import React from 'react' | |||
There was a problem hiding this comment.
Please rename the file to be more specific may be like SeatIcon or SeatComponent. Since this is a react component, change extension to jsx to stay consistent with naming and move it with other react components.
src/components/Modal.js
Outdated
| <MoviePic src={imgUrl + param.path} alt="movie" /> | ||
|
|
||
| <article> | ||
| <h3>Seats:</h3> |
There was a problem hiding this comment.
The seats text is not as per the design. Please check and update.
src/App.js
Outdated
| <> | ||
| <Routes> | ||
| <Route path="/" element={<HomePage/>}/> | ||
| <Route path="/bookingseat/:id/:path" element={<BookingSeat/>}/> |
There was a problem hiding this comment.
The redirected url when a movie is clicked - http://localhost:3000/bookingseat/438148/wKiOkZTN9lUUUNZLmtnwubZYONg.jpg The url should be - http://localhost:3000/book/438148/minions-the-rise-of-gru
|
Please use tool to format the code to have even indents and consistent spacing. |
src/components/MovieSeats.js
Outdated
| function handleSeats(id) { | ||
| if (selected.includes(id)) { | ||
| alert("already selected"); | ||
| let data = selectedSeats.filter((item) => { |
There was a problem hiding this comment.
Conflicting variable names. Please fix it.
src/components/BookingSeat.js
Outdated
| setSelectedSeats={setSelectedSeats} | ||
| /> | ||
| <ConfirmButton onClick={modalHandle}>Confirm booking</ConfirmButton> | ||
| <article> |
There was a problem hiding this comment.
Please remove this article tag.
src/components/BookingSeat.js
Outdated
| const [selectedSeats, setSelectedSeats] = useState([]); | ||
|
|
||
| let param = useParams(); | ||
| let selected = []; |
There was a problem hiding this comment.
Please elaborate the difference between the variable names selectedSeats and selected array.
src/components/HomePage.js
Outdated
| }, | ||
| []) | ||
|
|
||
| async function getMovies(url) { |
There was a problem hiding this comment.
Please declare function before calling the function.
| setMovies(result); | ||
| } | ||
|
|
||
| const submit = (e) => { |
There was a problem hiding this comment.
Please be consistent with syntax. either choose function keyword and use it across the app or choose arrow function syntax and use it across the app. Do not mix both.
src/components/Modal.js
Outdated
|
|
||
| import close from "../Asset/Xclose.png"; | ||
| import { useParams } from "react-router"; | ||
| const imgUrl = "https://image.tmdb.org/t/p/w500/"; |
There was a problem hiding this comment.
There is already a constant file to maintain all the urls, please move this variable to that file.
src/components/HomePage.js
Outdated
|
|
||
| return ( | ||
| <> | ||
| <GlobalStyles /> |
There was a problem hiding this comment.
Please add global styles in the index.js file instead of using it in a particular component.
src/components/HomePage.js
Outdated
| []) | ||
|
|
||
| async function getMovies(url) { | ||
| const response = await axios.get(url); |
There was a problem hiding this comment.
Please add error handling. Always wrap the API calls inside try catch block and handle errors if any.
| align-items: center; | ||
| position: relative; | ||
|
|
||
| ${ButtonStyle} { |
There was a problem hiding this comment.
👍 This is how we should be using styled components to style nested components.
No description provided.