-
Notifications
You must be signed in to change notification settings - Fork 0
Project 9 #1
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
base: main
Are you sure you want to change the base?
Conversation
feat: Add a webpack to run the project localy; fix: Update the directory; feat: Add required packages.
feat: Loading cards from the server; feat: Editing the profile text content; feat: Adding a new card; feat: Creating a popup for deleting a card; feat: Deleting a card; feat: Adding and removing likes; feat: Updating the profile picture; feat: Improving the UX of all forms.
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 15937127 | Triggered | Generic High Entropy Secret | 7a9c4d9 | src/pages/index.js | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secret safely. Learn here the best practices.
- Revoke and rotate this secret.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Please, expand the general comment ↓)
Dear Pavel,
Unfortunately, I am unable to complete the review at this time because:
- When I add a new card, the page reloads, but it should not happen because it's a huge waste of resources on refetching the whole data, rerendering and so on
- The
cancelbutton doesn't work https://snipboard.io/YmkLdK.jpg - When I delete cards, the page reloads, but it should not happen
- According to the task, you need to change submit buttons text (
Saving…) while waiting for the server response. And the user should see it. - Now all your cards (which you've tried to delete, but changed your mind) are deleted all at once. Please, try it yourself: add 3 cards in a row, then try to delete them, but don't do it (cancel and close the confirmation popup), and then delete any other card - and all your previous cards are deleted at once (all 3 and the other one). It’s because of a memory leak. All listeners are gathered in one element and they are fired all together when you confirm deleting. You need either to correctly remove submit listeners or update the card data.
- You need to
disablethe submit button in thecard formif the inputs are empty. It’s needed because I can click the button and add an empty card, but it should not happen. After the first submission, the button gets activated, and you need to deactivate it again. Even if I can't add an empty card, the button should look disabled (gray) anyway and have adisabledattribute. - According to the previous task, you need to create a
resetValidationfunction for clearing validation errors from the inputs https://snipboard.io/gGfQPA.jpg You’ll need to call it when you open the profile modal. You shouldn't change the right code of the 6th sprint - According to the previous task, you need to reset the form and disable the save button in the
cardform after submission https://snipboard.io/fjmNYG.jpg You shouldn't change the right code of the 6th sprint
If you need any assistance in the meantime, our dedicated tutors are available to support you. Please don’t hesitate to reach out to them for guidance.
If your code is correct locally on your computer, then check if you have pushed all the changes to github. I can see those changes only if they are in your branch
fix: Cancel delete button; fix: Saving... button; fix: Disable submit button;
gennady-bars
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Please, expand the general comment ↓)
Dear Pavel,
Unfortunately, I am unable to complete the review at this time because some part of the task hasn't been implemented:
- Please, implement the project according to the task. There should not be
OOPexcept for theApiclass. I can't review the code that is not according to the task. You should continue your project 6 (SPOTS) by adding theapiintegration
I've noticed other mistakes:
- New cards should be added only with
prependto the start of the section, otherwise the user doesn’t see the new card (if there are 6 or more cards on the page) and may think that it hasn’t been created - According to the task, you need to change submit buttons text (
Saving…) while waiting for the server response. And the user should see it. - The
cancelbutton doesn't work https://snipboard.io/YmkLdK.jpg - According to the previous task, you need to create a
resetValidationfunction for clearing validation errors from the inputs https://snipboard.io/gGfQPA.jpg You’ll need to call it when you open the profile modal. You shouldn't change the right code of the 6th sprint
If you need any assistance in the meantime, our dedicated tutors are available to support you. Please don’t hesitate to reach out to them for guidance.
If your code is correct locally on your computer, then check if you have pushed all the changes to github. I can see those changes only if they are in your branch
fix: Cancel delete button; fix: Saving... button; fix: Disable submit button;
gennady-bars
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
↓Expand to see the whole summary↓
Hello, Pavel!
Great job on submitting your project 9! This time you’ve implemented Api integration. So you can create your own simple sites from now on!
Parts I really liked in your project:
- Good job that you
catchpossible errors at the end of server requests. - Good job that you return the button text in
finally - Nice
_fetchResponsemethod
There are few things that need to be corrected in your project. They're mostly minor issues that are easy to fix.
Needs correcting:
- All comments in the code and the comments below:
- When I delete 3 new cards in a row, there are errors https://snipboard.io/CMa94i.jpg
- When I reload the page, all my new cards which have just been deleted, appear on the page again
- According to the task, you need to change submit buttons text (
Saving…) while waiting for the server response. And the user should see it. - You need to close modals only in
thenblocks after a successful response from the server so that when there is an error, it would be opened (the inputs will not be cleared and the user could send the data again to the server), and if you change the button text, you could see the change as well. Please, correct it everywhere in the code - If the inputs are empty, the submt button should e disabled https://snipboard.io/wXBWME.jpg
- You need to reset the form only in a
thenblock in order not to do that immediately aftersubmitbecause when there is an error, the modal will still be opened, and the user can try to send the data again to the server without having to type the data into the inputs once more. - You need to disable the button only in a
thenblock after a successful response to be able to click it again if there is a server error. - You need to delete cards only in a
thenblock after a successful response from the server, so that if there is an error, the card should stay in theDOM. - You need to toggle likes only in a
thenblock after a successful response from the server because if there is an error, the user will not know about that and will see the toggled heart color.
There are some comments in your project that can help you make it even better. Please check the Could be improved comments.
A reminder: It’s necessary to fix all comments to get the project accepted. And Could be improved comments are optional, they will be up to you if you want to implement them.
Your hard work on this project is truly commendable. While there are still some adjustments needed. If you need any assistance with the feedback or understanding certain concepts, our tutors are here to help. Keep moving forward and maintain your dedication — you are doing an amazing job!✨
| this._headers = options.headers; | ||
| } | ||
|
|
||
| async _fetchResponse(endpoint, method, body) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice _fetchResponse method
src/pages/index.js
Outdated
| finally(() => { | ||
| clearInterval(loadingAnimation); | ||
| getSubmitButton(popup).textContent = originalText; | ||
| togglePopup(popup); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
togglePopup(popup);
You need to close modals only in then blocks after a successful response from the server so that when there is an error, it would be opened (the inputs will not be cleared and the user could send the data again to the server), and if you change the button text, you could see the change as well. Please, correct it everywhere in the code
src/pages/index.js
Outdated
| editAvatarForm.reset(); | ||
| disableButton(editAvatarForm); | ||
| togglePopup(popup); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These 3 lines of code should be in .then
- You need to close modals only in
thenblocks after a successful response from the server so that when there is an error, it would be opened (the inputs will not be cleared and the user could send the data again to the server), and if you change the button text, you could see the change as well. Please, correct it everywhere in the code - You need to reset the form only in a
thenblock in order not to do that immediately aftersubmitbecause when there is an error, the modal will still be opened, and the user can try to send the data again to the server without having to type the data into the inputs once more. - You need to disable the button only in a
thenblock after a successful response to be able to click it again if there is a server error.
src/pages/index.js
Outdated
| clearInterval(loadingAnimation); | ||
| getSubmitButton(deleteCardSelectors.deleteCardPopup).textContent = originalText; | ||
| togglePopup(deleteCardSelectors.deleteCardPopup); | ||
| card.remove(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You need to delete cards only in a then block after a successful response from the server, so that if there is an error, the card should stay in the DOM.
src/pages/index.js
Outdated
|
|
||
| cardElementLike.addEventListener("click", () => { | ||
| api.likeCard(data.id).catch(err => console.log(err)); | ||
| cardElementLike.classList.toggle("card__like-button_active"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You need to toggle likes only in a then block after a successful response from the server because if there is an error, the user will not know about that and will see the toggled heart color.
src/pages/index.js
Outdated
| profileForm.addEventListener("submit", (evt) => { | ||
| evt.preventDefault(); | ||
| const popup = profileSelectors.editPopup; | ||
| const { loadingAnimation, originalText } = submitButtonText(popup); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
COULD BE IMPROVED
If it’s interesting for you here is how we can make a universal function for handling any submit. We can get rid of such duplicating as loading effect, resetting and catching errors
// define a function for changing the button text. It accepts 4 params (the 2 last are optional with default texts)
export function renderLoading(isLoading, button, buttonText='Save', loadingText='Saving...') {
if (isLoading) {
button.textContent = loadingText
} else {
button.textContent = buttonText
}
}
// define a universal function that accepts a request function, event and a default loading text
function handleSubmit(request, evt, loadingText = 'Saving...') {
// You need to prevent the default action in any submit handler
evt.preventDefault();
// the button is always available inside `event` as `submitter`
const submitButton = evt.submitter;
// fix the initial button text
const initialText = submitButton.textContent;
// change the button text before requesting
renderLoading(true, submitButton, initialText, loadingText);
// call the request function to be able to use the promise chain
request()
.then(() => {
// any form should be reset after a successful response
// evt.target is the form in any submit handler
evt.target.reset();
})
// we need to catch possible errors
// console.error is used to handle errors if you don’t have any other ways for that
.catch(console.error)
// and in finally we need to stop loading
.finally(() => {
renderLoading(false, submitButton, initialText);
});
}Here is an example of handling the profile form submit:
function handleProfileFormSubmit(evt) {
// create a request function that returns a promise
function makeRequest() {
// `return` lets us use a promise chain `then, catch, finally`
return editProfile(nameInput.value, jobInput.value).then((userData) => {
userName.textContent = userData.name;
userJob.textContent = userData.about;
});
}
// here we call handleSubmit passing the request and event (if you want a different loading text then you need to pass the 3rd param)
handleSubmit(makeRequest, evt);
}So, this way you can remove a lot of code duplicating. You will not need to search buttons, pass initial button texts and so on.
handleSubmit and renderLoading should be placed in utils.js, because they are utility functions
fix: Cancel delete button; fix: Saving... button; fix: Disable submit button;
gennady-bars
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything is almost fine, but:
- According to the task, you need to change submit buttons text (
Saving…) while waiting for the server response. And the user should see it.
gennady-bars
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You’ve done a great job!
Your project has been accepted.
Good luck!
feat: Loading user information from the server;
feat: Loading cards from the server;
feat: Editing the profile text content;
feat: Adding a new card;
feat: Creating a popup for deleting a card;
feat: Deleting a card;
feat: Adding and removing likes;
feat: Updating the profile picture;
feat: Improving the UX of all forms.