Skip to content
OverviewSolutions
4
Comments
4
P
Sclata
@Sclata

All comments

  • Sclata•100
    @SclataPosted about 3 hours ago

    The desktop view looks excellent but the design doesn't look to be responsive. Note that the (image) padding / margins and border radii in the provided previews are very different for the mobile views. Consider going back and modifying your design to include more responsive features. Note that this could require some modification of your html as well to allow targeting specific parts of the project in new ways (excellent structure in your index.html file by the way. It looks so clean). Pay particular attention to how padding impacts images and the rendering of those images with regards to those border radii. Good luck if you choose to change it!

    Marked as helpful
    0
  • Sclata•100
    @SclataPosted about 3 hours ago

    Pretty close, but I think a part of the challenge was overlooked here. It was looking for transitions on buttons to yield a kind of fade effect and the problem statement also suggested enabling those transitions for keyboard controls as well. Consider going back and adding these.

    Additionally, (this is very minor and probably not worth changing since I'm sure you know), but you might review the padding on your card div to match the end product. Really nice job, though.

    0
  • Sclata•100
    @SclataPosted 1 day ago

    Looks great! Consider the cascade and how inheritance and specificity will affect style rendering. Looks like this may have been an issue with the color-palette on the description text in the body. Still looks to be better than my solution.

    Marked as helpful
    0
  • hbeamaniii•10
    @hbeamaniiiSubmitted 2 days ago
    What are you most proud of, and what would you do differently next time?

    Design has always been my weakness. To be able to see a finished design, and finally bridge the gap between the design and the code needed to produce that design, and writing that code is what I am most proud of.

    Next time, I would like to incorporate some of the layout techniques using grid or flexbox. It did not seem necessary for this challenge.

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

    The main challenge was getting the size of the text right. It was not immediately clear from the design file whether the font size is represented in points or pixels. A google search then some trial and error helped to overcome that challenge.

    The second challenge was understanding the box model. There was too much extra space at the bottom of the design. I still don't think I overcame that challenge in the correct way. I simply changed the height of the container element to eliminate the extra space.

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

    I would like feedback on the structure of the html, layout techniques, and using the box model in css.

    QR Code Component

    3
    Sclata•100
    @SclataPosted 2 days ago

    Looks pretty good! I see a react tag but don't really see resources to that effect in the repo. Just a couple things to note on a quick review of your code:

    1. Remember the cascade. You can simplify some of the style declarations by specifying the rules in parent elements and letting them run down the chain. As an example, you declare font-family and text-align properties in both the h1 and p selectors, where you could declare them only once instead in the .info class selector (since it is the parent div).

    2. Understanding the box model and basic flow layout is good, but don't sleep on flexbox (and grid when needed). It/they simplify positioning of elements a lot.

    3. Look at how height is calculated for the body element and then see if you can use a style declaration relative to the viewport. This will help center your QR component on screen.

    Marked as helpful
    1