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

  • P
    Nikhil 60

    @nikhilxe

    Submitted

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

    awesome stuff

    sofiasmnk 80

    @sofiasmnk

    Posted

    You probably don't want to be manually setting a height and width in pixels for every element (.container, img, .content). For an instance, the image could've just had it's width set to 100% and it would've adjusted to the size of the container.

    The container appears off-centered to me. I think the margin-left:35%; margin-right: 35%; properties on it are the problem. If you want the margins to adapt so that the content is always centered, you can apply margin: auto. (in this case, where you have 128px set for the margin top and bottom, you could use the margin shorthand to write is all as margin: 128px auto. The first value will be applied to the top and bottom, the second value will be applied to the left and right.)

    The challenged proposed that you try to use semantic HTML. The ul and ol elements for the lists have been applied correctly. There are other places where you could've used semantic elements instead of div:

    • h1, h2, h3 etc. for headings;
    • p for paragraphs;
    • table for the nutrition table.

    Semantic HTML is better for accessibility and for search engine optimization.

    Finally, the design doesn't seem to be responsive. There are a few things you could do to make it responsive:

    • Set max-width rather than width on elements, allowing them to size themselves down in smaller screens.
    • Use margin: auto.
    • Change styles depending on screen size using media queries (ie. @media screen and (min-width: 375px) etc.)

    For this challenge, there's a different design provided for small/mobile screens, which has some differences from the desktop design. You'd probably have to use media queries to achieve that. However, just using more flexible properties and values like max-width and margin: auto should also help a lot to make things more responsive in general.

    0
  • @jvondungen

    Submitted

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

    I was able to work through this challenge quickly, so I feel like my understanding of CSS styling is improving.

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

    I did not have the Figma files, since I don't have a Pro account, so had to estimate the spacing and sizing. But this went okay and I think I was able to get a pretty close to the design.

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

    I welcome any feedback. Responsive design below 300px loses some of the card container, so suggestions on that would be helpful.

    sofiasmnk 80

    @sofiasmnk

    Posted

    In your media query you set the card container with a width of 325px, which is why it doesn't fit in a 300px screen. You could instead set it to a width of 100%, so that it automatically resizes itself to take up the width of the screen; that way it'll size itself down when the screen gets smaller. And if you add some horizontal padding to the body, it will stop the card from "touching" the sides of the screen, which would look strange for a card with rounded corners.

    Another piece of feedback is: you gave separate .social1, .social2 etc classes for each of the social links, and then when styling in CSS had to add all those classes as selectors. Since they all look the same anyway, you could've used just the same .social class for all of them and then styled that one class. If you ever needed to have a way to differentiate and target specific social links from the list, you could use id to tell them apart. While a class is supposed to be applied to multiple elements, id is the one that should apply to just one element in a page, in order to specifically identify it.

    Marked as helpful

    0
  • sofiasmnk 80

    @sofiasmnk

    Posted

    The font-sizes aren't responsive to the screen width. You could try to use vw as a unit when defining font-size so that it's proportional to the screen, just define a max and min value so that it doesn't spiral out of control in very large or very tiny screens.

    Additionally, I'm not sure why the quote-text class has its font-size set to small when it's the one element in the original design that uses the default paragraph size (on the Figma file it's 16px on desktop and 14px on mobile, while the smaller text elements are 14px on desktop and 12px on mobile). The title/heading isn't a link and doesn't have a hover state (in Figma, the text turns yellow on hover).

    Accessibility-wise, the alt-text on the images is just the file name without the extension, which doesn't really describe them. You should either properly describe them if they're important to understanding the page, or leave the alt-text blank (alt="") so that they're ignored by screenreaders if they're only decorative.

    I'm not sure why the media query sets a max-width for the quote-text instead of letting it fill the card, since the card's width was already set. Maybe the goal was to make the lines of text break the way they did in the design / preview image? It might not've been breaking exactly the same due to the font-size being smaller. But it's probably preferable to let the text break differently than set an arbitrary fixed width.

    I would reconsider the learning-container class name. The text in this example says "Learning", but supposedly that's the category of the blog post and other blog posts in this hypothetical website would have different text there. It would make more sense to either name it something like category-container or give it a name that just describes it more visibly so it can also be used for other elements sharing this style (maybe something like chip-primary or tag-primary?)

    0
  • sofiasmnk 80

    @sofiasmnk

    Posted

    It looks almost exactly like the design, I think the only difference I see is that the text is missing the additional padding on the sides (while the image only has 16px of padding on the left and right, the text is supposed to have 32px).

    The CSS is giving things fixed sizes where it's not necessary. For an instance, the height of the card doesn't need to be explicitly set to height: 499px, if you just set the width and padding correctly you'll see that just letting the card's height adjust to the content will result in the same height.

    It's also redundant to be setting both the card width and image width in pixels; usually you'll want to either set the width of the card and let the image scale to fill it, or set the width on the image and let the card size itself it fit around it.

    Marked as helpful

    0