1. Do the components render correctly? If not, what's wrong in the code?
Yes to some extent, the components render correctly but there are some issues in the code that need to be addressed:
The signup and signin page are not working due to the API not returning data.
- The ProtectedRoute component is missing the path prop in the Route component.
const ProtectedRoute = ({ component: Component, ...props }) => {
return (
<Route exact path={props.path}>
{() =>
props.loggedIn ? <Component {...props} /> : <Redirect to="./signin" />
}
</Route>
);
};
This affects the routing of the application and the protected routes are not working as expected and the user is redirected to the signin page even if they are logged in.
-
There is an unnecessary
debugger
statement in theLogin
component which should be removed for production. -
Add proper error handling for API calls -
handleUpdateUser
,handleUpdateAvatar
,handleCardLike
,handleCardDelete
,handleAddPlaceSubmit
functions need proper error handling with a .catch block to avoid unreponsive UI.(e.g. show a toast message or a modal with the error message) just like the case for theonLogin
function. -
Use proper keys in lists - In the
App
component, mapping over cards using the index as the key is not recommended. Instead, use the card's unique id as the key or generate a unique key for each card orcrypto.randomUUID()
oruuid
library. -
Add proper form validation for the signup and signin forms. Use a library like
yup
orjoi
orzod
for form validation. -
Handle loading states and errors in useEffect hooks - useEffect hooks for fetching user info and cards should handle potential errors and include proper cleanup functions.
-
Context Usage - The
CurrentUserContext
should be initialized with a default value likenull
or an empty object{}
to avoid undefined errors when the context is used in components.
2. Is it possible to simplify the project infrastructure? How would you recommend reorganizing it?
The project structure can be simplified by moving components that are pages to a separate folder like pages
or views
. This will make it easier to distinguish between components that are pages and components that are reusable.
3. Are all routes configured correctly (including protected routes)?
- No, The ProtectedRoute component is missing the path prop in the Route component.
const ProtectedRoute = ({ component: Component, ...props }) => {
return (
<Route exact path={props.path}>
{() =>
props.loggedIn ? <Component {...props} /> : <Redirect to="./signin" />
}
</Route>
);
};
4. Are the necessary manipulations with the JWT implemented correctly in App.js?
The JWT token should be stored in the localstorage after a successful signin.
The onSignOut function doesn't remove the JWT from localStorage. It should be:
function onSignOut() {
localStorage.removeItem("jwt");
setIsLoggedIn(false);
setEmail("");
history.push("/signin");
}
other improvements can be made to the onLogin
function to include error handling when the token is not returned from the API and using the toast or modal to display the error message.
Security consideration - The JWT token should be stored in an HttpOnly cookie to prevent XSS attacks and token should be refreshed periodically to avoid unauthorized access to the application. Token should expire after a certain period of time.
Loading states with a skeleton or spineer during the token verification process can be added to improve the user experience.
1. Do all requests get the correct responses?
- No, the
deleteCard
function in thecards
controller is not implemented correctly. The function should be:
const deleteCard = (req, res) => {
const { id } = req.params;
Card.findById(id)
.orFail(() => {
const error = new Error('Card not found');
error.statusCode = 404;
throw error;
})
.then((card) => Card.deleteOne(card)
.then(() => res.send({ data: card })))
.catch((err) => {
if (err.kind === 'ObjectId') {
res.status(400).send({ message: 'ID is not valid' });
} else if (err.statusCode === 404) {
res.status(404).send({ message: err.message });
} else {
res.status(500).send({ message: 'Error occurred' });
}
});
};
Replacing the Card.updateOne with Card.deleteOne and adding proper error handling for the delete operation.
2. Do these requests correspond to REST API principles?
Yes, Some of the basic REST principles are followed like
- The resource name is pluralized -
cards
- The HTTP methods are used correctly - GET (/cards, /cards/:id), POST (/cards), DELETE (/cards/:id)
- Clean URLs without verbs
- The response structure is consistent, success response includes a
data
key and error response includes amessage
property.
but there are several improvements needed to make the API more RESTful:
- Fix status codes for validation errors on
createCard
the response code for ValidationError should be 400 Bad Request instead of 404
Another significant improvement to routes using request parameters is to validate the id parameter to ensure it is a valid ObjectId before querying the database.
if (!mongoose.Types.ObjectId.isValid(id)) {
return res.status(400).send({ message: 'Invalid card ID' });
}
- Fix status code for the
deleteCard
function, the response code for a successful delete operation should be 204 No Content instead of 200 OK - Add missing CRUD operations (PUT, PATCH)
- Add pagination for
getCards
GET /cards endpoints
const getCards = (req, res) => {
const { page = 1, limit = 10 } = req.query;
Card.find({})
.limit(limit * 1)
.skip((page - 1) * limit)
.then((cards) => {
res.send({
data: cards,
meta: {
page: parseInt(page),
limit: parseInt(limit)
}
});
})
.catch(() => res.status(500).send({ message: 'Error occurred' }));
};
5. Implement proper error handling
```js
const errorHandler = (err, req, res, next) => {
const status = err.statusCode || 500;
const message = err.message || 'Internal server error';
res.status(status).send({ message });
};
app.use(errorHandler);
3. Is the incoming data correctly validated? The Validator.js library is used for validation. What can be removed or changed in validation if reviewers only require the URL validation for the image link?
Yes the incoming data is validated using the validator
library.
const mongoose = require('mongoose');
const validator = require('validator');
const cardsSchema = new mongoose.Schema({
name: {
type: String,
required: [true, 'The name field must be filled'],
minlength: [2, 'Min length is 2'],
maxlength: [30, 'Max length is 30'],
},
image: {
type: String,
required: [true, 'The image field must be filled'],
validate: {
validator: (v) => validator.isURL(v, { protocols: ['http', 'https'], require_protocol: true }),
message: 'Must be a valid URL',
},
},
createdAt: {
type: Date,
default: Date.now,
},
}, { versionKey: false });
module.exports = mongoose.model('card', cardsSchema);
4. Is it possible to add or remove unintended data to database? (I am not sure what you mean by unintended data to database, this is the best way I could interpret the question.)
Yes, with mongoose we can specify add or remove data that we do not want the database to return using the select
method. This can be used to exclude sensitive data like passwords or other fields that should not be returned in the response.
Card.findById(id)
.select('name') // only return the name field
Card.findById(id)
.select('-password') // exclude the password field
1. Is this markup semantically correct? Are there tags that are used incorrectly?
The markup is mostly semantically correct, but there are some issues:
- Heading element arrangement inconsistencies
The document starts with an h1
heading "What's going on in the world?" but then jumps to h3
for "Search Results" and "About author"
There are inconsistent jumps between h4
and h5
headings in the preloader section
Heading levels should follow a logical hierarchy (h1
→ h2
→ h3
→ h4
→ h5
)
-
Incorrect Use of Semantic Elements The
section
elements are used without proper headings in some cases The preloader section on line 42 doesn't have a heading The modal content should probably usedialog
instead ofsection
for better semantics -
Navigation Issues The menu structure should use
nav
instead of just<div class="menu">
since it's the main navigation The footer menu should also usenav
instead of just aul
. -
Article Content The news cards should use
article
tags instead ofa
tags since they represent self-contained content, while thea
tags could then be nested inside the articles for proper linking.
<article class="card">
<a href="#" class="card__link">
<div class="card__body">
<!-- card content -->
</div>
</a>
</article>
-
Form Labels The forms lack proper
label
elements for form controls Usingspan
instead oflabel
is not semantic -
Button vs Link Some elements using the "button" class which function actually as a links and vice versa.
2. Are there mistakes in the names, in the usage of the blocks, elements, and modifiers?
Inconsistent Element Delimiters - Some blocks use single underscore while others use double underscores for elements. title here is not a modifier, it's an element of the card block.
.card_title {
/* styles */
}
@media screen and (max-width: 1023px) {
.card_title {
/* styles */
}
}
@media screen and (max-width: 767px) {
.card_title {
/* styles */
}
}
According to the BEM methodology, card__title
should be used consistently for all elements within the card
block.
Avoiding mixing of BEM and non-BEM classes, adhering to the BEM convention consistently throughout the codebase.
card__body (✅) card_date (❌) - should be card__date card_text (❌) - should be card__text card_title (❌) - should be card__title card_src (❌) - should be card__src card_wrapper (❌) - should be card__wrapper
CSS Nesting according to the BEM methodology allows using nested selectors, but we recommend minimizing their use. Nested selectors increase code coupling and make reuse impossible.
Inside the auth-form__error-message_usr-exists
block,
.auth-form__error-message.auth-form__error-message_usr-exists {
text-align: center;
display: inline-block;
position: relative;
top: 10px;
}
Block names should always use kebab-case, if more than one word is needed. auth-form
, save-title
. etc.
CSS inside of the wrapper.css file should use the BEM methodology
Avoid too many modifiers
3. Are there mistakes in the BEM file structure? Explain in detail.
The BEM file structure is not being followed consistently. The BEM methodology is a naming convention that helps developers better understand the relationship between the HTML and CSS in a project. It stands for Block, Element, Modifier.
For Example:
menu/ (block)
__item/ (element)
_state/ (modifier)
menu__item_state_current.css
menu__item.css
menu.css
menu.js
Based on the CSS given in this task, the BEM file structure should be as follows:
card/ (block)
_title/ (element)
card_title.css
__body/ (element)
card__body.css
_date/ (element)
card__date.css
_text/ (element)
card_text.css
_src/ (element)
card_src.css
_wrapper/ (element)
card_wrapper.css
card.css
card.js
should be structured as follows:
card/ (block)
__title/ (element) Corrected
card__title.css
__body/ (element)
card__body.css
__date/ (element) Corrected
card__date.css
__text/ (element) Corrected
card__text.css
__src/ (element) Corrected
card__src.css
__wrapper/ (element)
card__wrapper.css
card.css
card.js
The case for the _white
in the blocks/header could be considered a modifier, but it's not consistent with the BEM naming convention. It should be header_white.css
or header--white.css
if it's a modifier.
Combining the Flat and Nested Structure is called Flex in the folder structuring convention. The CSS in the header_white.css
modifies the header
block to have a white background and gives a min height.
4. Analyze the adaptive design and comment on the decisions that were made in this code.
I do not have so much knowledge about the adaptive design, but I can give some general feedback.
- The design is responsive and adapts to different screen sizes using media queries. I noticed the Breakpoint Strategy The codebase consistently uses three main breakpoints: Desktop: > 1023px (default) Tablet: ≤ 1023px Mobile: ≤ 767px Extra small: ≤ 374px (used occasionally) This is a great decision that covers major device categories.
Responsive Grid System for the new layout. In desktop view, it uses the 3 columns, in tablet view, it uses 3 columns with reduced gap and margins, and in mobile view, it uses 1 column. Maintaining good readability and usability across different screen sizes.
Fluid Typography is noticable in the design i.e the typograpgy scales down proportionally considering the .header__title
with font-size from 3.75rem to 2.25rem.
Paddings and Margins are adjusted according to the screen size. For example, the .card
.client
block has different padding values for different screen sizes.
.client {
padding: 80px 104px 72px 104px; /* Desktop */
padding: 30px 40px 40px 40px; /* Tablet */
padding: 20px 16px 35px 16px; /* Mobile */
}
Mobile first design decisions are obvious in the codebase.
Layout shift are handled well in the design. Consider the .footer
Desktop: Single row flex layout Mobile: Wrapped layout with adjusted heights and spacing
Image sizing is adjusted according to the screen size. For example, the .card_img
block has different height values for different screen sizes.
Desktop: 272px height Tablet: 150px height Mobile: 196px height
Mobile menu is utilized by converting from horizontal to veritical navigation.