@namanajmeraSubmitted 18 days ago
Isaac Alvarez
@isAlvarezDevAll comments
- @isAlvarezDevPosted 15 days ago
Hey Naman, how you doing? Great solution. The only thing I will recommend you is, you should separate the html in several components instead add all in page.jsx and import these to the page. For example, there are 5 cards, which have img, name, header and description. You could make a base for these and use parameters to get the data. This will improve the readability and modularity
The rest is okay. Good job. Have a good day!
0 - @VaclavKeySubmitted 15 days ago@isAlvarezDevPosted 15 days ago
Hey man, great solution. It just looks exactly as the original design heh. The only thing I could tell you is, between 1025px and 1140px there are several changes with the top margin property. Sometimes it doesn't has margin but other times does.The rest is pretty good.
Have a good day!
1 - @moyinoluwa001Submitted 26 days agoWhat are you most proud of, and what would you do differently next time?
I am most proud of successfully implementing the staggered grid layout that gives the design a modern and dynamic look. It was challenging to balance the alignment and spacing while maintaining responsiveness across different screen sizes. Additionally, ensuring the cards maintained their visual hierarchy while preserving the clean and structured design was quite rewarding.
If I were to do this project again, I would focus more on optimizing the CSS for maintainability by using variables for colors and spacing. I would also explore using CSS Grid more extensively to enhance flexibility in layout adjustments. Finally, I would consider adding subtle animations to enhance user engagement and make the interactions feel more fluid.
What challenges did you encounter, and how did you overcome them?One of the main challenges I encountered was achieving the staggered grid layout while maintaining consistent card sizes and ensuring the design was responsive. It was tricky to position the cards in a way that looked visually appealing without breaking the layout on smaller screens.
To overcome this, I experimented with CSS Grid properties, such as
grid-template-columnsandgrid-template-rows, combined withtransform: translateY()to achieve the desired offset effect. I also used media queries to adjust the layout for different screen sizes, ensuring a smooth and adaptive user experience.Another challenge was maintaining consistent spacing and alignment between the cards. I addressed this by utilizing CSS Flexbox properties for alignment and ensuring all cards had equal padding and margins.
By breaking down the layout into smaller sections and testing frequently, I was able to incrementally adjust the design until it matched the desired look.
What specific areas of your project would you like help with?I would like help with the following areas of my project:
-
Responsiveness and Cross-Browser Compatibility: Although I implemented media queries to make the layout responsive, I want to ensure it looks consistent across all devices and browsers. Any feedback on improving the responsiveness or handling edge cases would be appreciated.
-
CSS Optimization: I used a combination of Grid and Flexbox for the layout. I would like feedback on optimizing the CSS for better performance and maintainability. Are there more efficient ways to achieve the staggered grid layout?
-
Accessibility Improvements: I aimed to make the site accessible, but I would appreciate suggestions on improving the semantic structure, ARIA labels, or any other accessibility best practices.
-
Code Readability and Maintainability: Any feedback on improving the organization, readability, or maintainability of my HTML and CSS would be beneficial.
I welcome any other general feedback or suggestions for enhancing the overall design and user experience.
@isAlvarezDevPosted 26 days agoHey, Abdulrahmon. I saw your code and I'd like to recommend you a few things.
First of all, try to remove all the comments because that only annoys when someone else is trying to watch the code. As a rule, it's not necessary adding comments except if the code (function, method, class...) is really complex and, these are really brief.
One thing to improve the accesibility is, instead add a
divwith a classcard, you could change it for<section>check the documentation about section tag.The media query, .card-layout has a
display:grid; gap: 20pxit's not necessary adding them again. You already had them before.Another thing is, convention. For example, it's better if you have all the colors in hex instead a mix with hex, hsl and rgba. Just follow one of them and that would be okay. This point is more to maintain the uniformity of the code.
That's all for now. Great solution and happy codding! :)
Marked as helpful0 -
- @chryspenalberSubmitted about 1 month agoWhat are you most proud of, and what would you do differently next time?
I was proud of successfully adapting the code for smaller screens without major difficulties. I finished the project in two days, but I believe it could have been faster. And I think I could work better on accessibility next time.
What challenges did you encounter, and how did you overcome them?Actually, I didn't encounter any major challenges with this project. Adapting the code for smaller screens was simple and straightforward. I was able to complete it quickly without any difficulties.
What specific areas of your project would you like help with?I would like to know what I can improve in terms of semantic HTML and accessibility.
@isAlvarezDevPosted about 1 month agoHey, Chrystiana. Good job, your solution is great.
There are two thing I would advise you. The first one is, the <b> tag is deprecated, MDN suggests using <strong> because <b> is an element doesn't convey such special semantic information. The second one is, you should read about clamp() property in css. It will help you to create dynamic font sizes and spacing. Instead of type a specific font-size o padding depending the screen width, with clamp() can make it easy for you.
Marked as helpful1 - P@nashrulmalikSubmitted 2 months ago@isAlvarezDevPosted 2 months ago
Hey man, I like your design, it's the same as the project itself. On the other hand, these lines in your css, its better if you use the link tag on the hmtl because it forces the browser to download imported stylesheets sequentially, rather than allowing parallel downloads and you can solve it using the link tag.
@import url('https://fonts.googleapis.com/css2?family=Montserrat:wght@500;700&display=swap'); @import url('https://fonts.googleapis.com/css2?family=Fraunces:wght@700&display=swap');By the way, the <div class="content"> you could change it for a section tag or article to improve the semantic. Have a good day!
Marked as helpful0 - @JosiasSandovalSubmitted 3 months agoWhat challenges did you encounter, and how did you overcome them?
Si tuve algunos incovenientes
@isAlvarezDevPosted 3 months agoHey man, I saw your code and... I'd like to recommend you to use more semantic HMTL, for example, the tags: <header><main><footer><section><article>
Instead of using a lot of divs, you can use those I mentioned and you will have a better SEO and accessibility. On the other hand, your github link is fallen.
Another thing is, please, don't use more than one <h1> tag. Google itself will penalize you because its not recommended. Have a hierarchy well structured with all the headers and thats all.
Marked as helpful1 - @hmillersarahSubmitted 3 months ago@isAlvarezDevPosted 3 months ago
Your solution is great. The only thing I'd recommend you is, just a
cursor:pointerwhen you are hovering the buttons. Also instead of buttons, you could use<a></a>and you could avoid the onClick event. Good job!0 - P@DetDet91Submitted 3 months ago@isAlvarezDevPosted 3 months ago
Hey man, all good? Your solution is great, I'd only say is, the image-avatar.webp isn't displaying on the card and you need to add a hover in <h1> (see in design/active-states.jpg). The rest is okay, I like it
1 - @budijiieSubmitted 3 months agoWhat challenges did you encounter, and how did you overcome them?
I still need to learn a lot about responsive layout. I feel difficult make it look the same as the original dispay and still keep it responsive.
Finally after seeing the other solution using clamp(), I also learned it and using it
What specific areas of your project would you like help with?Any suggestion to make it look exactly the same as the preview received. Thanks
@isAlvarezDevPosted 3 months agoGreat solution man. The only thing I'd say is, the
<div class="card">you could change it for<main>tag and the<div class="attribution">for<footer>These will help you to improve the accessibility. Have a good day :)1 - P@gkilasoniaSubmitted 3 months ago@isAlvarezDevPosted 3 months ago
Great solution, man. It looks exactly as the image itself. The only thing I'd say is, try to create some variables in CSS for those values are the same. For example, I saw in your CSS styles you typed several times the
margin-bottom: 12px;for that, you could write in your.root: {--medium-spacing: 12px;}and replace the value 12px for var(--medium-spacing).The rest I think It's great, congratulations :)
Marked as helpful0 - @edwin-hudsonSubmitted 3 months ago@isAlvarezDevPosted 3 months ago
Hey Edward, your solution is great but, I'd like to recommend you just, there was a
<div>with a class: container, if you change it for<main>tag, you will have a little bit more of accessibility in your code. The rest is okay 👍Marked as helpful1 - @kad5Submitted 3 months ago@isAlvarezDevPosted 3 months ago
To be honest, in this project, you can create one div actually because all the elements you need to use, which are 3, are in one column. If you used
display: flex; flex-direction: columnthat would be enough. But, I guess you used 3 divs inside of another div because they are display: block and as you know display: block, take all the horizontal space left1 - @kad5Submitted 3 months ago@isAlvarezDevPosted 3 months ago
Hey Kad5, I think It's great the solution you did but, the only thing I would recommend you is, the element:
<div class="headline-text"> Improve your front-end skills by building projects </div>You should change the <div> element for a <p>. It has the same result and It's better for accessibility.
Marked as helpful1 - @sonnyshoSubmitted 3 months agoWhat are you most proud of, and what would you do differently next time?
I am proud that I committed to finishing it and learning more about frontend web development. I would spend more time thinking about the style of the page and how best to keep it responsive without needing to do much tinkering later on. I think I would also comment more of the code as to keep track of specifically what needs to be done.
What challenges did you encounter, and how did you overcome them?Keeping it responsive across a wide range of devices was challenging. I wasn't familiar with media queries and vw/vh units before doing this challenge. Learning about both helped tremendously in completing the challenge.
What specific areas of your project would you like help with?I am completely new to this so pretty much any advice is welcome, although I am not explicitly seeking help I just don't seem to be able to finish this learning path without submitting my solution.
@isAlvarezDevPosted 3 months agoCongratulations for finishing the project. The only thing I would recommend you is, between 401px and 411px the layout behaves in a strange way. It's like you were using zoom 40%. That's all. Have a good day.
0
