Barv11• 30
@Barv11
Submitted
I will appreciate uf i could get feedbacks.
Yuliya
@Ambrolla
All comments
- Yuliya• 230
@Ambrolla
Posted
Hi!
Good job! However, I notice couple of things.
- Your second line from the bottom is a little off from where it should be (more on the top, probably try to make font-size smaller). Second line where "0.041 ETH" and "3 days left". Both are a bit off.
- Don't forget about code refactoring :)
But you did a very good job overall!
1 - Isaac C Anyim• 210
@CreatorLZ
Submitted
I see all solutions on the hub having scrollbars . I tried to eliminate that from this solution. Hope it doesn't look too cramped?
Yuliya• 230@Ambrolla
Posted
Hello!
First, I wanted to say how much I love love how you did mobile toggle version! It's so interesting and different from what I've been stuck on lol However, second, in the desktop version, I suggest to add margin and padding to your navbar + login and register buttons are too far away from each other.
Marked as helpful
0 - Adam• 60
@Vorczu
Submitted
- Dont know a thing about gradient bg, I dont know is that a good option or should I do it different
- Rating detector - its looking simple but maybe an event listener will be a better option ?
Yuliya• 230@Ambrolla
Posted
Hello Adam!
First, good job on your code and for trying! Second, I try to do my best following directions (answering your question about color gradient). So, yes, I think it is important. :) Third, there are some files in your github repo that I would include in the .gitignore. I recommend you reading about it! For example, you don't need to include style-guide :) And I'm a bit confused what is .css.map is, but I'll google it later.
Fourth, I would suggest instead of this: <div id="1" class="number" onclick="ratCilcked(this.id)">1</div> <div id="2" class="number" onclick="ratCilcked(this.id)">2</div> <div id="3" class="number" onclick="ratCilcked(this.id)">3</div> <div id="4" class="number" onclick="ratCilcked(this.id)">4</div> <div id="5" class="number" onclick="ratCilcked(this.id)">5</div>
Use event-target and event listener.
1 - imad• 3,310
@imadbg01
Submitted
I'm not sure if I get
errormessage correct, I need more work on Js, please feel free to give your feedback!!Yuliya• 230@Ambrolla
Posted
Hello!
Answering your question about the behavior. Right when I press "submit", I only get 1 error. But when I click on the additional field, then I get an error in the fields where the info is missing. That's the only issue that you have :)
- Looking at code. I'm not familiar with gitlab, maybe they have something like gitignore, but I was curious why do you have assets folder?
- Don't forget to delete console.logs and your notes that aren't needed later from code :) Basically clean up and refactor code.
- Also, the hover/design is a bit different from the original :) For example, the button.
Marked as helpful
0 - franklin okika• 40
@franklinscripts
Submitted
The difficult part building this project was the API part, because I literally knew nothing about API so I had to learn it step-by-step to implement it
I am not too sure I understand what's going on but I did it 🚀
Yuliya• 230@Ambrolla
Posted
Hello Franklin!
Good job on functionality, but I agree with Dimitar about the design. Also, if you don't know about the .gitignore, I recommend researching about it.
Also, style and code I suggest writing in the separate .css and .js file, if there are bigger projects, it's going to be confusing if you have everything in one file.
0 - Karina Estaba• 460
@karinaestaba
Submitted
Yuliya• 230@Ambrolla
Posted
Hello!
I've noticed couple of things, that probably needs to be fixed:
- Last name has specific amount of characters. However, some last names only have 2 letters.
- When I refresh the page, put in the html version in the input field "autocomplete: false" and it wouldn't let me see what I've already put when I refresh the page.
But you have a good code! Keep it up! Good job!
Marked as helpful
0 - Ibenyenwa Charles• 330
@zubyCharles
Submitted
The project was a really simple one though, took a couple of hours. I finally did get to use the browser
fetchmethod in a real site though and enjoyed chaining multiplethenmethods.Yuliya• 230@Ambrolla
Posted
Hi!
I've noticed some things, which will look better if you'll change them.
- In the quote, you forgot to add quotes.
- Also, I'd change the text color to plain white.
But you did a good job! I wanted to go with your method at first too, but then I went with asynch, await function, which I've heard is better.
Marked as helpful
1 - Thainá Fonseca• 70
@thaicamarelli
Submitted
i had a little difficult with JS because i'm starting programming, so i'd love feedbacks and details to improve thank you
Yuliya• 230@Ambrolla
Posted
Hi!
I've noticed multiple issues.
- In the github, there are some irrelevant files like how design should look like, you don't have to upload it and you can skip it using gitignore.
- When I push rating, I can push multiple ratings and it is as if I've picked all numbers, not just one.
- In javascript, using variable var isn't as good.
But overall, for the first js project, you did very good! Good job!
Marked as helpful
0 - Michael• 300
@Mfried95
Submitted
Let me know what I can Improve on :)
Yuliya• 230@Ambrolla
Posted
Hi!
Good job!
Some improvements can be made here:
- You can add margin (around 200-300px) at the bottom. Also, I didn't see any attributions.
- In html, you can make autofill false so that when you refresh, it will delete what you wrote in the input.
- When some of the fields are not filled out, there are no red letters + flag on the bottom of the input (error messages), you only have code (alert).
- In github, I saw that you've included actual design folder (how it should look like), you could just gitignore it :)
- Was confused by index.info folder on github as well.
1 - Daniel• 140
@DanielExe78
Submitted
Yuliya• 230@Ambrolla
Posted
Hi!
Good job!
However, there are a few comments:
- I suggest making footer notes and div at least with 30px margin apart from each other because they are chunked close together.
- There was also a flag or some icon in the challenge, which you had to put into the input fields when there was an error. I suggest you to put it :)
- When you refresh a page, I suggest you to turn off autocomplete (it's done in html input field)
- It doesn't check in the email input if it's an email or just random words.
Marked as helpful
1 - Reetesh Kumar• 550
@Virous77
Submitted
Yuliya• 230@Ambrolla
Posted
Hello!
As of right now, I have couple of issues (without looking at your code): When I click on the number, it doesn't show what number I've picked (clicked) unless I click "submit". It needs to be fixed, I think :) And I suggest making a circle background where there is a star, a bit darker, it'll make an eye flow better.
0 - Victor Gallo• 20
@vgallop
Submitted
I would appreciate some feedback on how to improve the cleanness of the code, for both HTML and CSS.
Yuliya• 230@Ambrolla
Posted
Hello!
The only big issue in css file I found is this: In .card you have: margin: auto; margin-top: 40px; Maybe pick one? :)
However, in order to keep everything more organized, I write comments (if it's more than 1 thing, for example for @media), I write that it's going to be media or Card images, etc. There are no comments in your code in CSS and I really recommend it because if you'll go back in 3-6 months and start looking at it, you might find yourself lost (especially if it's a bigger piece of code).
You did a good job, considering that it's your second repository! Keep going! And I'm serious!
Marked as helpful
0 - Udochukwu Amaefule• 570
@UDsGitHub
Submitted
Any feedback would be recommended
Yuliya• 230@Ambrolla
Posted
Hello!
You did a good job overall and with your project, however I wanted to point out to couple of things that you might have forgotten or left out:
- In the "submit" button, the color is off.
- you forgot to include "hover" state for rating + for the submit button.
But I Liked your JavaScript code!
Marked as helpful
0
