-
Notifications
You must be signed in to change notification settings - Fork 471
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Project business site #362
base: master
Are you sure you want to change the base?
Conversation
…alled placeholder
…e code more understandble to me
…op of the CSS code (Jennie's advise)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job with this project Johanna, it's well structured and I appreciate reading your reflections in the README.
HTML
- Great job using the required attribute on the inputs. This ensures that users cannot submit the form without filling out these fields.
- Great use of semantic elements, such as main and footer (just make sure the footer is within the body element). Consider using a header instead of a div for a more semantic approach.
CSS
- Excellent use of max-width: 100%; and object-fit: cover; for images. This ensures that your images scale correctly and maintain their aspect ratio across different screen sizes.
- Make sure that the form elements (inputs, buttons) are styled to match the overall aesthetic of the page. Aligning the name inputs would be nice! Reach out on Stack Overflow if you want tips on how to achieve it 😊
- Your CSS seems well-prepared for responsive design, but make sure that all elements, especially the form, adjust properly on smaller screens. You may want to use media queries to fine-tune e.g. the font-size of the placeholders for smaller screens.
Clean Code
- Make sure to properly indent your code, each new element should have it's own line:
<label>
<input type="radio" name="Agree" required>
Agree
</label>
and even better, if each attribute gets its own line to improve readability:
<label>
<input
required
type="radio"
name="Agree"
/>
Agree
</label>
You’re doing a fantastic job! Just a few small tweaks, like refining the radio button setup, will make your project even better.
Keep up the great work 🎉
The biggest issue with this was that I found it hard to understand/read the code in the inspector, so I had to google a lot. How to add the text inside of each input field ("placeholder"), how to add space around the boxes, etc. The form also contained things I couldn't understand, because they weren't visible to the eye, such as "<label for="frm_email_3">Om du är mänsklig, lämna det här fältet tomt.</label>". | ||
|
||
3. Minor layout issues. | ||
The name fields are not positioned the way I wanted, the radio button is oval and can't be unchecked, etc. Since time was running out, I started to write PostIts with all the flaws I wanted to correct, added some in a pile that need to be fixed before handing in my project (upload to Netlify, make the site responsive for different devices, etc), and some notes in a pile to fix later, if I have the time (like the oval radio button). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's the behaviour of radio buttons, it's a group of buttons and you always have to pick exactly one, i.e.
Are you a:
- Lunch Learner
- Early Bird
- Evening Owl
We know that each user completing our form will be one - and one only - of these options. For checking/unchecking, it's better to use a checkbox. Checkboxes can be grouped as well, but for when the users can choose more than one option, i.e.
Are you interested in:
- Frontend development
- Backend development
- UX
- UI
No description provided.