Skip to content
OverviewSolutions
0
Comments
4
David Frederick
@david-r-frederick

All comments

  • David Frederick•15
    @david-r-frederickPosted almost 4 years ago

    Sorry, I should clarify. That graphic isn't centered when the screen is wider. When it's phone width, it's fine. You can find a way to make it always centered though.

    0
  • David Frederick•15
    @david-r-frederickPosted almost 4 years ago

    Hey Dave! Looks great! A couple things: 1) I would add a transition effect for when the questions open and close. It's not necessary, but it would make a much smoother user experience. 2) The mobile version of the graphic with the woman and the computer is not centered. You have the FAQ header centered, so I'm sure you have the skills to figure it out ;)

    0
  • appsplash99•20
    @appsplash99Submitted almost 4 years ago

    This is my first submission via github.

    1. I am concerned about is the mess i have created in the stylesheet.css file, any suggestions on how should i improve??
    2. Did i do it okay??
    3. To the experts, what will you recommend me to code with less mess??

    And sorry for the looong url vercel is a long one :p Thank You :)

    Mostly used css flex and just one media query for mobile+tablet view

    1
    David Frederick•15
    @david-r-frederickPosted almost 4 years ago

    You should create separate files for either a) areas of the code by category (for instance: layout, typography, variables, etc.) or b) different components (for instance: the rating cards, the quote cards, etc.). After you have your separate files, you just import them separately into your html file. It sounds counterintuitive and like it's more work, but in the end it isn't because you save yourself time but not having to search for what you're looking for. Other than that, it looks responsive and pretty well done. It's bigger than the initial design, but that's not a bad thing to me. One thing I would change is your media query for the rating boxes. Looking at 5 stars on two different columns is weird. You could either micromanage it by making your content smaller as the width of the window shrinks or (to me, more ideally) you could set an earlier break point for when you change to mobile view.

    1
  • David Frederick•15
    @david-r-frederickPosted almost 4 years ago

    NoorAlhijab, Looks great overall! A few things I'd fix.

    1. Make the laptop and phone align to the right edge and use more whitespace in the middle with screens that are bigger than yours. I'm thinking something like justify-content: space-between in a flex display. Also, tied with this is that I'd probably make the navbar at the logo in the top left separate items so that justify-content can work on the left side and the right side.
    2. The navbar items go out of the gray box when you squish the screen a bit (horizontally). This should be fixed if you do number 1. Alternatively, you could just make your hamburger button show up sooner.
    3. A small thing, but I would add some sort of transition for your hamburger button/menu. It just pops up sharply. I'd probably make it come in from the right, but that's sort of up to personal preference.
    0