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

  • f1r3 120

    @f1r3place

    Submitted

    What are you most proud of, and what would you do differently next time?

    I love how it behaves at different sizes, just looks and feels awesome

    While I would rather not deal with lists ever again in my life, I could probably make the unordered lists look decent using only the ::marker pseudo-element instead of ::before like I did

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

    The padding on the lists just did not want to work, at all. Had to look it up, and found a whole article on marker behavior with padding, so that helped

    P
    Roberts P 120

    @indaqoo

    Posted

    Hi @f1r3place

    Project looks solid, html is decent, but you could improve some thing about css.

    The current CSS class .container can be improved by separating it into two distinct classes: .container and .card. This approach enhances modularity and reusability.

    Instead of adding a border on an element for horizontal line you could use <hr> element and style it accordingly.

    hr {
        height: 1px;
        background-color: var(--stone-150);
        border: none;
    }
    

    Looks like you are making a lot of flex box container where they are not really needed. since you can apply margin bottom to the heading. and If you could rename divs to sections it would be easy to contorl that

    section > h2 {
        font-family: 'Young Serif', serif;
        font-weight: 400;
        font-size: 1.75rem;
        margin-bottom: var(--size-300);
        color: var(--brown-800);
    }
    

    This is how I styled my lists:

    ul, ol {
        padding-left: var(--size-300);
    }
    
    ol li::marker, ul li::marker {
        color: var(--brown-800);
    }
    
    ol > li::marker {
        font-weight: 500;
    }
    
    li {
        margin-bottom: var(--size-100);
        padding-left: var(--size-200);
    }
    

    There is also some differences in font weights from the design

    Usefull video for lists:

    Marked as helpful

    0
  • P
    Roberts P 120

    @indaqoo

    Posted

    Hi @Avanti-L,

    Everything looks good at first glance. I took a look at your HTML and there are a couple of changes I’d suggest.

    1. For changing <p class="name">Jessica Randall</p> to <h2>Jessica Randall</h2>: h2 is better for headings and helps with page structure and SEO, while <p> is for paragraphs.
    2. For replacing button tags with links <a>: links are meant for navigation, and buttons are for actions. Using <a> tags for GitHub, LinkedIn, etc., fits their purpose better.

    When I checked the site in DevTools with a 320px width (which is a common mobile size), the layout was breaking - view screenshot.

    I also took a look at your CSS and noticed that all your styles are in just two media queries - which is not ideal

    1. For having only two media queries in your CSS: Complexity and Performance: Using too few media queries can make CSS complex and affect performance. More specific breakpoints make the layout more flexible and efficient.

    Here is my css of this challenge - view on github

    When I was doing this challenge I followed a mobile-first approach. There is a nice blog which explains what that means - view blog

    But in short: start with Mobile Styles, design the base styles for the smallest screens first and then apply media queries when screen size is getting bigger.

    I've noticed that you used .container class for the card itself i would suggest to seperate those out like <div class="card container">.

    You could benefit for taking a look at mdn docs, there you will be able to see all the available css selectors.

    0
  • P
    Roberts P 120

    @indaqoo

    Posted

    Hi @delfinjfb,

    The use of Remix with TypeScript feels a bit excessive for this project, especially since your component is contained in a single file. A simpler approach using HTML with the Tailwind CSS CDN might have been more efficient.

    Additionally, since Tailwind CSS is designed with a mobile-first approach in mind, adding padding for a 320px width screen would enhance the mobile experience.

    Overall, it looks great. Nice work!

    Edit: text sizes are a bit different than the design.

    Marked as helpful

    0
  • Lyna 260

    @lynaIFR

    Submitted

    What are you most proud of, and what would you do differently next time?

    I'm pretty proud of how the project came out, if you have any feedback, please feel free to share.

    P
    Roberts P 120

    @indaqoo

    Posted

    Hi @lynaIFR,

    Looking at the HTML, I noticed you are styling elements using IDs ( # ). This is not ideal.

    IDs have a higher specificity compared to classes. This can make it harder to override styles later, as you would need to use even more specific selectors.

    IDs are often used in JavaScript for element selection and manipulation.

    Classes are reusable across multiple elements, whereas an ID is supposed to be unique within a document.

    Using height: 100vh on .container might not be the best approach. It is best to set height of html and body tag:

    html,
    body {
        height: 100%;
    }
    

    The .container class is intended to wrap your main content, centering it both vertically and/or horizontally within the viewport. Here’s an example of how to use it correctly ( based on mobile first approach).

    .container {
        max-width: 90%;
        margin-inline: auto;
    }
    
    @media only screen and (min-width: 425px) {
        .container {
            max-width: 100%;
            margin-inline: unset;
        }
    }
    
    <body>
        <main class="container">
            <article class="article">crazy content</article>
        </main>
    </body>
    
    

    At the moment there are no media queries to handle different screen sizes, which makes the design less adaptable to various devices.

    Project looks good on desktop size but that cannot be said about mobile versions - view images

    You should consider a "mobile-first" approach. You can also check out my project on article cards where I implemented this approach.

    Have a look at Kevin Powell's responsive web design video: youtube

    Overall great job, keep it up.

    Marked as helpful

    1
  • @damigand

    Submitted

    What are you most proud of, and what would you do differently next time?

    I am proud of how I managed to get the dimensions down because I don't have a sharp eye just yet.

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

    I didn't encounter any particular challenge.

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

    I'd like to know if there's any better way I could have written my HTML document, but help with anything overall is greatly, greatly appreciated.

    P
    Roberts P 120

    @indaqoo

    Posted

    Hey,

    First of all, I wanted to welcome you to Frontend Mentor and congratulate you on successfully completing your first challenge.

    Let's start by reviewing your HTML. The general structure looks good.

    Maybe I'm nitpicking here, but I think it would be great if you used the <div> element for the card itself and wrapped both text elements in their own <div> element, like in the example below:

    <main>
       <div class="card">
          <img src="./assets/images/image-qr-code.png" alt="qr code" loading="lazy" />
          <div class="card-content">
            <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>
       </div>
    </main>
    

    Here is a good resource of example card - w3school example card

    Looking at the css..

    I see that you missed the left and right padding in the text section, which is included in the Figma design file's "Text" component - view screenshot

    The border-radius on the card should be 20px instead of 16px - view screenshot

    The padding on the card should be 16px instead of 15px.

    The card's image is currently 240px by 240px but in the design it is 288px by 288px

    There are a lot of incorrect/missing values that are specific to this task and provided to us.

    I think you need to pay more attention to the design system provided in the Figma files and maybe watch a Figma tutorial to see where to get all the values. This will ensure that all the challenges look as good as possible.

    Overall, it is a good project, but I would recommend following the provided design system more closely to improve accuracy and consistency.

    Marked as helpful

    2