Survey Form Roast me!

Please come at me with all the criticism (constructive) you’ve got!

link: https://codepen.io/elijah2k12/pen/pZzQve?editors=1100

And thanks :smiley:

Hello Elijah,

Just checked your work.

I think the style and design looks good, except the text over the banne(image), I suggest you add a contrasted color as outline(text-shadow) to make it easier to read. Also make it a little larger. And since it’s about the fashion, so let’s have a fancy shadow, why not?!
My suggestion:


The css for title(h1) tag:

#title {
    margin: 0px;
    text-shadow: 0px 2px 4px #8BC34A, 0px -2px 4px #2196F3;
}

And css for subtitle:

#description {
    font-size: 1.5em;
    font-style: italic;
    text-shadow: 0px 2px 4px #00BCD4, 0px -2px 4px #FFEB3B;
}

Go for any color(kep it sync with otehr component colours like button) and shadow amount you like, but just add text-shadow to make a contrast with bg image to make it easier to read.

About the layout, Opps! it’s broken for mobile, check:

I think you used too many media queries, if you code the layout perfectly, so you will need one media query for mobile I think, but it also depends on page context too.
Also remember when you go for a media query that between two sizes, you should specify both min and max value, for example, following code(your code) could make issues:

@media (max-width: 620px) {
  #container {
    width: 320px;
  }
}

@media(max-width: 520px) {
  #background-image {
    background:none;
  }
}

This is better you set min size for a media query when another query could handle that size, so the fix could be like:

@media (max-width: 620px and min-width:519px) {
  #container {
    width: 320px;
  }
}

@media(max-width: 520px and min-width:459px) {
  #background-image {
    background:none;
  }
}

Your checkbox button come with broken label tags, All label tags refer to first checkbox, please fix, even label for textarea is linked to first checkbox.

If you fix the layout, it will be great.

Maybe reading this survey form challenge walkthrough article could help you about the layout Elijah, I suggest a quick read about the layout and template sections.

Keep going on great work, happy coding.

1 Like

HOLY! The roast is insane!

Haha, all jokes aside, thank you so much for all the feedback.

  1. I’ll definitely try to spice up the title like you said!

  2. Regarding the mobile layout fail, I just checked it on my device on the same window size and it works fine for some reason. Would you happen to know why that is?

  3. I added the min-width values! You said it’s possible to make only 1 media query but I was wondering how?

  4. Time to read about input checkboxes again :joy:

EDIT: the amount of edits on this one comment is insane.

Maybe your mobile screen is more wide! or it comes without zoomed content.

You may not need media queries for every 100px steps! not really, you may go for a query than is triggered for screen less than 3in width for example, and another for desktop/tablet.

Not a problem, but if you go for another epley could be better so we would catch your reply easier, thanks.

After I added the min-width, my responsiveness died haha. Gotta do it the proper way I guess. I’ll try for the 2 queries this time.

Hey NULL, I just realized there’s a small syntax error for the fixed solution @media post. I was confused until I searched it.

Here’s the fixed version if anyone comes by this thread.

@media (max-width: 620px) and (min-width: 519px) {
  #container {
    width: 320px;
  }
}

@media(max-width: 520px) and (min-width: 459px) {
  #background-image {
    background:none;
  }
}

Hi Null, could you check if the layout problem is fixed?
I can’t seem to look at it on that layout but I did change some things. Thanks.

Hi Elijah,

Very good progress about layout, but there is one very small issue let me tell you.

Considering following CSS media query(lats one)

@media (min-width: 300px) and (max-width: 460px) {
  #container {
    width: 100%;
  }
  ...
}

Since there is no any media query to get triggered about screen width less than 300px, so your page goes broken again.

Fix is easy, since you don’t have anything less than 300px, you may let the last media query gets triggered size less than 460px and it’s fixed! Simply remove the (min-width: 300px) from media query, and it’s okay!

I also suggest let the title “Fashion” is smaller when in mobile.

I see you set font-size with pixel unit(like 110px for h1 tag), please don’t, use em instead. If you read the article I shared again, it has a section about it with detailed explanation.

Good progress, keep going on greta work.