Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found

All comments

  • MasterKrab 940

    @MasterKrab

    Submitted

    I am just a beginner and would appreciate any feedback.

    Do I have good practices? What errors do I have? How could I improve my code?

    Thanks <3

    @iamjmitch

    Posted

    Hi Masterkrab,

    Some nice work here but can I ask what lead you to choose using createElement and appending it to the page? Just seems like a really over complicated way of implementing that error message.

    0
  • @iamjmitch

    Posted

    Hey mate nice looking website. Really nice folder structure and good use of components.

    I see in your section2 component, you just have react render the card component 3 times however as the info inside the card is hardcoded, it displays the same reviewer and comment 3 times.

    If you were to change the card to accept props, you could still have the same card component for all 3 however you could pass it seperate info for the img, h4 and p tags and have it render as 3 seperate reviews like in the design guide.

    1
  • @iamjmitch

    Posted

    Hey mate looks good. Code is nice and clean.

    One suggestion though is you have CSS in both an external stylesheet and internally inside your HTML file. While this isn't a big issue on smaller projects like this, as your projects get bigger, it will make your coding experience easier if you pick one universal way of maintaining your CSS

    1
  • @iamjmitch

    Posted

    Hi Luca,

    Really nice work on this, looks really good.

    Nothing wrong with your code, I would suggest though that you only us one H1 tag on the page. You've currently got 3 so I would suggest using a H2 and H3 aswell. If all the headings are H1 then none of them are really H1 if you know what I mean. Keeping it won't effect anything as such but it is technically semantically incorrect

    0
  • @iamjmitch

    Posted

    Hi,

    Interesting use of the hr tag there to make the dividing line. Would have never thought of it.

    With your code, you've used the h1 tag alot where not necessarily needed. Generally you only want to use 1 h1 tag per page or section

    0
  • @lamaolo

    Submitted

    I need to do it responsive, but first I should refactorize the code. Definitely it would have been a better idea do it mobile-first.

    @iamjmitch

    Posted

    Really nice animation on this!

    Just a thought with the different tokens when it comes to refactoring. all 3 are essentially the same thing just with different images and colors. You could potentially just create one token component and pass it in the img url and colors as props. This would essentially turn 3 separate components into one.

    0
  • @iamjmitch

    Posted

    Nice work so far!

    couple of suggestions with the "latest articles" cards. Have a play with the box-shadow on them and see if you can make them a bit softer. I like that they scale on hover but I think it would take it to the next level if you added some sort of transition in your CSS for them

    2
  • Al11o 180

    @Al11o

    Submitted

    I don't succeed the size of the card

    @iamjmitch

    Posted

    Hi, You're almost there with your CSS. To fix you issue of the gap at the bottom, try the following:

    Remove .card #content{ top: -52.5px; }

    Add .card #content img { margin-top: -50px; }

    1
  • Tommy 10

    @Tommy394

    Submitted

    Hi! I'm still confused when is right time to use px, em, or rem, so I'm mostly use rem for the length value, is that the right thing to do?.

    And any suggestion on the other CSS technologies that I can use for this challenge?

    I'm new to the front end development, I don't know if this is the best approach to this challenge, so a feedback would meant a lot to me.

    Thanks!

    @iamjmitch

    Posted

    Hi, in terms of when to use rem vs em vs px vs %, for your own personal projects, use what ever you're comfortable with. In a professional environment, have a read of this article https://engageinteractive.co.uk/blog/em-vs-rem-vs-px

    With the CSS there's nothing wrong with using SASS or something similar. How youve done it is fine as the CSS required is quite short however once you get to bigger projects you'll make your coding faster by using some sort of CSS pre-processor

    1
  • @iamjmitch

    Posted

    Nice work. I am genuinely curious how you came to name one of your classes "poop" though

    0
  • @iamjmitch

    Posted

    I think you have used grid quite effectively and its actually quite a nice approach to solving the task. Well done

    2
  • @iamjmitch

    Posted

    Nice looking work mate, well done. Really like how you've made it so your classes are reusable, that's a good practice to keep up.

    Have a look on mobile though, your attribution statement is randomly halfway down the screen and not at the bottom as intended. Fox that and you'll be golden

    0
  • P
    Katrien S 1,070

    @graficdoctor

    Submitted

    I'm just completely lost in the javascript-form validation. And I have no idea how to clear the input. I've tried .reset(), I tried resetting the values with (something similar to) email[0].value = ''. Would anyone know a good source for more information? I tried a zillion YouTube-video's, but noone resets the value and I think that is why I'm running into errors?

    As for the css & html: this was fairly logic and no questions here really.

    @iamjmitch

    Posted

    Hi there,

    Ill do my best to try and explain some stuff

    To answer your reset question, try form.reset(). this worked for me.

    the other issue you are having is that the JS is not actually capturing the value of the input. const email = document.getElementById('email').value.trim() is running at page load, not when you hit the notify button so the inputted email address is never actually being captured and passed to the checkInputs() function for validation. Try declaring email on load const email = document.getElementById('email') and then inside your eventListener have something that looks like

    form.addEventListener('submit', (e) => {
      e.preventDefault();
     var inputtedValue = email.value.trim();
      checkInputs(inputtedValue);
    })
    

    this will make sure that the most up to date value is always passed to your check function when the user hits the notify button

    Then with your actual check function, if(!email) is checking to see if the variable doesn't exist which, as you are declaring it on page load, email will never not exist and the first block of code will never run. What you want to do is check whether that value is an empty string or ""

      function checkInputs(inputtedValue) {
      if (inputtedValue  === "") {
        message.classList.remove('valid')
        message.classList.add('invalid')
        message.innerText = 'You forgot to enter your address'
        message.style.display = 'inline-block'
      } else if (inputtedValue.match(emailPattern)) {
        message.classList.add('valid')
        message.classList.remove('invalid')
        message.innerText = 'Your email address is valid'
        message.style.display = 'inline-block'
      } else {
        message.classList.remove('valid')
        message.classList.add('invalid')
        message.innerText = 'Oops, something went wrong'
        message.style.display = 'inline-block'
      }
    }
    

    Have a look at the above modified function and see if you can work out whats going on. All the above put together should result in the functionality that your after. Hopefully this somewhat explains some stuff. Please let me know if you still dont understand and I can see how I can help

    1
  • juanma1331 110

    @juanma1331

    Submitted

    I was unable to solve the next. When decreasing viewport from desktop reviews text takes more vertical space breaking the reviews layout.

    @iamjmitch

    Posted

    Hi mate,

    Not 100% sure what your referring to. Loaded the site in mobile and it looks pretty spot on to the design. If you can elaborate some more id be happy to give advice

    1
  • @iamjmitch

    Posted

    Hello there

    Really nice work. Just a suggestion, I would change the following in your body tag in the CSS

    Remove

     height: 120vh
    

    Add

     min-height: 100vh;
     display: flex;
     flex-direction: column;
     justify-content: center;
     align-items: center;
    

    This will help center the content on the screen and remove the white space at the bottom of the page

    0
  • @kofinartey

    Submitted

    Hi everyone. This is my first published project since I begun learning to code. I'd appreciate it if anyone goes through my solution and suggest areas I could improve upon. Super happy to learn from y'all.

    @iamjmitch

    Posted

    Nice work. Good job on commenting your code!

    Only suggestion I have is maybe run the code though a HTML beautifier or install the plugin prettier just to clean up the white space. Definitely not a must do, nothing wrong with your code, just helps clean it up a bit visually

    1
  • @iamjmitch

    Posted

    In answer of your question: when do you use: px, em/rem, %?

    Best practice is supposedly to use rem but for your own personal projects, use what ever you feel most comfortable with.

    Personally I use px and % for container tags and px for text.

    0
  • @Cats-n-coffee

    Submitted

    Hi everyone! I did this challenge using plain HTML, JS and organized the styles with SCSS. I did a little bit of research/learning on how to scale SVGs and use them the proper way so they behave as expected, let me know if I should've done things differently. I believe accessibility could be a lot better, let me know what I should change first. The dynamic result list is inside a ul tag, so all rows are li tags with span tags inside (does it make sense?). Any feedback on anything is greatly appreciated, thank you!

    @iamjmitch

    Posted

    Nice looking website. Only thing I would suggest is to add the max-width of 1440px from the style guide as when viewing the site on a 2k monitor, everything is really spread out and far apart

    0
  • @iamjmitch

    Posted

    I've checked both resolutions for you and everything scales really nicely.

    One issue I noticed is that your social media sprites don't load on chrome on my machine. Sprites load perfectly on Firefox and MS Edge

    1
  • @iamjmitch

    Posted

    Really clean work, I like it.

    Small suggestion is you've jumped from a H1 tag to a H3 tag. Normally, you don't really want to jump like that. I would suggest changing your very first H3 tag into a H2 tag to follow correct heading hierarchy. Just remember to change in your CSS too!

    0
  • @iamjmitch

    Posted

    Nice work!

    Having a look at your code, I've noticed you use br tags a lot to vertically separate your span tags. While you can do this, I'd recommend playing around with using padding/margin in your CSS to create this gap as you will be able to customize the spacing a lot easier. You might have to set your span tags to width:100% for it to push the next item onto the next line.

    2
  • @iamjmitch

    Posted

    I cant seem to load your preview site from above however going purely off the comparison images i'd suggest vertically aligning the left side of the heading, subheading and the first card

    0
  • Cristian 100

    @CrisCaGoVe

    Submitted

    Hello everyone, i'd like to recieve all kinds of advices and recommendations to improve my coding, thanks!

    @iamjmitch

    Posted

    This is really good! Nice clean code, well done!

    0
  • @BlakeDykes

    Submitted

    Learned a lot doing this, and proud of how this turned out! I'd love any constructive feedback/comments.

    @iamjmitch

    Posted

    This is really cool man! Smooth animations.

    Small suggestion I'd make is have the "show all" button have a slide in animation aswell after the last image

    0