@pastyenriqueSubmitted about 2 years ago
Hello I recently finished my first project Qr-code-component, I hope you will like, I would like any advices to be better and learn each day all new technics to be a better frontend developer.
Amobi Jonathan Chukwudi
@JonathanthedeveloperAll comments
- @JonathanthedeveloperPosted about 2 years ago
Hey Enrigue, Your challenge looks excellent, Kudos to that but, I noticed that you failed to add a closing HTML tag and with that, you may end up with unexpected issues with your webpage. However, excluding a closing tag will not cause error messages or crashes, so they technically are not an absolute requirement. But, this does not mean you should exclude them.
Marked as helpful0 - @welangaiericSubmitted about 2 years ago
there is a lot to be fixed. but i can say i tried
@JonathanthedeveloperPosted about 2 years agoHello Eric, your solution looks great except for the fact that it isn't interactive.
In case you're wondering what Event listener could be used you can use the
keyup event listenerhere's a sample solution code
// dom elements const inputs= document.querySelectorAll('input'); const outputs = document.querySelectorAll('output'); inputs.forEach(function(input, index){ input.addEventListener("keyup", function('keyup', function(){ if(validation code === true) { outputs[index].textcontent = input.value; } }) ) });Marked as helpful0 - @shalash23Submitted about 2 years ago
I would really any advice about refactoring the JavaScript code. Is there room for improvement here?
@JonathanthedeveloperPosted about 2 years agoHey shalash23, Your overall solution looks great.
However, you didn't save the total for each person, you are supposed to save the total of every calculation until the reset button is pressed please, do well to look into it.
0 - @loicsolbesSubmitted about 2 years ago
Hi ! I have just finish the challange and I have a few questions. Is this a responsive design ? Which parts of my code can I improve? Do I have a good use of em and rem ? Thank you for your help
@JonathanthedeveloperPosted about 2 years agoHello Soles, Whilst your solution looked great and responsive I've noticed that has an overflow-X maybe you could try changing your container's max-width to 80% or you could also try setting the body's overflow to hidden. Thank you
Marked as helpful0 - @davidicabelloSubmitted about 2 years ago
Any feedback welcome. I have to look for some stuff online to resolve this!. Thanks!
@JonathanthedeveloperPosted about 2 years agoHello David, Your Solution looks great, however, you should that the following into account
-
your
div.productDescriptionneeds some more padding 30px should be great -
your
button.addToCartButtonneeds some resizing as it's too high, and the top margin seems to be too high -
you should also add a line-height to your
<p>...</p>say 1.2 -
Then on mobile view the picture on the left seems to be distorted, maybe you could try changing the
<img>src on the mobile view to the image provided for mobile devices.
But overall your solution looks great
Marked as helpful1 -
- @KonstantinchristSubmitted about 2 years ago
Hi guys, I hope you're doing great. I just didn't know how to implement the errrors if you leave something blank. Maybe someone can tell me :). Thanks for the support!
Konstantin
@JonathanthedeveloperPosted about 2 years agoHello Konstatin, your solution to this challenge was great and awesome, very responsive and that's really cool, However Instead of putting the required keyword on the HTML input tags. you could use some form validation using javascript to verify that an input was filled and if not, you then display its dedicated error messages, also you should also add some box shadow to the div.special-offer and button attached below is a code that might help you understand what I mean
inputs.forEach( input => { // check if the input is empty })Marked as helpful0 - @SebastianLakaSubmitted about 2 years ago
Hi it's my first interactive project with JS. All is in repo at my github.
@JonathanthedeveloperPosted about 2 years agoHello Sebastian, You did excellent work on your rating component, however, it would be best if you validated that a button has been clicked before displaying the thank you message. I added a code below to help you with that
submitButton.addEventListener("click", () => { if (/* check if user has click on a rating*/ ){ secondCard.classList.remove("hidden"); firstCard.style.display = "none"; } else { // prompt user to click on a rating } });Marked as helpful0 - @NabilWasirSubmitted about 2 years ago
Don't forget to tell me if something is wrong in my code or if I can improve my code and give your opinion/feedback.
@JonathanthedeveloperPosted about 2 years agoHey Nabil, whilst your solution looked great, I however noticed that you used px and now your solution isn't fully responsive on some mobiles changing you width units to % percentages would go a long way
example you could do
.main-container { width: 60%; }0 - @marekbrzeSubmitted about 2 years ago
I would like to know if my layout decisions were good. Especially regarding margins and positioning of the elements.
@JonathanthedeveloperPosted about 2 years agoHey Marek, I noticed that you used a lot of divs . so you can refactor your code to
<div class="cardContent"> <h1>Improve your front-end skills by building projects</h1> <p> Scan the QR code to visit Frontend Mentor and take your coding skills to the next level </p> </div>instead of
<div class="cardContent"> <h1>Improve your front-end skills by building projects</h1> <div class="cardContent"> <p> Scan the QR code to visit Frontend Mentor and take your coding skills to the next level </p> </div> </div>You
Marked as helpful1 - @BennyBenVSubmitted about 2 years ago
Hello ! Here is my solution to this challenge ! I enjoyed doing this challenge and learn how to do responsive forms.
Feedbacks would be very helpful to me!
@JonathanthedeveloperPosted about 2 years agoBenjamin nice work you did there. however, I noticed your solution wasn't responsive even though you used flex on your div#main, try adding this to help improve responsivity
#main { flex-direction: row; } @media (max-width: $width ){ #main { flex-direction: column; /* add more styles for your solution*/ } }replace $width with the screen width you want to make it responsive at e.g 800px Thank you
Marked as helpful0 - @MicMond01Submitted about 2 years ago
This is my first API challenge. Kindly leave a comment in case there's an improvement to be made.
@JonathanthedeveloperPosted about 2 years agoNice work you did there Micheal. Notice how your div.container is small on mobile devices, make it have a total width of 90% of the view width, then try reducing the font size of your h3 of smaller devices, and lastly change the let keyword you used while targeting your dom elements in your script.js to const as the dom elements are static, the only thing that changes it their values. Thank you.
1 - @KaydenGiang2512Submitted over 2 years ago
Definitely one of the harder challenges I encountered on Frontend Mentor, especially with the difficulties of building a multi-layout navbar (since it's my first time), so if anyone has a more concise solution or know any useful knowledge on how to achieve the same outcome with a more efficient approach, please don't hesitate to leave a comment and help me out :D Thank you very much!
@JonathanthedeveloperPosted over 2 years agoI noticed that you picture was aligned to the top right, Which isn't supposed to be so.
I think this would solve it
img { width: #px; height: #px; position: absolute; left: #px; }
0
