I started learning due to the lock down following a course or 2 on udemy.
This is my first challenge on Frontend Mentor so any feedback would be good.
I had to focus quite a lot on learning css grid and making the design responsive to complete the challenge.
I'm still unsure on text scaling and how that works with responsive designs so any insight would be good.
Many thanks,
Hey Paul :)
Your solution looks great! After a quick look, I found couple small things.
I noticed that you have headings h3 being smallest and h5 being largest. I don't think in this design there is need for more than 1 or maximum two headings - h3 (h4). Generally, you shouldn't chose headings based on their style (color, size, weight). Now, in design you come that across quite often, but not always. I think in this case it makes more sense to stick just with single h2 element, one for each section inside the card.
From you CSS file.
Just an opinion, but if you put color values inside css variables, I think it would make sense to make white color a variable too. For example line 72, you have #ffffff as color value. I would put it up with the rest of color variables at the top.
I noticed you removed outline for button element. In general you want to avoid that. Or if you do remove outline, then add some styles inside :focus selector for button. That way if you are using keyboard to tab through your page, user will know when their focus is on the button.
For some reason you have two box-shadow properties inside .container selector. line 33 is overwritten with line 38.
You could simplify your layout and reduce CSS file by applying mobile first approach. That is, keep all the card sections display: block; as they are by default because of div elements. They will naturally by default stack on top of each other. And then you can apply grid only inside media query on larger screens, where you need it. Note that you would need to change max-width to min-width inside your media query.
Answering your question about responsive text scaling. You have couple options. You can simple change the font-size property inside your media query. That's probably the easiest, simplest and most predictable way. Almost always I do it this way.
You could also look up some fluid font size scaling with % units. I haven't used fluid scaling much, but it has some uses, depending on how you want your responsive design to work.
I hope I cleared up some things. If you have any questions, feel free to ask here or on Slack.
Nice work, keep it up! :)
I have a tiny issue with the top border of the cards on table port.. I couldn't find the solution for that. If you can explain the possible solution would be nice. thanks
Hey Ruben.
The issue occurs because you are applying border to ::before pseudo element, leaving the background color transparent. Adding background-color property with the same color as border fills in that empty gap.
I would remove border and border-color properties completely and just add background-color with specific height. It will give the desired effect without any issues :)
Is my SCSS efficient? How browser-compatible is my code? How could my HTML be more semantic - have I used the HTML5 tags correctly?
Hey Natalie :)
Your styling with SCSS in general looks great. There are some inconsistencies with variable names. Neutral colors are PascalCase whereas rest are camelCase. Also, it's not necessary to put tag selector in front of class selector for sections.
As for semantic tags and accessibility. Only thing I noticed are alt attributes for icons. You should have some meaningful text that describe the icon itself. Or if you want to leave as purely decorative and not for screen readers, you could leave alt attribute empty.
Otherwise everything looks good. Nice work :)
Hello Everyone This is my first project "Please tell me how I can improve” please check the project and give your valuable feedback .
Hey Manish, solid solution and looks great!
Having a quick look I noticed some small things that could be improved.
First, I would rethink how you structured your heading element. In general, you want to avoid skipping heading tags. For example you have h4 after h2 in .item1. I think it would make more sense to use only h2 and h3 or even just h2. For Join our community, Monthly Subscription and Why Us leaving rest as p or small elements. You shouldn't choose heading elements based purely on their color and size. Most often yes, but not always.
Another thing, you can try applying styles with "mobile first" approach, I think it would reduce your CSS a bit :) Continuing about responsiveness, you should extend breakpoint, because right now on smaller screen sizes, applying desktop styles, text inside two columns at bottom is really narrow.
Other than that, nice work. Keep it up! :)
Hey everyone, I worked hard on this challenge. I'm uploading this second time as the first one doesn't look at how I want to make it. Let's see what I got now, If you have any suggestions or recommendation or anything which can be modified, please tell me, I'd love to hear from this amazing community. Any suggestion on how I can fix this HTML issue
Hey @Adarsh.
The design is spot on! The crazy animation on page load made me laugh:D
You did not use any SASS or CSS Modules or any other css compilers, right? I see that you css is 1000+ lines of code, so I would probably think about splitting it into several smaller files. I like that you at left some comments on different sections inside css file :)
About your HTML issue, you are missing an element with id #subscribe-btn. In the repo the input element has aria-labelledby="news-heading" though.. But the aria-labelledby looks for the element with id of the same name.
Hi, I just wanted to know how to integrate Zeit Now to my repo. I've tried to import this project through my Zeit Dashboard but it's not working. I was forced to use Github Pages.
Hey! Essentially it doesn't really matter how you deploy your solution :) How did you try to import the repo in Now dashboard? Matt just couple days ago added a compete guide for submitting solutions including some instructions to deploy it on Now.
https://medium.com/frontend-mentor/a-complete-guide-to-submitting-solutions-on-frontend-mentor-ac6384162248
Looking back, i would definitely use grid instead of only flexbox, wanted to have some practice with flex though. Thanks for looking!
Hey @Michal!
You solution looks good :) I think it's great that you chose to use Flexbox in most parts. I actually would have used Flexbox too :)
Just quickly looking through.
I noticed you have justify-content: flex-end; on header. You don't need it if you have margin-right: auto; to logo :) Or you could remove margin-right from logo and leave justify-content.
Nice work, keep it up :)
I'd like to have some specific feedback on spacing, I think I did too many margins and paddings on classes. But I'll be glad to have general feedback also. Many Thanks!
Hey @Alan!
Design looks pretty good! For spacing I use browser extension called PerfectPixel. With it you can overlay the original design image on top of your website, change opacity, toggle it on or off. It is pretty great tool specifically for making "pixel perfect" design. As you practice more and more you should be getting better and more consistent with guessing spacing :)
Hi all,
Please be so kind to let me know how I can further improve my coding skills.
Kind regards,
Frederik
Hey Frederik! What exactly would you want to improve and focus on?
Feel free to leave any comments!
Looks really good and spot on!
Couple small things I noticed.
For card buttons, I would use actually button element:) Wanted to tab around with keyboard and couldn't:( Also, I would add some :hover and :focus styles for toggle at the top for the same reason.
One other thing, between screen width 861px and 1010px the cards are right against the side. Maybe put some padding around:)
Otherwise, nice work:)
I'd like a feedback regarding the code, as this is my first website and I'd like to know if I wrote a "clean code". I know that my attempt to make it responsive was a fail, but I'll study more and I will fix my mistakes in my next project. Also, any suggestions or opinions are very much welcomed.
Hey Mihnea!
The project looks nice, especially for the first website. Good work. Couple of things.
First, as you see in your report there are some issues with img elements missing alt attribute. You should always provide alt attribute:) If the image is purely decorative (in this case there are none) you can leave the alt attribute with empty string like alt="". That's totally fine, but it's good practice to always include it.
Keeping on semantic HTML topic, there should not be two h1 elements on single page. h1 element represents the main title of the page and it only makes sense to have one main title per page. If you need to break the title in several lines, as in this case, you can put <br> between the words you want to break.
One other thing you could do is to change <div class="flex"> to <main> element as it is the main content of your page.
I also can see that you have quite a few unused css classes (e.g. .flex and .container) and unnecessary wrapper elements. Try to get the result with as few elements as possible and add them later if necessary.
As for "clean code". I notice there are some inconsistent tabbing inside HTML file. Inside css file, you have indentation set to 4 spaces instead of 2 as you have inside HTML file. It's good practice to stick to either 2 or 4 spaces and be consistent though project, even in different files. Also inside css file, there are a lot of inconsistencies regarding spaces before : and {. In some places you have space before/after, in some you don't.
For these issues you can look into file linting and formatting:) You won't need linting much for this case, but file formatting using for example Prettier is definitely very useful and keeps your code clean automatically once you set it up;)
One side note, it's good practice to have your css file be the last <link> inside <head> element. In this case it wouldn't matter because you are only getting fonts and icons, but it's good practice, if you have external css files (not yours), they should be before your ones:)
Aaand finally you should have atleast :hover and :focus for the button;)
These are the major things I picked up on. Keep it up!:)
