Conversation
| @@ -1,58 +0,0 @@ | |||
| # Photo Album sites. | |||
There was a problem hiding this comment.
Nice readme, some English mistakes, you could try to use some VS code extension for grammar.
| </p> | ||
| <p className="mb-1">Photo Web Album - Here you can collect yours photos. You can gropued by category, tags ...</p> | ||
| <p className="mb-0"> | ||
| Maid by Maciej using Bootstrap ver 5.0 |
There was a problem hiding this comment.
nice one ;). "Pokojówka Macieja korzystająca z Bootstrap w wersji 5.0"
| @@ -1,38 +0,0 @@ | |||
| .App { | |||
There was a problem hiding this comment.
Are styles defined in this file used?
| @@ -1,42 +0,0 @@ | |||
| import './App.css'; | |||
| @@ -1,8 +0,0 @@ | |||
| import { render, screen } from '@testing-library/react'; | |||
There was a problem hiding this comment.
I think you can remove this file.
|
|
||
| <link href="https://cdn.jsdelivr.net/npm/bootstrap@5.0.0-beta1/dist/css/bootstrap.min.css" rel="stylesheet" integrity="sha384-giJF6kkoqNQ00vy+HMDP7azOuL0xtbfIcaT9wjKHr8RbDVddVHyTfAAsrekwKmP1" crossorigin="anonymous"> | ||
|
|
||
| <style> |
There was a problem hiding this comment.
Do we need this as inline style or can it also be placed in style.scss?
| border-width: 0px; | ||
| text-transform: lowercase; | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
It's good practice to have newline at end o file, see: https://stackoverflow.com/questions/729692/why-should-text-files-end-with-a-newline#:~:text=If%20content%20is%20added%20to,to%20include%20a%20newline%20character.
Also you can add some linting extensions for your IDE to check JS and styles code for consistent formatting and simple things like newlines for example.
| background-color: $main-color; | ||
| color: #FFF; | ||
| font-size: 1rem; | ||
| border-radius: 50px; |
There was a problem hiding this comment.
Using more constants for default values of margin, padding, font-size etc. would be nice.
| import { Link } from "react-router-dom"; | ||
| import { timestamp } from './../firebase/config'; | ||
| import useComments from './../composables/useComments'; | ||
| // import getComments from './../composables/getComments'; |
|
|
||
| const Album = ({ title, description, imageUrl, createdAt, category, id }) => { | ||
|
|
||
| const timeSince = (date) => { |
There was a problem hiding this comment.
extract to some utility file.
| const [comments, setComments] = useState([]); | ||
| const [comment, setComment] = useState([]); | ||
| const { error, addComment } = useComments('comments', id); | ||
| // const { comments } = getComments('comments', id); |
MLewiDev
left a comment
MLewiDev
left a comment
There was a problem hiding this comment.
The Good:
- Good cohesive chunks of code
- Good structure of application
- Simple and easy to understand components
- Properly extracted reusable parts
- Good use of html and styling
Things to Improve:
- More structure in styles
- Some grammar issues, check if some extension could help you a little bit with that
- Some linting could be improved for more consistency across files
Remarks:
- Didn't check firebase related API calls
- Testing isn't considered as scope of this task
- I found some areas of app that could be improved like for example "enter on comment input works as reload of page", but they are considered as further improvements and not bugs.
Conclusion
Very nice code for a first attempt in React. It's readable and maintainable, properly structured with clean HTML. Only some minor things to improve from my side.
| return Math.floor(interval) + " minutes ago"; | ||
| } | ||
|
|
||
| if (seconds === 0) { |
There was a problem hiding this comment.
You can use here <= 0 as I saw a bug that in the beginning the time starts at minus values.
| } | ||
| </datalist> | ||
| </div> | ||
| {/* <input className="form-control me-2 search-bar" type="search" placeholder="Search" aria-label="Search" onChange={searchAlbum} /> */} |
| ); | ||
| } | ||
|
|
||
| export default Hero; No newline at end of file |
| <div className="row"> | ||
| <div className="col-lg-6 col-md-8 mx-auto"> | ||
| <h1 className="fw-light">Photo Album</h1> | ||
| <p className="lead text-muted">Here you can collect yours photos. You can gropued by category, tags ... </p> |
There was a problem hiding this comment.
...your photos....grouped....
| } | ||
|
|
||
| return ( | ||
|
|
There was a problem hiding this comment.
Why do you have sometimes whitelines here and sometimes not?
| const [error, setError] = useState(null); | ||
|
|
||
| const uploadImage = async (file) => { | ||
| let filePath = `images/${file.name}`; |
| @@ -1,29 +0,0 @@ | |||
| import { projectFirestore } from '../firebase/config' | |||
There was a problem hiding this comment.
why is it getComments and not useComments?
| @@ -1,38 +0,0 @@ | |||
| import { projectFirestore } from '../firebase/config' | |||
There was a problem hiding this comment.
why getCollection no useCollection?
| setFileError(null); | ||
| } else { | ||
| setFile(null); | ||
| setFileError('Wron type of file.'); |
| <h2>Create New Album</h2> | ||
| <form onSubmit={handleSubmit} className="php-email-form"> | ||
| <div className="form-group mt-3"> | ||
| <input |
There was a problem hiding this comment.
You could add some guidance which ones are required
1 participant
Pull request for review purposes.