Skip to content
OverviewSolutions
9
Comments
5
Fabianthorsen
@Fabianthorsen

All comments

  • Alexis Quizhpe•30
    @ArekshissSubmitted over 1 year ago

    I tried my best but I did not know how to define @media to make it responsive. I tried to use min-width and max-width but I was not sure how to do it.

    Thank you for the opportunity.

    Designing QR Component with HTML and CSS

    3
    Fabianthorsen•170
    @FabianthorsenPosted over 1 year ago

    Hey, great work! For this solution you don't really need to use media queries, but if you would like to understand how they work and their syntax it is very simple:

    @media screen and (min-width: 600px) {
    // Logic that should override the css when browser window is larger than 600px
}

    For the above example everything we put inside the "curlies" would only apply once the browser window meets the condition we set inside the parentheses. If for example we had some class with a height of 100px like so

    .some-class {
    height: 100px
}

    This object would be 100px no matter the screen size, i.e. non-responsive. However if we want it to become 200px if our screen size increases above 500px we could make a media-query like so:

    @media screen and (min-width: 500px) {
    .some-class {
        height: 200px
    }
}

    This would make it so that it changes once the browser window is larger than 500px. Hope this made some sense, if not you should read this article on responsive web. Hope this helped!

    2
  • Fabianthorsen•170
    @FabianthorsenPosted over 1 year ago

    Great solution, and good attention to detail. Here are som points that I noticed when looking at your code:

    CSS

    When you're using css pre-processing frameworks such as SCSS there are so many cool things you can do, one thing you're doing now is:

      .image-div img {
    max-width: 100%;
    border-radius: 15px;
  }

    while you could reference the parent with & making it:

      & img {
    max-width: 100%;
    border-radius: 15px;
  }

    You can do this when you're using pseudo-selectors like &:hover and much more as well.

    HTML

    Try using semantic-html instead of only relying on div for everything, this can improve SEO and accessibility. Read more on sematic HTML here

    Design

    Lastly just a quick tip in terms of design, your font as it is not in some places, is really difficult to read as it is both thin and small at the same time. Try to emphasise areas of importance by making them bold and maybe increase their fonts a little, or changing the colour is also a good alternative.

    Marked as helpful
    0
  • Fabianthorsen•170
    @FabianthorsenPosted over 1 year ago

    Hey, good work on this one. Here are some of my pointers as to what I think you could have done differently:

    HTML:

    • <div class="main"> is not semantically the same as <main></main>. All HTML sites should have a main tag, and thus it should be quite simple to change. And thus rather have the main wrapping the div serving as a container.
    • Im unsure in the use of a section tag to enclose images that are inside of other objects. I think the a section or an article tag should be what wraps the other objects, meaning that it could make sense to use a section tag to wrap everything related to the text body, but not a div wrapping the section, if that made any sense.

    CSS:

    • Nice job using variables, but I would not use variables for anything that is just used once or twice.
    • When values are 0, such as the 0em you can omit the suffix, so 0em, 0px, 0rem can simply be written as 0.

    Im not a pro, and do not take everything I say face value, but hopefully some of my comments are helpful! :D

    0
  • Fabianthorsen•170
    @FabianthorsenPosted almost 2 years ago

    Hey, great solution! A small tip for me in regards to the sizing of the box component. In css there is a calc() function that lets you make calculations using numbers, variables, etc. For this solution you could use a the minimum width 360px calculated with the approximate size you want your .box to take of the entire scree. So the css could look something like calc(360px * 0.7) to get 70% of the minimum width. Hope you find this helpful, good solution! :D

    0
  • Fabianthorsen•170
    @FabianthorsenPosted almost 2 years ago

    Hey, great solution! A couple of things I would refactor if I was doing it myself is to use the cascading to the fullest. This is a trivial challenge, but setting the font-family on the overall html page I feel would be make more semantic sense, and rather set the font-family to the individual object if you are using different fonts for titles and so on. Also in regards to text styling, you could set the text-align property on the higher-level element like the container instead. Lastly I would consider giving the example a background just for the sake of accessibility with better contrasts!

    Anyways, great submission :D

    Marked as helpful
    0