I think i did pretty well in this challenge, but i would still like some feedback on things i could improve on.
What challenges did you encounter, and how did you overcome them?None so far in this challenge just waiting for some feedback
What specific areas of your project would you like help with?Recommendations on my code too improve are very open of course
I don't see many issues and it's a nice solution. Here are couple of suggestion.
header is usually for content that repeats across multiple pages on a site. Usually it's positioned at the very top of a page and contains the the company/brand logo and the navbar. The content you have put inside the header is specific to this page. So i wouldn't use a header here. Also a h1 being inside the header doesn't sound right either. I would include the h1 and that p in the main and remove the header.padding-top you are using on the section called four-card-feature should actually be a margin. Generally padding is used to get the spacing between an element's edges and it's content. margin and gap is what you use to get the spacing between two sibling element. What you have done is not wrong, but i think it's a good mental model to have and follow.Looks nice, i have couple of suggestions.
width: 100vw for the body. Body is a block element, and by default block elements take full width of the screen.card. If some more text were added to the card, text will start to overflow. Because the card can't grow it's height past the fixed height you have defined. You don't need to specify a height at all. The browser will calculate the height based on the content inside the container and things like font sizes and padding etc.div and span doesn't have any meaning. They are good for layout and and styling purposes. The label and the date should be p elements. Same way the user's name should also be a p. It's fine to have text inside a span, but the span must be inside a meaningful element like a heading or a p.alt=""). But if you feel like it should have an alt text, it's fine.a element inside that heading. So in a real site, when the user clicks on it, they will be taken to the actual blog page. That is the purpose of this card component. Picture it like this, there will be multiple of these cards on a page. Clicking on each card, should direct the user to the relevant blog post. For that to happen you need to have an a element inside the card.Nice solution. I have couple of suggestions.
height for text containers. In your card__content class you are using a fixed height. If some more text were added to the children of that element, text will overflow. Because of the fixed height the container can't grow freely to fit all the text content. There's no need to specify height for text containers. The browser will calculate it for you based on the content inside the element, and stuff like font size, padding etc.height: 100vh on main. Use min-height: 100vh there, so your main can grow if needed.img tag, so that i can give it an alt text like "QR code leads to frontendmentor.io". Currently if a screen reader user comes across this component there is no way for them to know that there is a QR image present in this.But yeah, other than that, it's a nice solution.
The custom font configured in the project is not displaying correctly on the deployed site, even though it works fine in the development environment.
.css file when referencing the font file. That should fix your issue.height to your grid, which causes text to overflow when the screen gets smaller. You should never limit the height of containers that contain text.I was able to write some CSS! As someone with very strong background in backend development, cloud and devops, this is a nice change of pace.
I would definitely write CSS in more scalable way next time. I was aiming for the best replication of the design, so if I wanted to scale the design a bit, I would need to improve the solution (e.g. less specificity for texts, etc.)
What challenges did you encounter, and how did you overcome them?Reading design file was a bit of a hassle at first, reading Figma without DEV mode is clunky. I'm not gonna pay for Figma unless I actually use it for real project.
What specific areas of your project would you like help with?I think that further down the line, spacing, gaps and layout will be a tricky to do right without frustration.
Looks good. You have avoided lot of mistakes everyone does on this first challenge. Here are couple more improvements you can do.
h1. In a real site this card would be placed in a page along with some other content, so it's unlikely the card heading would be the h1. It would probably be a h2 depending on the context. But in the given scenario you can use a h1 for it.<main> element, wrapping the main content of the page. In this case your div with the class .container can be the <main>, instead of being a <div>. Some other landmark elements would be <header> and <footer>. Both are usually for content that repeats across multiple pages of the site. <header> usually contains the company logo and the navbar and is positioned at the very top of each page. <footer> is where the attribution stuff go and is positioned at the very bottom. So your attribution should actually be inside a <footer>. You can change the div with the class .attribution to be <footer>.I'm proud that I have started my challenege path and would love to learn most out of this. Next time I would use different approach to solve the same challenge. I have used plain CSS in this challenge. Next time I would prefer using Tailwind CSS.
What challenges did you encounter, and how did you overcome them?There were no challenges I faced, since this was a beginner level challenge.
What specific areas of your project would you like help with?I would like to know if my approach and design principles were good enough to be applied in real projects.
Couple of suggestions.
divs. divs are good for structuring things and help with layouts, but div itself is not a semantic element, so it doesn't really have any meaning.h1. Usually this entire card will be placed in a page along with other elements, so in that case this won't likely be the h1. The h1 would be some other element that explains the content of the entire page. But in the given scenario you can make it a h1.<p>.rem unit for font-sizes. px units won't scale with the browsers font settings, so if the user who visits your webpage have changed their browser default font sizes, they won't be reflected on your page. rem on the other hand respects the browsers font settings and change accordingly. So always use rem for font sizes.height. It will cause overflowing if the content inside the card changes. You don't have to specify the height of text containers. The browser will automatically calculate it for you based on the content inside the card, and other values like font sizes, paddings etc.width either. When a fixed width is used the card can't shrink past that value, even if the screen size is small and the card doesn't have any space to fit in. That again will cause overflow issues and break your site. Instead use a max-width on the card. So you can control how large the card is allowed to be, but at the same time the card has the freedom to shrink automatically when there is no room to fit in smaller screens.ids and classes when it comes to styling. Always use classes for styling. Mixing the two will make you run into specificity issues. You can use ids when linking two elements together. For example linking an input element with the label element, or when using aria-describedby.<main>, except the content in <header> and <footer>. So the <div> that you have given the id="card" should actually be a <main>. The attribution should ideally go inside the <footer> and should be outside of the <main>.position: absolute on the card. Also i don't see a reason for the .card-body div. You can directly use the .card div. Try to keep the HTML simple.height: 100vh to the body instead of giving it to that div. And make sure you use min-height and not height. Never limit the height of the body or any element that contains text.Looks good, i have couple of suggestions.
flex or grid on the parent element, which is the body element. You will have to give a the body a min-height: 100dvh. Giving large margins in order to center stuff is not really correct.max-width on the grid.width for each card. Avoid giving fixed widths for cards that contain text. Because of the fixed width, the card can't shrink past that fixed width even if there is no space for the card to fit in smaller screen sizes. Instead use a max-width. That way the card's can shrink without any issues. That being said you don't have to specify the max-width on individual cards. Instead you can control the width using the grid. Because all cards are contained inside the grid, you can use the max-width on the grid.h1). Then it will be h2 headings, and then h3 etc. You have a used a h4 for the testimonials which is not really right. Also i don't think the testimonial should be a heading of any sort. A heading should be something that explains what comes after it. There is nothing that comes after the testimonial texts. I think the users name is the ideal heading for each card. Because all the content that comes after it sort of belongs to that user name element. It's the user's verification status, user's testimonial etc. So i would make the user name of each card a h2. Why not a h1? Because ideally there should be only one h1 in a page. In this design there is no visible h1, so you can use a visually-hidden (also known as sr-only) element for that.blockquote element for the two testimonial texts on each card.I practiced using a mobile-first approach. I practiced using Figma to get the design specs.
What specific areas of your project would you like help with?
clamp() CSS function to change the font size based on the screen size, instead of using media queries.Couple of other suggestions.
rem for font sizes. Currently in your variables you have defined the font sizes in rem, but on the body you are using px.width, use max-width for the container size. When you use width the container can't scale below that size even if the screen sizes shrinks. This won't be very obvious in this specific situation since in this case the width value is around 330px, but still it's better to get into the habit of avoiding fixed widths for containers that contain text. So instead use max-width, which allows the container to scale down when there is not enough space in the screen to fit.HTML & CSS element should be the heading of this card. Not a p. You can use a h1 for that. On a real site it won't probably be the h1 since there will be multiple cards like these on a page, and the h1 of the page would be something else that describes the content of the whole page, so in that case this would probably be a h2. But in the given scenario this can be the h1.<main> landmark element. It should wrap all the content of your page except the contents of <header> and <footer>. In this project you don't have a <header>. But you do have a <footer>. The content of the attribution should ideally go inside a <footer> and it should be outside of the <main>. So it will look like this.<html> <head></head> <body> <header></header> //Not relevant in this project <main></main> <footer>Your attribution content</footer> <body> <html>
What I’m most proud of:
What I would do differently next time:
Challenges I faced:
Password Generation Logic:
The most challenging part was designing a solution to generate passwords while handling various options, such as lowercase, uppercase, numbers, and symbols.
How I overcame it:
I came up with the idea of generating an array containing random characters for each setting. However, I spent some time researching how to toggle these options effectively, which helped me refine my approach.
Custom Range Slider Styling:
Styling a custom range slider was quite difficult.
How I overcame it:
After conducting research, I found an article with a solution similar to what I needed. I adjusted the code to suit my project, which allowed me to style the slider successfully.
Code Review:
I believe feedback from more experienced developers is the best way to improve my skills and get closer to the job I’ve been dreaming of for so long. Please let me know if way that I was write a code is good enough for working as a developer, and how much time such a project should take according to the real life job?
State Management:
I used a lot of states in my implementation. While this approach makes the code clear and understandable for me, I wonder if it's the best practice or if there’s a better way to handle it. I’d love to hear suggestions on improving state management in my project.
Hey nice solution. Couple of things i noted.
word-break: break-all on the password text. I see you are using overflow on it, and it's currently causing a scrollbar to appear next to the password. You can remove the overflow and use break-all instead. I think it will look better.hanging which i am not familiar with. However that is causing a weird result i am not sure how to explain but you can see it your self by tabbing into the button using the tab key. I think instead of hanging you may have wanted to use height.Strong strength password. Pushing the scrollbar to the very end and checking all 4 checkboxes i am still getting the strength as Medium. You can either let the user chose more characters or tweak your conditions so that a 20 character password is Strong. I feel like a password with 20 characters and all 4 of those options selected should definitely be marked as Strong.Hey, your solution looks good, however i have few suggestions.
input type="text", you can use input type="number" for the bill, custom and people count. That way the user won't be able to enter any text entries at all. You can still do your validation in JS, but as a first layer of filtering i think it's far better to limit what the user can enter into the input fields.bill, number of people should actually be label elements and their for attributes should point to the input element they are labelling. Currently you have them as p elements.alt attributes. alt="". They are decorative images so no need any descriptive alt text but still the alt attribute needs to be their with an empty string. Otherwise some screen readers may read the file path of the icon as the alt text.aria-live attributes. So when that element is populated with the error message it will be announced to screen reader users.input type="radio") rather than normal buttons. For this design you will need to custom style the radio buttons if you went that way. There are few ways you can do that. How i have done it is visually hide the radio button and style the label to look like radio buttons and based on the checked and focus states of the radio button i styled the label.main as well. Usually the logo image along with a navbar lives in the header landmark element but in this design i think it's fine to have it inside the main. Either way it should be inside an landmark element. Be it a header or the main. Currently it's not inside any.Looks ok but i have some suggestions.
Daily, Weekly, Monthly should be buttons. Those elements do something when the user clicks them. So they are interactive elements. You currently have them as <li> elements, which alone is not an interactive element. Because of this a keyboard user can't actually navigate through those 3 elements because they lack the interactive features a button or an anchor would offer.button, because it will do something when the user clicks on it. You currently have it as an image. Again, an image alone is not interactive so it doesn't make much sense to just have an image there.h1 and then other heading levels should be increased in order. h2, h3 etc.h3 and also the card hours as a h3. I don't feel like both of them are in the same level. The name would be a h1 because it acts as a heading for all the other elements in this page. Then the terms Work, Play, Study etc. would be h2s. Those words acts as a heading for what comes after them, which are the current hours and previous hours. So in an ideal situation the hours would be just a p. Because hours doesn't act as a heading for any content.<main> element in your page. It's a landmark element that wraps all the content in your page except the content that goes in <header> and <footer> (which are not needed in this design). You can look them up.max-width for the container wrapping all the elements, so that your cards doesn't keep expanding forever in larger screens. It doesn't look good when it's far stretched out. You can check it using the browser dev tools and increasing the screen size.cursor: pointer and a hover effect to the card. To a user, that implies the cards are interactive. But again you have not used any interactive element so clicking on them won't do anything and keyboard users won't be able to navigate into them. The card's should actually be a elements and in a real site clicking them would probably load a new page to show in depth details about the activities that were logged for that category.Daily, Weekly, Monthly are tabs. When the user clicks on one of them the corresponding tabs panel will be displayed. You can check the resource i have linked, it explains it further.As part of the learning paths, i was asked to give some feedback on your solution. Couple of suggestions i have.
onblur.I am really proud how smoothy the JS came along. I had been worried that it might take me a little bit longer. I would like to structure my css more. Sure, I could use SCSS or Co. but I didn't believe it was neccessary for this challenge.
What challenges did you encounter, and how did you overcome them?Well, the challange came with Netlify. When I run npm run build the JS file wasn't exporting into the dist file and so the function didn't work either. I did some testing outside then I realised I needed to add type="module" into the script file.
Any feedback is welcome!
Hey. Looks like this is a 4 month old submission but somehow in the learning paths i was asked to give feedback on this. Overall it looks good, but here are some suggestions i have.
<header> and a <footer> in this challenge. And even if there was, those two should not go inside the <main>. They should be outside of the <main>. This is just a component, so <main> should be the only landmark element needed here.<header> and <footer> both are usually used to wrap content that repeats across multiple pages in a website. <header> you usually see at the very top of a site containing the site logo and the navbar (site navigation). <footer> at the very bottom usually contains any attribution of the site, some contact info, a sitemap (secondary site navigation) etc.aria-label. That way screen readers can announce that name when a user navigates into that link. Otherwise the user won't know what that link/button will do since there is no text on it to be announced. You have already given an aria-label to the share button, you can do the same for the social media links as well.alt="".Responsiveness
Looks nice. Some suggestions.
rem for font-sizes. That way if the user changes their default browser font-size, the text in your website will scale accordingly. This won't happen if you used px for font-sizes.div or span alone. They are meaningless elements used for grouping or styling purposes. Change that quote text to a p element or better, for a blockquote element. You can use span if you want to select some text that is already inside a p.<a>) element inside those list items, because if a user clicked on one of those social media names they should be taken into that site. Otherwise having those social media names there would be useless.min-width. It should have a max-width if needed. Because of that massive padding and the min-width on the card, the card is not responsive in small screen sizes. Ideally a site should look good (responsive without any overflow issues) until around 320px screen sizes.body so that the card won't hit the edges of the screen.Looks nice. Some suggestions.
<main> landmark element wrapping the main content of the page.<footer> landmark element, which will be outside of the <main>.<h2> for the user's name here. It acts as a heading for the content that comes after it.rem for font-sizes. If the user change their browser font size, the text in your site won't react to it because your font-sizes are in px. If you have the font-sizes in rem, text in your webpage will scale according to the the users preferred browser font size. I would use rem for max-width too.div that wraps the <p> element which contains the user's bio? I think it's not needed. <p> element alone is fine. Try to keep the HTML simple.I am proud of the fact that i am getting comfortable using CSS grid. i am really proud of that, i tried using the mobile first design technique and i prefer that next time i would try to make use of Sass technique.
What challenges did you encounter, and how did you overcome them?Nothing much.
What specific areas of your project would you like help with?well, i am still learning and practicing my efficiency with grid layout.
Looks nice. Some feedback.
h3 for the name, you shouldn't be using a h1 for the testimonial text that comes after the name.h1 elements in a web page. h1 is usually the top most heading that describes the content of the page. In this design you actually don't have a h1. But you can create a visually-hidden element and give that a h1 if you want.h2 for each name and then use <p> elements for the testimonial text. I would also wrap the testimonial <p> elements in a <blockquote> element.max-width in rem, so that the grid items doesn't keep increasing in size on larger screens.rem for font-sizes, media queries and max-width. If the user changes their browser font size, text in your site won't respect that if the font-sizes are defined in px. If you have them in rem, the text will scale accordingly with the user's preferred font size. I usually use rem for all values, not only fonts. But it's just preference.Any feedback is appreciated
Looks nice. I have couple of feedback.
<header> or the banner role is usually used to wrap content that is common across multiple pages of a website. Usually you see it at the top of the webpage wrapping the site logo and a navbar. I would not use a <header> in this challenge.<main> landmark element. Every web page needs a <main> element wrapping the main content of the page.<nav> element is for a list of links that navigate within the site. Not for links that navigate to outside websites such as social media sites. In this case having the links inside a <ul> is enough.I'm happy with the result, I don't think I'll do anything differently at this stage, maybe with more practice I'll have better methods.
What challenges did you encounter, and how did you overcome them?Right now my biggest challenge is understanding how to use git and GitHub. I don't use the terminal much, so it's something that I need to start doing a lot more.
Looks good. Some suggestions.
<nav> landmark element is intended for links that navigate within your site, not for external links like the social links. Usually you see the <nav> at the top of the page inside the <header> along with the brand logo. Having the social links inside a <ul> with <li> elements is enough.rem for font sizes, padding and margins. Why not use rem for media queries and max-width as well? I usually use rem for everything. So if the user changes their browser font size everything will scale accordingly.