This is my first project on Frontend Mentor and I am very curious what people think of my code. Thanks in advance for your reply!
Christopher Till
@chri55All comments
- @iAmKarinSubmitted about 4 years ago@chri55Posted about 4 years ago
Hi Karin, very nice work on the design!
One aesthetic suggestion I would recommend is putting
cursor: pointer
on the.button
class instead of the containeda
, this way the user knows the entire button is clickable.As for your code, very nice. You used media queries which is good, but something I'd do is make one large section for a media query at the end or beginning of the file, since it seems you wrote the same media query over again multiple times. It'll let you write less code overall! See this code on my GitHub for an example of the structure of a larger media query section. Don't mind the content, as its from a different challenge.
Keep it up, happy coding!
1 - @tarasisSubmitted about 4 years ago
Is there a way to say use X width in small displays, and Y width on larger displays? I tried various combinations of width: clamp/min/max/min(max())/max(min()) in an effort to avoid doing it in media queries but couldn't figure it out (using width: 90vw for < 600px, 75vw for 600-1050px and 50vw above that).
—
Why does my input field for email turn blue when you start to enter something? AFAIK I don’t set that.
--
Slightly irked that despite having the desktop image match before submission, they don't here. I assume its because I used
margin-top: auto
for the footer section, rather than specifying a specific size.—
Is there a better way to handle the hiding / showing of the error message? The input field, the error message and notify button are in a flex column (mobile) or row (desktop). The error message div is set to take up zero space so it doesn’t affect positioning of the other two items, and when in a row I use flex ordering to put the error message first in the row.
Then when an error occurs I apply a css class that moves the message’’s location down. (And when in column, I push the notify button down like the design shows). It works, but feels hacky.
@chri55Posted about 4 years agoHey Robert, I saw your updated question about the error message placement. In looking at your code I saw what you meant by transforming the button's location when the error message is shown.
I think your solution fine for desktop, where space is ample but on mobile
transform
could become weird at any point, or on any device/browser. So I replicated a quick example of your form on CodePen and modified some things so that no transform or opacity is necessary on mobile. Take a look and let me know what you think, I left some more detailed comments about what I did in there.1 - @kulczynskiSubmitted about 4 years ago
How to write better js and sass code? Why my deployed app doesn't work correctly?
@chri55Posted about 4 years agoHi Tomy,
If you look in your developer tools console after searching an IP, you may see an error which says "Mixed Content": your
fetch
call is getting data overhttp
, but the app itself is being served onhttps
through Vercel. Change your URL in thefetch
call to usehttps
and it should at least be able to make the call to the API once deployed.0 - @tarasisSubmitted about 4 years ago
Is there a way to say use X width in small displays, and Y width on larger displays? I tried various combinations of width: clamp/min/max/min(max())/max(min()) in an effort to avoid doing it in media queries but couldn't figure it out (using width: 90vw for < 600px, 75vw for 600-1050px and 50vw above that).
—
Why does my input field for email turn blue when you start to enter something? AFAIK I don’t set that.
--
Slightly irked that despite having the desktop image match before submission, they don't here. I assume its because I used
margin-top: auto
for the footer section, rather than specifying a specific size.—
Is there a better way to handle the hiding / showing of the error message? The input field, the error message and notify button are in a flex column (mobile) or row (desktop). The error message div is set to take up zero space so it doesn’t affect positioning of the other two items, and when in a row I use flex ordering to put the error message first in the row.
Then when an error occurs I apply a css class that moves the message’’s location down. (And when in column, I push the notify button down like the design shows). It works, but feels hacky.
@chri55Posted about 4 years agoHey Robert, well done. A couple answers to hopefully point you in the right direction:
-
I think I read your question right, you said you tried to use a media query with
vw
in place ofpx
. Thevw
andvh
units (like rem and em) are relative units, and they are relative to screen size rather than absolute, so it's always best to use an absolutely defined unit like px to ensure the media queries work. -
I believe some browsers set the
outline
property by default when an input field is clicked on, try adding a state rule for:focus
on the text box and give itoutline: none
and see if it works. However this is not recommended in production as visual impaired users need that outline to see what they have selected.
Great work, hopefully I answered some of your questions, and good job matching the design!
1 -
- @simonhernandezSubmitted about 4 years ago
Hello Guys!
Feedback is more than welcome! :)
@chri55Posted about 4 years agoVery nicely done, looks great on mobile and your code is neat, concise, and easy to read. Great work!
1 - @Aryan-desaleSubmitted about 4 years ago
How's the site?
@chri55Posted about 4 years agoLooks great on mobile, good job with responsiveness!
The tiny suggestion I have would be to add the box shadow to the card itself, and the sign up button, but that's just a very small detail. I feel it helps solidify the "card" look and make it more obvious to the user. It also helps the white to stand out against a light grey background.
1 - @Prajwal518Submitted about 4 years ago
I'm open any suggestions or guidance..
@chri55Posted about 4 years agoHey, very nice on the layout!
A few suggestions, try to get the paragraph text under the h1 to be a little darker, as it's hard to read on the light background. Same with the background of the star cards, it's a bit too dark to read the text. Try experimenting with some of the colors in the style guide.
Also, a little bit more padding on the dark magenta cards would be nice, to space them out a little bit.
Good work so far!
0 - @steve271974Submitted over 4 years ago
any feedback would be appreciated keeping things professional is all I ask . I do believe that I have everything pretty close to where it should be.
Thx Steven
@chri55Posted over 4 years agoHey Steven, nice work! Don't forget to add commas in on the
font-family
property when specifying fallback fonts in CSS. Example:font-family: 'Poppins' , sans-serif;
Also, you can utilize the
* { }
css selector at the top of your file to define a base font that will apply to all elements (this is typically recommended) so that you don't have to keep repeating thefont-family
property throughout the code, and only need to change it when specifying things like headers that have a different font type.0 - @stoefySubmitted over 4 years ago
First HTML CSS Exercise, would love some feedback! Like: Wondering if this is the correct way to make it responsive. If I could improve the splitting up of elements. Make it more readable.
@chri55Posted over 4 years agoHi Stoefy,
You have the right idea so far, where the elements stack once they hit the media query size. Good work on that! You also did a great job at matching the design colors and styles, and your use of great semantic HTML is a plus. One thing I would say to improve readability would be to split up the long
<p>
line with lots of<br>
inside. But otherwise, was quite clean and easy to read.I suggest doing some research into Flexbox.
For this I recommend going through the side of Parent Flex Container first as it goes over the essentials of FlexBox. It is a bit different from the
flex
property that you used in that it allows a lot more options for alignment and justification of items inside the flex container. It's also a really great way to make a component become centered inside of the container or inside the page, like it is in the design.You may also notice that it could be useful for mobile, as flex children that are stacked in row format on desktop can be moved to column format on mobile without having to move too much code.
Check it out, and let me know what you think! Happy coding!
1 - @BraianGazanoSubmitted over 4 years ago
Like i said, this is my very first page with responsive, so I accept any suggestions and recommendations on changes thanks!
@chri55Posted over 4 years agoHey Braian, very nice!
Between about 850px and 640px, the text in the bottom columns tends to get a bit cramped. When this happens, but I still want the page to be in "desktop" mode more than "mobile" mode, I let the outermost container take up some more space. I'd recommend using a short media query of max-width 850px to make
.principal-container
a bit wider and you will see the space of the bottom columns free up a bit and look nicer.Well done on the responsiveness, it looks good in both modes!
2 - @thomaspalopoliSubmitted over 4 years ago
I didn't know how to make the code fit also for the mobile layout. If you can help me with that I'd really appreciate !
@chri55Posted over 4 years agoNice work, Thomas. If you were to use a media query to change the flex direction of .boxes to be column at a certain point (at and below 750px would be my estimate), you can stack the boxes nicely. Then the inner boxes can also be allowed to take up more width.
I also liked your solution for adding the tops of the boxes using a border color. Well done!
1 - @rodrcastroSubmitted over 4 years ago
Since this is my first ever challenge - and real world project - any feedback is invaluable. I struggled a lot and jumped through a ton of hoops to get to this final design, and I know it's not super close to the design, I also couldn't get mobile to work at all.
That said, I wanted to learn what I could have done better in structuring my file and getting the components right, specially to get mobile working.
@chri55Posted over 4 years agoHey Rodrigo, Very nice solution. You've matched the desktop design very closely. For mobile, I would recommend using flexbox ( https://css-tricks.com/snippets/css/a-guide-to-flexbox/ ) on the bottom 2 containers, coupled with media queries, so that once the screen is small enough, the containers will stack into a column shape. I also think using a bold font weight on the sign up button would increase contrast and make it easier to see. Well done, and happy coding!
2