Skip to content
OverviewSolutions
7
Comments
7
kacper
@Ikacper

All comments

  • kacper•110
    @IkacperPosted over 3 years ago

    hey, your main card is little bit to wide, max width of 750px would be better

    instead of creating div for image you could use pseudo class before to set image there

    on smaller screen popup 'share' should look differently than on 1440px width

    your alt message on avatar image is not good in my opinion, screen reader dont know who is michelle i would say its smiling woman or you can describe it more. Its only my thought, i have read one article about creating alt description thats what i remember.

    In general i really like your solution, nice work

    1
  • kacper•110
    @IkacperPosted over 3 years ago

    hey Aastha! your 'share' div goes on wrong position on higher screen width than 1440px drawer img should be handled by css not html ( if pictrue is responsible for design purposes only, should be handled by css) you could set max width on your main card so that the component does not expand on small screen, after clicking share btn you are not hiding card__last component

    other things looks nice, good job! ;)

    0
  • Bryan Sanchez•590
    @0-BSCodeSubmitted over 3 years ago

    Questions:

    1. Is there another way to vertically center an element that preserves its place in the normal flow of the document? The method I used here required the element's position to be set to absolute but, from what I understand, this removes it from the normal flow of the document. I'd be more than happy to hear any suggestions or advice!

    2. When submitting solutions to Frontend Mentor, is editing the README files necessary and, if so, how to go about it? This is the first challenge I've done and I'm still not sure how to go about proper documentation for my solution and how to properly submit my solution to the site. I'd greatly appreciate any help on this!

    HTML, CSS

    1
    kacper•110
    @IkacperPosted over 3 years ago

    i think you shouldn't use box shadow

    images which are responsible for design only should be handled by css not html

    it is possible to make only one class for buttons, you can check my solution

    responding to your first question, i think, you can add on your body tag display: flex, flex-direction: column and height: 100vh and remove position absolute from container to achive vertically centered an element

    0
  • kacper•110
    @IkacperPosted over 3 years ago

    i have two suggestions

    color of your buttons should be same color as color of the background

    images which are responsible for design only should be handled by css not html

    1
  • kacper•110
    @IkacperPosted over 3 years ago

    Im not sure why you put overflow: auto?

    i think you shouldn't make this div, for img, just try to use background img on #content

    you probably should use classes instead of id's for styling

    instead of putting in every single id box-sizing: border-box you can use universal selector like that *{ box-sizing: border-box} it means every element will get those property

    for fonts, paddings etc. its good practice to use rem instead px.

    0
  • Aksharmeet Singh•135
    @AksharmeetSubmitted over 3 years ago

    It would be great help if someone could tell instruct me on how I can make the image more responsive.. though overall it was a great learning experience

    Stats Preview Card Component

    1
    kacper•110
    @IkacperPosted over 3 years ago

    image for design only should be handled by css not by html ( use background-image) you should use flex direction: row-reverse for desktop view you didn't imported proper font

    thats all my suggestions to change ;) good luck!

    0
  • kacper•110
    @IkacperPosted over 3 years ago

    if an image truly doesn't convey any meaning/value and is just there for design purposes, it should live within the CSS, not HTML. https://moz.com/learn/seo/alt-text

    im not sure but i heard that its a good practice to use css variables in :root

    you set margin and padding with px and also i heard that good practice is to use rem, what do you think?

    0