Skip to content
OverviewSolutions
7
Comments
7
defPhisy
@defPhisy

All comments

  • defPhisy•140
    @defPhisyPosted 3 months ago

    Looks great Michael 👍

    • Does the solution include semantic HTML? Yes!

    • Is it accessible, and what improvements could be made? Yes, you had the same accessibility issue with h2 😂. Nice hack with h1 hidden. I was to lazy for that. For screenreader it is redundant to use a alt text for the images. They have no meaning you have already the name in h2. I would omit the alt text.

    • Does the layout look good on a range of screen sizes? Yes, you could improve the transition between your mobile and desktop a bit better. When reducing screen size form desktop to mobile around 1080px width your layout shrinks. At this point it is not responsive. The font-sizes getting very small.

    • Is the code well-structured, readable, and reusable? Yes!

    • Does the solution differ considerably from the design? Yes, an improvement would be better matched font-sizes and font-weights

    0
  • defPhisy•140
    @defPhisyPosted 3 months ago

    Hey Hoaxilog, your solution looks great:

    • Does the solution include semantic HTML? Yes!
    • Is it accessible, and what improvements could be made? Yes, but i would suggest to leave the alt text from the svg icons, because i think they are just cosmetic and screenreaders don´t need this information, they already get it from the headings.
    • Does the layout look good on a range of screen sizes? Yes, i like your middle layout with 2x2 cards. Only your mobile view could get a bit tweaking. The headings have to little line height.
    • Is the code well-structured, readable, and reusable? Yes.
    • Does the solution differ considerably from the design? The font-weight of your normal text is a bit off.
    Marked as helpful
    0
  • defPhisy•140
    @defPhisyPosted 3 months ago

    Wrong challenge upload? 🤔

    0
  • defPhisy•140
    @defPhisyPosted 3 months ago

    Hi Emir, great work the card is very close to the design, looks very good.

    • Does the solution include semantic HTML? Yes, good use of lists and table. Try to use more semantic elements like section and article instead of divs. That concerns me too 😁.

    • Is it accessible, and what improvements could be made? I am too inexperienced to give you tips here. From my point it is ok, since there is no interaction on the site. Beside that you could improve your alt text on the omlette picture. "img" is not very descriptive.

    • Does the layout look good on a range of screen sizes? Desktop size looking great, except the white bar on the bottom and the missing margin on top of the card. When you have a look on the design files the mobile version should have a different layout. you can work with media queries to solve that. You should consider to use less px an more relative measures lite rem to ensure a responsive design.

    • Is the code well-structured, readable, and reusable? Yes. Is there a reason why you use <span> elements on all links?

    • Does the solution differ considerably from the design? The card itself looks very good. You should improve the padding on the body to give some margin at the top and bottom of the card. The background also needs to be continuous. You do not want the white bar on the bottom.

    Keep up the good work. 👍

    0
  • defPhisy•140
    @defPhisyPosted 3 months ago

    Hey Victor your solution looks great. Might be a bit late but i have some proposals/notes:

    Does the solution include semantic HTML?

    Yes, but i would suggest to replace the <button> elements with <a> elements because buttons should be used for on page actions only. For example a form submission or a modal. Anchor elements are supposed for external links that route you to another page.

    Is it accessible, and what improvements could be made? No, you cannot navigate with your keyboard to all the links. Anchor elements in your buttons could solve it. You can also play with the :focus preudo-class. Antoher improvement could be better contrast between your buttons and the background.

    Does the layout look good on a range of screen sizes? Yes.

    Is the code well-structured, readable, and reusable? Yes.

    Does the solution differ considerably from the design? No, your font is not correctly loaded. Use instead of:

    src: url(/assets/fonts/static/Inter-Regular.ttf) format("ttf");

    the correct format "truetype"

    src: url(/assets/fonts/static/Inter-Regular.ttf) format("truetype");

    Hope that helps.

    0
  • UMo0HU•40
    @UMo0HUSubmitted 4 months ago
    What are you most proud of, and what would you do differently next time?

    I am proud of developing my search skills to overcome the problems I face.

    blog-preview-card-main

    1
    defPhisy•140
    @defPhisyPosted 4 months ago

    Great Job.

    To make your html more semantic u can use instead of <div> elements like <article>, <header>, <footer>, <main>, ... There is a good overview on w3schools.

    I think you can improve your css-reset. For example omit padding: 0; on the star selector for your css reset. I read fantasstic articles form Josh Comeau and Andy Bell about modern css reset. Made my own css-reset.css based on theses articles in this project.

    Another great advise i got: Why font-size must NEVER be in pixels

    Hope i could help you a bit. Keep going ;)

    Marked as helpful
    1
  • Yash Bhatt•280
    @Y-ashbhattSubmitted 4 months ago
    What are you most proud of, and what would you do differently next time?

    I was able to use Flexbox with ease and was happy with it but would try to use Grid next time.

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

    It was troublesome to make the mobile design had to go through media queries again to make it barely ok.

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

    I would like to know what's the approach to make mobile design and help with the sizing in the same.

    QR code component solution

    2
    defPhisy•140
    @defPhisyPosted 4 months ago

    Hi, first of all I want to gratualtion you for doing this challenge, even if it's the first one, at least for me. This is my first review and i am not an experienced developer but i try to give you some good feedback related to my actual knowledge:

    • html looks good from my side, but i would omit the div .container as you use it on the whole page with 100 vw/vh. Just move the styles to body:
    body {
  background-color: var(--Light-gray);
  font-family: var(--main-font);
  width: 100vw;
  height: 100vh;
  display: flex;
  justify-content: center;
  align-items: center;
}

    • i would´ve been more descriptive with the class names. For example, instead of class="box" you could use for example class="qrCodeCard". I think if you are working on bigger projects you may have many elements like "box" and you want to style them differently. That might be difficult if you just name it box.

    • I think in this challenge it is not necessary to make a separate mobile version since the size of this qr code is very small. for mobile your mediaquery is at 480px and the qr code has a width of 320px.

    • what you should fix: width and height are declared with different percentages. You should use fix values in this case. The card has clear measures. Your styles deform the img.

    @media (max-width: 480px) {
    
    .box {
      width: 55%;
      height: 70%;

    .box {
  width: 23%;
  height: 72%;

    img {
  display: block;
  height: 63%;
  width: 100%;

    Hope i could help you. Keep up the great work.

    Marked as helpful
    1