Skip to content
OverviewSolutions
40
Comments
57
P

Katrien Schuermans

@katrien-sBelgium1,400 points

Transitioning into Front-End Development after a successful career as a Senior Graphic Designer with nearly 20 years in print-centric industries.

I’m currently learning...

CSS/HTML/JavaScript and React

Latest solutions

View more solutions

Latest comments

  • Miguel Bruce•200
    @FMB237
    What are you most proud of, and what would you do differently next time?

    I'm proud i learn how to manipulate multiple DOM elements

    What challenges did you encounter, and how did you overcome them?

    using Multiple elements

    FAQ-accordion

    1
    P
    Katrien Schuermans•1,400
    @katrien-s

    Something I just want to point out: what's up with the capital letters in your css and javascript?

    The convention is:

    • in javaScript: camelCase your words. Which means, first letter is not a capital
    • in css: no capital letters

    It makes your code look messy.

    There also seems to be double code. You can remove (min-width: 48rem):

    @media (min-width: 40rem) {
    .Faq-Accodation {
        height: 70vh;
        width: 70%;
    }
    .answers p {
        font-size: 1rem;
    }
}

@media (min-width: 48rem) {
    .Faq-Accodation {
        height: 70vh;
        width: 70%;
    } 
    .answers p {
        font-size: 1rem;
    }
}

    Also, in all your media-queries, the .answers p gets the same font-size. It'll work if you would only add it to (min-width: 40rem) as it will be used for all screen-sizes with a minimum-width of 40 rem.

    .answers p{
    font-size: 1rem;
}

    Aside from this, your exercise works and looks good. So I'd say just keep on coding though pay a bit more attention to how you write it. Happy coding!

  • P
    Katrien Schuermans•1,400
    @katrien-s

    Hi, it overall looks good but there's some minor issues.

    I can click on all buttons, have them all highlighted before I click on submit. You should limit this to have only one button highlighted.

    It's advised to combine all css in one file. There's no need to split it up. The same for your javascript. You can have it all in one file.

    There is also no need to set a width on your body. It always takes by default 100vw.

    Why did you use !important so often? Check also if by adding all your css to seperate classes, you maybe wrote too much css.

    For example: .icon-wrapper and .thanks-icon-wrapper seem to have the same flex-styling which you could maybe put on section instead of adding the same code to 2 classes, that have the same use/outcome.

    But great job on this one. The Javascript works well, and it does what it needs to do. Have a look at the things I pointed out, and keep coding.

  • P
    Kiara•220
    @Kiara523
    What are you most proud of, and what would you do differently next time?

    It was fun making it. I think I can do better with class names, I want to improve on that. Also there are different ways to approach this challenge.

    What challenges did you encounter, and how did you overcome them?

    I didn't encounter major challenges, but I've learned more about grid.

    What specific areas of your project would you like help with?

    I want to improve naming classes in html. I think that a good markup helps a lot.

    time-tracking-dashboard HTML CSS Javascript

    1
    P
    Katrien Schuermans•1,400
    @katrien-s

    Not much to add as this looks good. Though, watch out for typing errors. You managed to spell social in various ways.

    Having myself also just finished the exercise it shows pretty well how different JavaScript-approaches lead to the same result.

    Happy coding!

    Marked as helpful
  • P
    Katrien Schuermans•1,400
    @katrien-s

    Not that much to add actually. Your solution looks really good. Are you going to continue writing your js and css inside your html?

    As for your image, look into how to use <picture> to have responsive images instead of showing and hiding mobile and desktop image.

    The padding on your form-container is not equal on both sides. You have set a padding-inline: 3rem 2rem which really shows on mobile view. But it also gives too much white space on desktop view.

    There's something really strange here. When I look at your code on Github and the code of your webpage, I see 2 different solutions. Your webpage does not have inline styles and JS. It shows the too much padding on 'form-container' while on Github it says the padding is on detail. Are you copying in other people's webpages? If so, I stop giving feedback here as I am not looking at your solution but someone else's.

  • P
    Katrien Schuermans•1,400
    @katrien-s

    Your solution is like looking at a squeezed version of the exercise. There's a few things that I immediately to be odd.

    • Don't set a width on body. Your body-element is that entire visible in your browser. That always needs to be a full 100%, which it is by default. And which you don't change. In this exercise you did it obviously to have the component have the right width. But, that component is in your main-element. Guess what is ok to do with the main-element... If you want to change the width do it here.
    • use width on your img instead of max-inline-size, and add an object-fit to have your img fit nicely. Read the article on MDN:
    img {
  width: 100%;
  height: 100%;
  object-fit: cover;
}

    Did you have the Figma-file to work with? As your typography for your h4 element seems a bit off. Besides, having an h5 on the author-name isn't really correct. You could write this as a p, but add a bold-class to it.

    This is a tough exercise. I didn't read through your javascript because it actually looks really complicated while this is a beginners-exercise. So I'm wondering why your JS-solution looks this complicated. Why are you not using the camelCase naming convention?

    If you pay a bit more attention to the design I think you're well on your way to be a good developer. Keep on coding and more importantly, enjoy it :D.

    Marked as helpful
  • P
    SKszymek•180
    @SKszymek
    What are you most proud of, and what would you do differently next time?

    I'm proud of my self that I end up with that result. Next time I have to learn more about responsive design. I make It but I'm felling not comfortable with It.

    What specific areas of your project would you like help with?

    That's my first landing page so I would be gratefull if somebody check my code. I need help with queries and withds. To be honest I don't know when exacly use percentages and when max-width. Thank's for any advices.

    Resopnsive meet landing page using css grid/flexbox

    1
    P
    Katrien Schuermans•1,400
    @katrien-s

    Hi, you nailed it! There's some little things here and there, but overall looks good. Also your HTML reads very well organized. Same for your CSS.

    Small error in your organisation of HTML could be that you entered your header and footer in main. It's actually possible and sometimes done. But, for HTML to be semantic and in terms of accessibility, you also need to have a header and a footer outside of main. A basic HTML-structure looks like

      <header></header>
  <main></main>
  <footer></footer>

    As for your question about responsiveness. I do see looking at your code you're a bit lost in all the information. First of all, whether you use max-width or min-width depends on whether you style starting mobile first of desktop first. You've been using a lot of media-widths, which is not that necessary. You can get by by using 2 or 3 settings.

    You've been indeed using percentages a lot. I did that too when just starting out, but it's again not always necessary. You actually don't need to declare a width that often.

    Kevin Powell has this awesome course on Responsive Layouts. This is going to help you a lot.

    Marked as helpful
View more comments