Looking to hire developers?
@Miculino
@momenkamal221
Submitted
Hey, I was a little bit confused about what challenge should I pick. I chose that one and it looks very similar to the last challenge I picked - NFT preview card component -, just the background was challenging for me, so my question is, what approach should I follow that will help me at picking the next challenges? or should I just do all of it?
@Miculino
Posted
Hey @momenkamal221, congrats on completing the challenge!
I had a look at your final solution and I have a few suggestions for you that I hope will be useful:
You don't need to set a height property on your .card - your layout should be created in such a way to naturally adjust to the available content. When you set fixed values for your height, or even width, it becomes more difficult to create a robust responsive design.
You can vertically and horizontally center your design by using flex display on your body element. Also, add min-height: 100vh to your body element
https://alligator.io/css/viewport-units/
Your .card is also missing a box-shadow -> refer to the original design
Change styling for the .unit elements. Currently, they're a bit different from the original design
You don't need an hr element. You can simply set a border-top property on your .flip-scores element and then also add padding-top as to push the line further above
Hope this helps. Keep up the good work!
Marked as helpful
@wisdompython
Submitted
i need some help with using box shadow for this code And also some helping in reducing the thickness of an horizontal line
@Miculino
Posted
Hey @wisdompython , good job on completing this challenge!
I had a look at your final solution and I have a few suggestions that I hope will be useful to you:
Don't use a fixed height on your card element, especially not with an absolute unit such as px. Your layout should be built around the available content.
Set min-height: 100vh on your body element if you want to center vertically and horizontally everything inside it.
Wrap your card inside a <main></main> element to remove some of the errors from your report
Add the alt attribute to your img element - if the img is just for decorative purposes, then you can simply write alt=""
The 2 text hover effects and the image overlay hover effect are missing from your solution. Consider implementing them using the :hover pseudo-class https://www.w3schools.com/cssref/sel_hover.asp
You don't need an hr to create that line. You can set a border-top property on your footer and then also add a padding-top to push the line away from the content
Hope my suggestions will be useful to you. Keep up the good work!
Marked as helpful
@TejaswiniLabade
Submitted
Hi, after the completion of my first frontend mentor challenge, I was very excited to finish the second challenge as well, so here is my solution to NFT preview card component. Any feedback will be really appreciated.
@Miculino
Posted
Hey @TejaswiniLabade, good job on completing this challenge!
I'm impressed with how similar to the original design your final solution looks - well done!
As @denielden already suggested, you can consider adding the transition property for your hover effects, mainly the image hover effect as it can give it that extra nice detail ;)
Had a look at your Github repo and you should definitely consider looking into BEM methodology. Also, some parts of your code require a bit of formatting so they can be more readable.
Overall, well done. Keep up the good work!
Marked as helpful
@jdevelops
Submitted
Still have some issues with this design. Most irritating is <a> attribute sticking out of a parent div any ideas how can i fix it quickly ?
@Miculino
Posted
Hey @jdevelops, congrats on completing this challenge!
I had a look at your final solution and I have a few suggestions that I hope will be useful to you:
You can vertically and horizontally center your .main-box by using flex display on your body element
There's an issue with the responsive design when scaling down to smaller screen sizes. I suggest not using fixed px values for your width as it can make it more difficult when building a responsive design. Have a look through your media queries code and see what improvements you can make
Adding a little transition on the image hover effect would be a nice bonus ;)
Overall, you did quite a nice job. Keep up the good work!
Marked as helpful
@abhishekdwaghmare2000
Submitted
Any suggestions about improving specific part of webpage, mistakes and other suggestions about improvements are really welcome ! It would be really helpful, if you suggest the mistakes I made and the improvements need to be done at specific point or any other small thing need to be fix in this challenge. Thank you!
@Miculino
Posted
Hey @abhishekdwaghmare2000, good job on completing the Stats Preview Card challenge!
I had a look at your final solutions and here are a few suggestions I have for you:
Consider implementing the responsive media query breakpoint sooner (i.e at a larger size)
The top right and bottom right edges of the image should be rounded (same value for the card's left side)
Increase line-height and font-size of the p element inside .content-left
Use the values 100, 400, and 700 for your font-weight property
Use actual values (or variables) for your font-size property. Don't use small, medium` or other such keywords
Setting a background-color property on your img element and using it with mix-blend-mode won't work because mix-blend-mode is actually blending the img element with the card's background color. A possible solution is to remove the img element and just add a background image on the .content-right div and then you can use background-blend-mode to create the desired effect
Hope these suggestions will prove useful. Keep up the good work!
Hi! Everyone, I made this project using Vanilla JS with SCSS. I learnt a lot from this challenge and it was really a good experience for me, I learnt some cool things about how to make custom input component. I added the prefixes in the CSS in order to make design consistent through all browsers. I also made it responsive, but let this decide by you (current viewer LOL ). So please go through the design, code and leave your precious feedback below, So that I can improve myself more. I would love to hear feedback regarding the design, code and structure. Any feedback to make it more accessible to screen-reader will more highly appreciated.
@Miculino
Posted
Hey @skyv26 , always nice seeing more of your projects here on Frontend Mentor!
I appreciate how you always try to make your solution as pixel-perfect as possible! ;)
Here are my suggestions / observations based on your design:
There shouldn't be a 0 value on the slider as far as I know. The first value on the slider should be 10k page views at 8$ (I might be wrong about this, please do double check to make sure)
On laptop resolution, the background image doesn't fully stretch horizontally from one end to another of the body's width
Try to reduce the card's box shadow a bit - reduce the shadow opacity and maybe also tweak the blur amount too
The folder structure was a bit confusing because you have a sass folder and a css folder but the main sass file is actually inside the CSS folder
I have to ask you something: Did you actually write over 1000 lines of raw SCSS code?? That's incredible!
As developers, we all should strive to write clean, efficient, maintainable, and reusable code with the least amount of lines of code that's realistically possible. An educated guess from me would be that the design for this project can be completed in less than 500 lines of SCSS code.
I like the way you write and structure your code. You're quite a competent developer, no doubt. But you shouldn't do two things at once while working on such projects: coding and writing novels because that's how long your lines of code can get to the point that it feels like reading a page from a novel :))
I'm looking forward to seeing more of your solutions here on Frontend Mentor. Keep up the amazing work!
Marked as helpful
@Programming-School-Pro-Coding
Submitted
Feedback
@Miculino
Posted
Hey @Programing-School, thanks for sharing your solution with us!
I had a look at your final solution and I have a few suggestions to offer that I hope will be useful to you:
Change the background color to the one in the original design
Add the two circle background images that can be seen in the original design
Don't use margin to position your card. There are multiple good ways to do that - flexbox, CSS grid, absolute positioning, etc
Change the card's box-shadow color and reduce its size - make it more subtle
Make the edges of the card rounded using border-radius property
Center the "London" and "Victor Crest 26" texts
Change the card's background-color
Remove the padding around the bg pattern image inside the card
Hope this helps. Keep up the great work!
Marked as helpful
@chrisvn188
Submitted
I use the combination of flexbox and grid to build layout for this challenge. I learned to use transform property to make card move left or right, up or down. Simple challenge but learned a lot :). I like the color pallette of this design. Any feedbacks to my code are appreciated! Thank you all!
@Miculino
Posted
Hey @chrisvn188, congrats on completing this challenge!
Your solution came out looking really good and the responsive design works as expected!
Just a few suggestions based on your final solution:
The background images that you can see in the original design are missing from your final solution. There are multiple ways to tackle this task. You can use background-image property on the body element or add two divs and use absolute positioning on them or use the ::after and ::before pseudo-classes
On mobile resolution, add just a bit more horizontal padding on your body
Overall, great job!
Marked as helpful
@jhonGriGi
Submitted
I had some problems with the dev tools and the responsive option, so I don't know if the responsive is right :c
@Miculino
Posted
Hey @jhonGriGi, thanks for sharing your solution with us!
I had a look at your final solution and here are my suggestions:
Reduce the card's dimension just a tiny bit
Add a box-shadow as the one shown in the original design
Consider using background-image property for the two images or you can also use ::after and ::before pseudo-classes
Use Flexbox display to vertically and horizontally center your design display: flex; align-items: center justify-content: center min-height: 100vh should all be applied to the body element
There's a huge amount of space on smaller screen sizes that creates horizontal and vertical scrolling. That's due to how the current layout is setup. You have to make some changes to how you position and align items in your page to make sure your design is also responsive
Hope these suggestions will be useful. Keep up the good work!
Marked as helpful
@Rebeitte
Submitted
In this project I tried to improve my code, making it more readable and more order. Also, I used the feedback of my previous project to give a better accesibility to my project.
Any feedback is welcome, I'm still learning, and I like to receive advices.
@Miculino
Posted
Hey @Rebeitte, congrats on completing the challenge!
Overall, your solution looks really good and the responsive design works as expected.
Just a few suggestions based on your final solution that I hope will be useful to you:
Add a background-color for the .plans element - refer to the original design for the color
On larger screen sizes, make the "Proceed to Payment" button a bit wider
Increase the y offset of the box-shadow of the card and increase the blur as well. Perhaps the color is different as well from the looks of it
Marked as helpful
@jakubinhoo
Submitted
Any feedback appreciated :)
@Miculino
Posted
Congrats on completing the challenge, @jakubinhoo
Your solution looks really well done! And the responsive design works as expected. Good job!
Looking at the comparison screenshot, it looks like the background color is a bit different, but that's not an issue ;)
I also had a look at your Github code repo. Here are a few suggestions I have for you:
width: auto; height: auto; margin: 0 auto;The height is auto by default and you already set the width to 800px. Also, you don't need the margin property in that case because you've already centered the container using flex display
You can rewrite this line grid-template-columns: 1fr 1fr 1fr; using repeat function like this grid-template-columns: repeat(3, 1fr); and if you want to have a single column layout for mobile screen you can do this grid-template-columns: repeat(3, 1fr);
Consider adding min-height: 100vh on body element so you know the container will be vertically centered no matter the resolution
Marked as helpful
@Programming-School-Pro-Coding
Submitted
Feedback
@Miculino
Posted
Hey @Programing-School
Congrats on completing the challenge.
Here are my suggestions based on your final solution:
Structure your HTML elements better. For example, wrap the avatar img and the text next to it inside another <div></div> element
Avoid using inline CSS styling
Use more meaningful class names
It's bad practice to use margin values like that to position your card component. Better to use something such as flexbox display
Box shadow for your card is too much - reduce it and also soften it a bit more
The ETH icon is missing
Image hover effect is missing. Check this out for reference https://www.w3schools.com/howto/howto_css_image_overlay.asp
Reduce space around image
Reduce rounded corners of the image
Add more space around the card's content
Missing the appropriate font(s) from the challenge
Make hr line color more subtle / change color
Try to avoid using fixed values for width / height properties (especially if they're px based)
Hope my tips will be useful to you. Keep up the good work!
Marked as helpful
@dodytrifa
Submitted
Hello everyone, please check out the three column challenge. I appreciate your feedback
@Miculino
Posted
Congrats on completing the challenge, @dodytrifa
You've done quite a nice job and the responsive design works as expected!
Just a few suggestions based on your final solution that I hope will be useful to you:
Increase the size of the buttons just a tiny bit
Missing the background color that's in the original design
Try to remove the vertical scrollbar. Use absolute positioning for the attribution that's at the bottom of the page. That can help fix the scrollbar situation
You should add class names for the other elements as well (h3, p, img)
Consider implementing BEM methodology https://en.bem.info/methodology/
You can vertically and horizontally center your container by setting flex display on body and using min-height: 100vh and align-items: center and justify-content: center properties
Hope this helps. Keep up the great work!
@LucasTamaya
Submitted
Hello, I would like a global opinion on my realization. Thank you in advance for your feedback!
@Miculino
Posted
Hey @LucasTamaya!
Good job on completing the challenge! It looks pretty much the same as the original design and your responsive design works as expected. Well done!
Consider adding a hover effect on the buttons as shown in the original design files and challenge requirements.
The code for your CSS grid is a bit messy. You're using multiple approaches for this layout. You should just pick one from those. For example, you can have the 3 horizontal columns and 3 vertically stacked columns by using grid-template-columns: repeat(3, 1fr) and grid-template-columns: repeat(1, 1fr). This will get the job done ;)
Hope these suggestions will be useful. Keep up the great work!
Marked as helpful
@alt-plusF4
Submitted
Feedbacks, please!
@Miculino
Posted
Hey @alt-plusF4
Congrats on completing the challenge. It looks really nice!
Here are my suggestions based on your final solution:
Consider changing the box shadow of the "Proceed to Payment" button - refer to the original design
Add box- shadow for the card component - refer to original design
I don't think there's any utily for the transform: scale(0.94) property there
You can vertically center the card by adding min-height: 100vh on the body element and then using flex display properties align-items: center
Try removing vertical scrollbar. You can do that by absolute positioning the .attribution element
Marked as helpful
@Mansour0007
Submitted
second attempt for this challenge with some changes
@Miculino
Posted
Hey @Mansour0007
Good job on completing the challenge. It looks pretty nice!
Here are my suggestions based on your final solution:
Consider adding some padding-right on the .all-text div on larger screen sizes to push the content a bit more
You can also add some horizontal padding on the body element to make sure there's enough space around the card itself
On smaller screen sizes, consider adding vertical padding on the body element to make more space for the card at the top and bottom
Try to avoid using px (except maybe for small values between 1 - 10px) and instead, use relative units such as rem, em, vh / vw, % - they're better alternatives for building responsive layouts
Don't use fixed values for your width and height, especially if they're px based. Build your layout around the content that's available. If I recall, you can build this project without using the height property at all. If you use it, it can complicate everything unnecessarily.
Hope this helps. Keep up the amazing work!
Marked as helpful
@tobiasz121
Submitted
First try at the css grid. Open to all suggestions especially css grid and responsive design.
@Miculino
Posted
Hey @tobiasz121
Congrats on completing the challenge! You've done a nice job overall!
Here are my suggestions based on your final solution:
The background color used is a bit different - refer to original design
The box shadow of the cards has a different color. Also, consider reducing the amount of shadow
Use a similar color to the one in original design for the text
Increase line-height of p inside header
Your layout stays the same even on mobile screen sizes.
Use padding property instead of width on the container
Consider using grid-template-areas for building this layout and making it responsive using media queries https://developer.mozilla.org/en-US/docs/Web/CSS/grid-template-areas
Hope this helps. Keep up the great work!
Marked as helpful
@efs0-cod3
Submitted
First time using JSON... Any feedback will be appreciated. Thanks in advance
@Miculino
Posted
Hey @efs0-cod3
Good job on completing the challenge. It looks pretty good and the main feature works as expected. A few suggestions based on your final solution:
.cont__info doesn't fully cover the background color of the parent element around the edges. You can see a little bit of the color
Responsive design needs some adjustments. You should implement the media query sooner and also make sure everything is vertically and horizontally centered
Try to avoid fixed px based width and heights values. It's a bad practice when trying to build responsive layouts. Build your layout around the available content and try to use relative units such as rem, em, vh / vw, %
You already have all the data inside the data.json file, you don't need to write it again in your JS file
Avoid using innerHTML property. It's not a safe option as it can lead to security risks. The appendChild() method combined with DOM Nodes is a much better approach to this issue
Use const instead of let when declaring a function expression
Hope this helps. Keep up the good work!
Marked as helpful
@bdarab
Submitted
Here it is. It took 5 hrs. The main challenge was to decide which technic to use. Grid or flex. Looking forward to your feed back.
@Miculino
Posted
Hey @bdarab
Congrats on completing the challenge! It looks nice overall, I'd say. I have a few suggestions for you that I hope will be useful:
The container should be vertically centered as well
There shouldn't be any box shadow on the container
Add more padding around the content inside the cards
Change background color according to the one in the original design
Icons dimensions are different. Currently, they look a bit stretched and disproportionate
For the .card-text, you can use the text from the original design
Responsive design needs some adjustments. On mobile screen sizes, the container isn't centered. It stays more on the right side. You should also go from 3 horizontal columns to 3 vertically stacked columns. On tablet resolution, you have two columns on one line and the third column is underneath them.
Hope this helps. Keep up the good work!
@JuHnr
Submitted
Hi ! Here's my first challenge. I try my best to get it looking as close as the original. I struggled a bit with the hover effect on the image. I'm not sure about the reusability of my classes and styles using the DRY principle (eg. colors). Also, I didn't find a way to choose the color of the <hr> line. Overall I spent around 4 hours and I'm pretty happy with the result for a first one. I'm looking forward to your feedback, thank you :)
@Miculino
Posted
Hey @JuHnr
Congrats on completing this challenge! Your design looks quite nice and it's responsive as well!
Just a few suggestions based on your final solution and Github repo:
You have a vertical scrollbar that shouldn't be there. It's probably caused by the footer. Try using absolute positioning on the footer and or / reduce the space below the card
The hr line should be a different color and also more thin. Btw, you can achieve the same effect in a simpler way by just setting a border top on the .third element
It's a bad practice to use fixed px values for your width / height properties when building a responsive layout. Consider building your layout around the content available. You don't need height property at all in this project.
Look into relative units and try to implement those over px unit https://developer.mozilla.org/en-US/docs/Learn/CSS/Building_blocks/Values_and_units
Hope this helps. Keep up the good work!
Marked as helpful
@Nabster2010
Submitted
@Miculino
Posted
Congrats on completing the challenge, @Nabster2010
Your design looks quite nice and the responsive design works as expected. Just a few suggestions based on your final solution:
Use only one <h1> element in your page
The paragraph below your h1 should have more space at the top and be centered horizontally
Change the box shadow color and make it more subtle - refer to design
Hope this helps. Keep up the good work!
@Schmelzofen
Submitted
@Miculino
Posted
Congrats on completing the challenge, @Schmelzofen
Your solution looks really nice and it's responsive as well. Just a few suggestions based on what I've seen in your final solution:
The share menu doesn't work as expected. There are multiple ways to approach this feature. You can create the share menu, hide it with display: none and then use JavaScript to toggle a class that'll hide / reveal the menu
On smaller screen sizes, consider adding some horizontal padding on the .details element
Marked as helpful
@Luzanne-S
Submitted
@Miculino
Posted
Congrats on completing this challenge, @Luzanne-S!
You did quite a nice job on your project and it looks really good. Just a few suggestions that I hope will be useful to you:
Consider also adding a background color besides the background image - refer to the original design
Add more space around the card on smaller screen sizes - use media queries for that
You can also use <a> element for Cancel Order button. Also, add a hover effect on it and a cursor pointer
Add a bit more vertical padding on the "Proceed to Payment" button
Change box shadow color and make it a bit softer as shown in original design
Hope this helps. Keep up the great work!
Marked as helpful
@Rohitgour03
Submitted
Here is my solution to this challenge. 🔥 I am facing a problem that I have stopped the form from submitting using e.preventDefault() but when all the entries are correctly filled, the form is not submitted, what should I add so that it gets submitted after filling in the correct inputs and the form will then be completely empty to fill the entries again??? And what about other things on which I can improve ? Please provide your valuable suggestion. 🙏
@Miculino
Posted
Hey @Rohitgour03
Congrats on completing the challenge. Your solution looks really good.
As @ayoam suggested, that could be a possible solution, but given your project, I don't think you'd actually want to submit the form. You can recreate the submit effect by emptying the inputs after you click the submit button.
Also, based on @ayoam suggestion, you don't need to write (allValid==true) - you can simply write if (allValid) as that variable will be converted to a boolean value automatically and checked for validity
Marked as helpful
