Skip to content
OverviewSolutions
1
Comments
29
James Mitchell
@iamjmitch

All comments

  • MasterKrab•940
    @MasterKrabSubmitted almost 4 years ago

    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

    Base apparel | Html, css & Js

    1
    James Mitchell•295
    @iamjmitchPosted almost 4 years ago

    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
  • James Mitchell•295
    @iamjmitchPosted almost 4 years ago

    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
  • James Mitchell•295
    @iamjmitchPosted almost 4 years ago

    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
  • James Mitchell•295
    @iamjmitchPosted almost 4 years ago

    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
  • James Mitchell•295
    @iamjmitchPosted almost 4 years ago

    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
  • Lucero Amaolo•260
    @lamaoloSubmitted almost 4 years ago

    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.

    Using React & Hooks

    1
    James Mitchell•295
    @iamjmitchPosted almost 4 years ago

    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
  • James Mitchell•295
    @iamjmitchPosted almost 4 years ago

    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
    @Al11oSubmitted almost 4 years ago

    I don't succeed the size of the card

    Profile card

    1
    James Mitchell•295
    @iamjmitchPosted almost 4 years ago

    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
    @Tommy394Submitted almost 4 years ago

    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!

    Mobile First Four Card Solution | Using CSS Grid

    1
    James Mitchell•295
    @iamjmitchPosted almost 4 years ago

    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
  • James Mitchell•295
    @iamjmitchPosted almost 4 years ago

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

    0
  • James Mitchell•295
    @iamjmitchPosted almost 4 years ago

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

    2
  • James Mitchell•295
    @iamjmitchPosted almost 4 years ago

    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