FCC: Survey Form-Feedback

Hello everyone, I’d really appreciate some feedback about my attempt to this challenge :slight_smile:

Thanks !

Pretty neat!

Try increasing paddings for your select boxes.
Spread your radio boxes.
Notice a scrollbar is there horizontally.
Make it responsive.

Good luck!

Hey I totally forgot about responsiveness lol. Thank you very much, I’m working on it

just want you to know ahead of time that my use of all caps are “emphasizing”, not “yelling” :slight_smile:

Someone mentioned making it responsive already so I’ll let you finish addressing that to see if you catch the things I saw. Aside from that I took a look at your code and saw this interesting chunk of CSS

html, body {
  background: #171819;
  color: whitesmoke;
  font-size 1.2em;
  margin: 0;
  padding: 0;
  /* photo from unsplash */
  background: url("https://images.unsplash.com/photo-1501390660742-c95d2b9d31f7?ixlib=rb-0.3.5&ixid=eyJhcHBfaWQiOjEyMDd9&s=9d00daf8e3fe7fe7c2d4fb5b98438314&auto=format&fit=crop&w=1534&q=80");
  background-repeat: no-repeat;
  background-size: cover;
  background-position: center center;
  font-family: 'Quicksand', sans-serif;

body {
  background: none;

What led you to doing that? The <html> tag doesn’t get styled because nothing outside of the <body> is visible on the page anyways. Then for you to define both the <html> and <body> tags in that same chunk THEN the body all by itself with a background of none after setting it to use a photo, seems like you were a bit confused and it eventually started working “somehow” so you just left it alone lol.

The solution to that would be to just delete the body{ background: none; } part and delete the html off the other part and just leave the body tag on it. If the results of your page changes that means you have something funky going on somewhere else that we need to resolve. Now that I think about it you have margin: 0; padding: 0; in there. If that roots to how you wound up with this code to get things to show up the way you expect them you can just extract that part out and do this *{margin: 0; padding: 0;}. The * in this instance targets “all” which means the entire document like you were most likely trying to achieve by setting it to the html tag in the first place.

Another thing I noticed in your CSS is your media query uses a max-width property. It works, but as you work on projects that need more intervals to shift through it forces you to have to keep working towards “smaller” which can make you double back to the “bigger” states putting your “default” styling somewhere in one of the “middle” states… if that makes any sense. Basically you’ll have media queries that represent states both smaller and larger than your “non media query” code that comes before them.

I personally start down at around 260px wide as my default size as there are now smart watches that are commonly 280px X 280px. I don’t know how common it actually is to view stuff that small but if you can get everything aesthetic and properly spaced at THAT size… it’s easier to find ways to take advantage of alllllll that extra space as it gets bigger while seeing how you don’t need to take up ALL of it just because it’s there because of how clean and organized everything already is, then it’s just a matter of using “white space” aesthetically and balanced and maybe adjusting font sizes a little. Another plus side to it is if for some reason your media queries wind up not working on some browser or another for some reason or another, the smaller size as the “default” size will balance out and be more intelligible to a user than that largest size being defaulted on anything that’s not a 60 inch or however big you go.

… I just got to your HTML… that’s the problem, you’re including the ENTIRE html code as if you’re doing it in your own code editor lol. Codepen and other online editors start you off INSIDE THE BODY TAG lol. So you can get rid of all that and just start with your header tag. You can still keep the styling for your body tag in the css though as it’s still effective and useful.

You’re also using several <form> tags, which even though it works, really isn’t what you should be doing. You’re also giving them all the same id name which you should NEVER do. The purpose of an id is to be unique to that one thing. Right now with simpler projects it works, but as you get into bigger ones and you do indeed want one individual thing out of possibly literally millions of things… that’s how you get that one thing. For styling multiple things the same you could make a class and apply that same class to everything you want it to effect.

The reason I’m saying your use of <form> tags is wrong is because in this particular instance you SHOULD essentially want all of those elements to be part of the same form, not broken into their own individual forms. If you have one submit button, which one gets submitted?.. or if you apply the same method to each one… which one is going to be THE data that gets submitted while none of the others do because it keeps overwriting itself IF it doesn’t get confused and freeze?

So you should make one <form> tag and maybe use <section>s and <article>s for all the chunks you have in the other <form> tags. You can keep the same styling again as a CLASS and apply it and it should still work and look the same.

I also encourage you to make a huge bowl of popcorn and veg out on youtube for a day or two watching videos of how people make use of CSS Grid. It makes this project soooo much easier in terms of making use of available space and switching the entire placement of everything with simple media queries. It also has inherent align, justify and gap features that you can apply and eliminate a good 90% of the wizardry we need to perform with margin: 0 auto; and combinations of width, min-width and max-width settings just to get something to show up where we want it and stay there.

That’s about all I can really say at the moment. You got something together and it works which is the important part, you just have to now refactor and clean it up to bring it closer to being what would actually be used in the real world.

1 Like

Thank you very much for the time you put into this review and for the precious information, it helps a lot!!
So, actually I’m very very confused about the form tag. I keep reading about it on MDN and other sources but I can’t quite understand the proper way to do it yet. I thought I had to connect all the forms with the same id to the submit button for validation… here we go again to MDN lmao

seems like you were a bit confused and it eventually 
started working “somehow” so you just left it alone lol.

that’s exactly what happened :laughing:

You got something together and it works which is the important part,
you just have to now refactor and clean it up to bring it closer 
to being what would actually be used in the real world.

great, I’m definitely going to work on it now. thanks again!

You have the proper mechanics down for making the <form>s… in fact you have them down so well you made several of them perfectly lol. The first form has 3 text inputs that work, the next form has a dropdown that works, the next form has radio buttons that work, though now looking at them I’d suggest separating them with wider gaps or pushing them to new lines because they’re a bit confusing in terms of which one selects which answer because usually they’re to the left of the response, which is now getting into UI/UX stuff in terms of what the users expect intuitively. But you have a grasp on how the elements are suppose to work, you also have a grasp on the CSS to separate and style each section of the form, so all the bits are already there. You might want to start a new pen and just copy chunks from this one to fill in on the newer more structured one as you go, it will help you keep track of everything a LOT easier. If something goes off some kind of way you won’t have the entire thing to scroll through to find the bug.