Skip to content
OverviewSolutions
10
Comments
4
P
Stefan Vetsch
@vstm

All comments

  • nimrodEDE•170
    @nimrodEDESubmitted about 1 month ago
    What are you most proud of, and what would you do differently next time?

    I'm proud of starting to use grid , I need to learn alot more and to be better with it .

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

    i was having trouble layering the cards in + shape ! I really dont know if i did a good job or not , but it seems to work .

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

    fixing my grid . thanks !

    four-card-feature-section-master

    2
    Stefan Vetsch•140
    @vstmPosted about 1 month ago

    Congratulations on finishing that challenge, nicely done.

    I have looked at your code with regards to the grid and there are some things I think you could do.

    • You rely on margin-top: 7em; to vertically center the boxes, this could be solved easier if you remove the margin-top and just set align-items: center on the .cards-container. The align-items and justify-content properties are helpful to know here.
    • Inside the grid you wouldn't (or shouldn't) rely "margin" that often, you would rather use the gap property to make the space smaller/larger (which you actually used)

    Hope this helps as a first pointer, if you have any questions about my comment don't hesitate to ask.

    0
  • Stefan Vetsch•140
    @vstmPosted 2 months ago

    Congratulations on your solution, the code is really clean and easy to read. I liked how you solved the responsive image using the hidden/block method with the breakpoints - why didn't I hink of that :D. I don't have that much to add to improve it, but there are some things that caught my eye (and some of those points are a bit nit-picky):

    • The empty div around the <img>-tags could be removed
    • There are two <h1> tags, one for the product name and one for the price, which is not strictly wrong but here it could be easily avoided - for example using a <h2>, a <strong> tag or just a plain <span> tag for the price (there is no semantic HTML tag for a money amount as far as I know).
    • xl:w-2/5 lg:w-3/5 w-11/12 - I'm not sure if those width's are necessary, just setting the max-w-* might be sufficient (I haven tested it though).
    • rounded-bl-2xl rounded-tl-2xl could be shortened to rounded-l-2xl, and rounded-tr-2xl rounded-tl-2xl could be shortened to rounded-t-2xl. This one is a matter of taste.
    • I've seen you kept the camelCase notation for the color names, I would have changed it to kebab-case (so instead of text-neutral-darkGrayishBlue I would have set it up so I could write text-neutral-dark-grayish-blue). Again this one is just a matter of taste.

    So again, great solution overall - I actually learned about the ratio-widths (w-11/12 etc) which I didn't know. I nevertheless hope my points might help you a bit.

    Happy coding! :D

    0
  • Stefan Vetsch•140
    @vstmPosted 3 months ago

    Congratulations on finishing that challenge, nicely done!

    I have looked at your code and I have found some things that I would recommend to improve your code:

    • You have applied margin-left and margin-right of 25px to almost all of your elements in your content-area. This can be done a bit easier by applying a single padding to your #menu element, this way you can define the spacing once in the parent element and you don't have to add it to your child elements
    • For the table at the end you could use the <table> HTML element. This makes aligning the content inside the table easier (but I have to admit that styling a table can also be tricky).
    • Instead of using absolute position of your #menu element you can add display: flex; justify-content: center to the body element to center the menu.

    I hope these pointers help you out a bit, don't hesitate to ask if you have any question about my feedback

    0
  • Eucalyptus2•10
    @Eucalyptus2Submitted 4 months ago
    What are you most proud of, and what would you do differently next time?

    I did a walkthrough code along and had trouble linking my second time qr code without help. The second time around I was able to mimic somethings so im most proud of that. Next time I will do it without help and just simply submit lmao

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

    All feedback is welcomed and appreciated!

    Responsive CSS HTML Qrcode

    1
    Stefan Vetsch•140
    @vstmPosted 4 months ago

    Nicely done, congratulations on finishing your first challenge :D

    I think your HTML looks really clean, I also like the comments in your CSS (comments are something I should use more often).

    Here are some things I think you could improve:

    • I think the line <link rel="stylesheet" href="Outfit.zip"> can be removed, since stylesheets should be CSS files directly, the browser doesn't know what to do with a ZIP in this context
    • The line <link href='https://fonts.googleapis.com/css?family=Outfit' rel='stylesheet'> can be moved inside the <head> tag. The browser handles it correctly but if you check out the DOM in the inspector that is because the browser moves it automatically inside the <head> tag.
    • <img id="qr" ... -> I would change that id attribute to class and use a class selector in your css (#qr -> .qr). This way your "way of styling" is consistent (all the other elements you also style with class selectors). Also depending on who you ask ID selectors are discouraged because they are not reusable. For example and you can only style one element per page with it, because only one element can have id="qr".

    I hope my comment helps you, if you have any questions regarding my feedback don't hesitate to ask me :D

    0