Design comparison
Solution retrospective
Any feedback is highly appreciated!
Community feedback
- @ApplePieGiraffePosted almost 4 years ago
Hey, rafet! 👋
Nicely done on this challenge! 👏 As the others have said, your solution looks great and responds well! 👍
I only suggest,
- Adding a
max-width
to the main container or wrapper so that the content of the page doesn't become too wide on extra-large screens. - Perhaps hiding the default error message that gets displayed when an invalid email is submitted in the form near the bottom of the page so that the custom error message you added can clearly be seen. You also might want to add
cursor: pointer
to the submit button of the form. 😉
Keep coding (and happy coding, too)! 😁
1@rafetbasturkPosted almost 4 years ago@ApplePieGiraffe Hello! 👋
I set
max-width: 1440px
to body. I have addednovalidate
attribute to form andcursor: pointer
to button.Thanks for your critical suggestions.
0@ApplePieGiraffePosted almost 4 years ago@rafetbasturk
Just took another look, and the changes are good! (You just might want to make sure that the extra space around the
body
on extra-wide screens has the same background-color as thebody
). 👍 Keep it up!0@rafetbasturkPosted almost 4 years ago@ApplePieGiraffe
Is using
div
for main wrapper to change the extra space around the body suitable? Does it ruin semantic structure?0@ApplePieGiraffePosted almost 4 years ago@rafetbasturk
Hm... I think it'll be okay. Just give it a sensible class name or so. 😀
1 - Adding a
- @dpayne713Posted almost 4 years ago
Hi Rafet,
Your solution looks great! I'm having trouble finding any issues! nice job!
Take a quick look at your accessibility and HTML issues when you have a chance.
To assist in future projects, especially more complex projects, I'd recommend using class names for everything and looking into Block Element Modifier naming.
https://css-tricks.com/bem-101/
It really helps me keep track of code and also is very helpful while using SASS / SCSS nesting.
Keep it up!
David
1@rafetbasturkPosted almost 4 years ago@dpayne713 Hi! Thanks for your feedback. I will try to use BEM from now on.
0 - @axevldkPosted almost 4 years ago
Hi, @rafet~ I have studied your work, and it's really great ~ 👍 Here are some of tiny opinions on your work.
-
I hope you to add
background-size: 100%
to header element so that it fills whole area above 1440px. -
Semantically, section elements must have heading tags.
Everything else is perfect! Happy coding ~ ✨
1@rafetbasturkPosted almost 4 years ago@axevldk Hi! Thanks for your comment. I used "contain" for background-size. I will try to be more careful about semantic elements. Thanks again. Happy coding.
0 -
Please log in to post a comment
Log in with GitHubJoin our Discord community
Join thousands of Frontend Mentor community members taking the challenges, sharing resources, helping each other, and chatting about all things front-end!
Join our Discord