All Comments

    • HTML
    • CSS

    Pure css and html done with atom text writer.

    2
    Ken455 | Posted 18 days agocommented on Thomas Smith's "Four card feature section" solution

    To get rid of accessibility issues as well, I would wrap the entire content starting from div class="header" in a main tag like you did for your footer

    For linking images, I would keep the files in the image folder that frontendmentor provides you. There's no need to extract it all in one place like you did in your github.

    In your Index.html I would then link images like so: <img src="images/icon-supervisor.svg">

    0
    • HTML
    • CSS

    Pure css and html done with atom text writer.

    2
    Ken455 | Posted 18 days agocommented on Thomas Smith's "Four card feature section" solution

    I believe having margin: 2rem on .card is one of the factors of your "phantom space"

    I also applied:

    .middle {
        ...
        ...
        display: flex;
        flex-direction: column;
        align-items: center;
    }
    

    which seems to perfectly space out your cards.

    I also recommend setting max-width with set values such as rem to keep your card sizing more consistent, and scaling it down as your screen gets smaller/bigger.

    Using % for the width on .card and .middle shrinks the width too much when scaling down.

    I hope this helps!

    0
    • HTML
    • CSS
    • JS

    Article preview component with Vanilla JS

    4
    Ken455 | Posted 18 days agocommented on Dusan Lukic's "Article preview component" solution

    Hey! Your design and project looks great.

    For your <button class="card__footer--btn"> I would add `aria-label="Name" to be more accessible. You can read more about that here

    Don't forget to generate a new report after making those changes.

    I hope this helps!

    1
    • HTML
    • CSS

    four-card-feature-section-master

    2
    Ken455 | Posted 18 days agocommented on Allan's "Four card feature section" solution

    Desktop and mobile layouts look great! I don't know if it was intentional but around 1150px the layout of the individual cards become a little strange.

    To fix some accessibility issues you have, I would make your <div class="container" to <main class="container"

    Heading tags should also be in chronological order (h1, h2, h3, etc) and you are missing an h2 in your document. I suggest you change the h3 into a p or find other text to turn into an h2.

    Since the icon images are decoration, I would put:

    <img src="./images/icon-team-builder.svg" alt="" aria-hidden="true">
    

    This is for screen readers, which should be taken into account for best practice. You can read more about aria-hidden here

    Don't forget to generate a new report !

    I hope this helps, keep it up!

    1
    • HTML
    • CSS

    profile card component, using : css, html

    4
    Ken455 | Posted 18 days agocommented on Saepul Malik (Malik)'s "Profile card component" solution

    The card sizing looks great!

    I suggest you add some padding to the left and right for card_footer to have the spacing closer.

    I would also change the font size and font weight of certain text and also align certain items center to more accurately match the original design.

    To clear the accessibility issues I would:

    • change your <div class="container"> to <main class="container>

    I would also do the same for the attribution as well by either:

    • changing the div to footer or wrapping the entire div with a footer tag.
    • Although this is supposed to be a small component of a larger document, I see that you have not used a heading tag in your document. Always have an h1 in your document that increases in chronological order.

    Don't forget to generate a new report after you make those changes!

    Since bg-pattern-bottom.svg and bg-pattern-top.svg are decorations, I would change your HTML to:

    <img src="./images/bg-pattern-bottom.svg" alt="" aria-hidden="true">
    <img src="./images/bg-pattern-top.svg" alt="" aria-hidden="true">
    

    This is for screen readers, which should be taken into account for best practice. You can read more about aria-hidden here

    I hope this helps! Keep it up :)

    1
    • HTML
    • CSS

    four-card-feature-section-master

    6
    Ken455 | Posted 19 days agocommented on omar farid's "Four card feature section" solution

    I would get rid of margin: 50px auto; styling on your main element. Instead I suggest doing:

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

    which will center your content.

    I would also give more of a gap between your containers in desktop view.

    Other than that, your mobile layout looks great! I hope this helps.

    0
    • HTML
    • CSS

    four card feature section made with html and css

    4
    Ken455 | Posted 19 days agocommented on Abdihafid Adan's "Four card feature section" solution

    Looks good at 1440px initially.

    I would add a breakpoint around 1300px and make a "tablet layout" as the containers get too squeezed together.

    I think you also forgot to make a mobile layout for this project at 375px.

    In fact, I would suggest using max-width with a set value (rem) for your containers. Using % is the reason why your containers stretch like that, and get too small when the resolution shrinks.

    You can then adjust the containers to become smaller or larger as you change screen resolutions.

    Your heading levels should also increase by one. You jump from h1 to h3 in your project.

    I hope this helped! Good luck on your future projects.

    1
    • HTML
    • CSS
    • JS

    CSS, HTML & JS (First time with JS) Found this quite difficult

    4
    Ken455 | Posted 19 days agocommented on Sedar Yildirim's "Article preview component" solution

    Hey Sedar!

    It seems like there's an issue with your content being centered. for the body I would:

    body {
      min-height: 100vh;
      display: flex;
      justify-content: center;
      align-items: center;
    }
    

    This will set any contents inside your body tag to the center of your page. That way you won't have to set margins and paddings to guess where the center of the page is.

    I also recommend not styling the html tag, as it is not the best practice.

    Other than that, Tereza made some good points to fix your project.

    1
    • HTML
    • CSS
    • JS

    Base apparel coming soon page completed using HTML, CSS, JS & Jquery

    2
    Ken455 | Posted 19 days agocommented on Abdur Rahaman's "Base Apparel coming soon page" solution

    Hey! The project looks great.

    It seems like you are missing the background image pattern for desktop.

    To get rid of your accessibility issues in this project, I would:

    • Change your container div to a main element. <main class="container">
    • You should also always have an h1 in your HTML. I would make it your hero-text
    • For best practice, I also recommend putting text-transform: uppercase in hero-text instead of writing "WE'RE COMING SOON" in html.
    • On your button I would put <button type="submit" aria-label="Name">

    Don't forget to generate a new report on frontendmentor after changing your code in github!

    To learn more about accessibility issues, I would read up on:

    There are times like this challenge, where buttons may have no text in it, which would raise an accessibility issue flag. You can read about that here

    Believe it or not, images also raise accessibility issues. There are also a lot of images used on these challenges that are purely decoration. In those cases you would use aria-hidden. You can read about that here

    There is also a video DesignCourse makes about ARIA that helped me understand better as well.

    I hope this helped. Keep it up!

    1
    • HTML
    • CSS
    • JS

    Blogr Landing Page with SCSS, BEM, and Vanilla JS

    2
    Ken455 | Posted 19 days agocommented on Ken's "Blogr landing page" solution

    Look absolutely amazing! Slowly making my way toward projects like this on frontendmentor and I cannot wait.

    I agree with your reflection, sometimes you should just step away from your project and come back later. So many times stepping away has helped me come up with new ideas, or clear my head so I can come back with a more open mind.

    1
    • HTML
    • CSS

    Responsive four card feature section using CSS Grid and Media Queries

    1
    Ken455 | Posted 21 days agocommented on Manuel's "Four card feature section" solution

    For small projects like this I believe media queries are fine.

    You are also missing an h1 in your entire project. All HTML projects should start with an h1. In my project, I set the h1 to "Powered by Technology", h2 was "Reliable, efficient delivery" but that was my vision.

    You should never use heading tags depending on their style, size, and font weight. Always focus on having that correct heading hierarchy. You can set style and all that in CSS later.

    Whenever you use a section tag, you should always have heading elements as child elements or it isn't semantically correct. In your case, <section class="cards-section"> lacks heading tags. Instead of using article elements, I would just turn them into divs.

    0
    • HTML
    • CSS
    • JS

    faq-accordion-card

    1
    Ken455 | Posted 21 days agocommented on Harsh Gupta's "FAQ accordion card" solution

    Great job! Just a quick suggestion:

    I would recommend wrapping your entire content in a main tag to get rid of your accessibility issues. It's just good practice for correct semantic HTML

    In your case it would be

    <main>
    <section class="card">
    ...
    ...
    </section>
    </main>
    

    Don't forget to generate a new report to get rid of the accessibility issues!

    0
    • HTML
    • CSS

    Four Card Feature Section

    3
    Ken455 | Posted 21 days agocommented on Shawnathan McCourt's "Four card feature section" solution

    I would set min-height: 100vh for the body which might help with your "excess white space."

    I'm not sure what your code looked like during that time so I'm not able to give you proper feedback on it, but great job on the overall project!

    1
    • HTML
    • CSS
    • JS

    Article Preview Component

    1
    Ken455 | Posted 25 days agocommented on Katherine Ebel's "Article preview component" solution

    Great job! Your styling looks great. Just a few suggestions:

    Make sure you wrap your content with the correct HTML Semantic Elements In your case, I would wrap all the contents inside including article in a main tag.

    I would also do that for your attribution section as well. However, this time I would wrap all the contents inside including the div class="attribution in a footer tag.

    Instead of using article for your card, I would recommend just using a div.

    If you are using section tags, your heading tags should always start in chronological order. You are missing an h1 in a lot of places which create HTML issues.

    I hope this helps. There is nothing wrong when it comes to visual aspects of your project. I'm just giving you tips on better practice. Your JavaScript is also A LOT cleaner than my attempt!

    0
    • HTML
    • CSS
    • JS

    accordion with tutorial by Coder Coder

    1
    Ken455 | Posted 25 days agocommented on Elekoot's "FAQ accordion card" solution

    Great job!

    I recommend you to wrap all your content in a main tag to get rid of accessibility issues. In your case, I would just simply wrap everything beginning and end from section with <main> </main>

    Although tutorials are great, if you are serious about learning front-end development, I strongly encourage you to try and complete a challenge here without the use of a tutorial! Refer back to what you learned from tutorials, but don't let it hand hold you through the entire solution.

    I hope this helped. Keep up the good work :)

    0
    • HTML
    • CSS

    social-proof-selection lance | completed using scss

    1
    Ken455 | Posted 25 days agocommented on Lance's "Social proof section" solution

    Looks great on desktop!

    I did come across a few issues when I switched over to mobile.

    You have two main wrapper divs which are .container and .content doing the same thing.

    Get rid of your absolute positioning. You should almost never position: absolute a wrapper div. Because of that, your entire content in mobile view is pushed down.

    If you want to center content on your screen, I recommend doing:

    body {
      min-height: 100vh;
      display: flex;
      justify-content: center;
      align-items: center;
    }
    

    Your breaking point for mobile view is slightly too late, making your content cut off around 1100px.

    I would also recommend using correct HTML Semantic Elements for your future projects. I realized you used the nav tag as an entire main content for your project. I would use nav strictly for navigation links. If you change nav to main it will be semantically correct, and would get rid of that accessibility flag.

    You also have a ul nested in a span tag, which is not the best practice either. I believe instead of using a span just changing it to a div would have the same results without having any HTML validation issues.

    I hope this helped, good luck!

    0
    • HTML
    • CSS
    • JS

    Article Preview Comp. with BEM, SCSS and Vanilla JS

    5
    Ken455 | Posted about 1 month agocommented on Ken's "Article preview component" solution

    Hey, I've been stuck on this challenge for a week now - trying to teach myself basic JS to make that sharing preview work.

    I've been watching Vanilla JS crash courses, going through the freecodecamp course for JS, but it doesn't seem to explain how I would use JS to do what this project is asking.

    Do you have any recommendations or suggestions on how I could teach myself? I don't want to copy someone else's solution without having a thorough understanding.

    0
    • HTML
    • CSS

    responsive landing page

    3
    Ken455 | Posted about 1 month agocommented on Olaphoxy's "Social proof section" solution

    Great job! Just a suggestion to get rid of your accessibility issues:

    Stray away from using heading tags to match the font size. Make sure you are increasing the heading tags from h1-h4, etc. Don't skip h2 and h3. I recommend changing your h4 headings to h2 and changing the font size of h2 to match the card.

    Your h1 seems a little too small compared to the original design.

    You should also wrap your content in a main tag. In your case it would be <main class="cover"> Make sure you do that for your attribution section as well, but make it a footer tag.

    I hope this helps and make sure you generate a new report once you fix this!

    0
    • HTML
    • CSS

    Social Proof Section

    1
    Ken455 | Posted about 2 months agocommented on Ogundeyi Joshua's "Social proof section" solution

    Great job!

    Just a quick suggestion to remove your accessibility issues: Avoid hopping headings from <h1> to <h3>. Without having an h2 on your page.

    You should also wrap the attribution div with a <footer>

    0
    • HTML
    • CSS

    frontMentor5

    2
    Ken455 | Posted about 2 months agocommented on Grzegorz Sowinski's "Social proof section" solution

    Great job! Just a few suggestions:

    Always have a <main> element wrapping your entire content. On your project you would have the main element instead of div for .container

    <main class="container"> </main>

    When you use the section element, it should always follow with child heading tags (h2-h6). Changing those sections to divs will also solve some errors that are being flagged.

    The layout for 1440px and 375px looks great but everything in between looks like your page is spilling out to the top and being hidden. I know the challenge is to make it for those two screen sizes. However, It's good practice to make it responsive for all screen sizes! I'm sure adding a quick media query for the other sizes and applying padding-top or margin-top will be a quick fix to that problem!

    Don't forget to generate a new report!

    0