@ClaytonDewey
P
rmmcfarlin
@rmmcfarlinAll comments
- P@rmmcfarlin
Great solution!
- @NathanGeovaneP@rmmcfarlin
Solution looks good, calculator functions well. Only thing I would add would be getting it centered in the viewport on the desktop version, but awesome job.
- @timothyho512P@rmmcfarlin
The desktop design looks pretty close, and the javascript is working as intended. However, I think you could spend some time making your design responsive, as it does not work except on large viewport widths. Either using different flexbox techniques eg. row-wrap, or using media queries to achieve different layouts for smaller viewports.
- @DebabrataBanikWhat specific areas of your project would you like help with?
Any feedback is welcome
P@rmmcfarlinSolution looks great!
- @proAbenezerP@rmmcfarlin
Your solution looks good, very close to the original design. Padding / margins and proportion of the image to the text section could be improved to get it pixel-perfect, but the solution still looks good and is close enough. Could add some semantic HTML for accessibility. Overall though, looks great!
- P@Pokerlo442What are you most proud of, and what would you do differently next time?
Grid systems are easier for me again. Next time I would build the grid system with the mobile first workflow.
What challenges did you encounter, and how did you overcome them?It was very challenging to design some testimonials differently without assigning them their own classes. To solve this I used pseudo selectors with functional selection.
What specific areas of your project would you like help with?None in particular, but I welcome any suggestions for improvement.
P@rmmcfarlinYour solution is spot on, your code is well structured and easy to read.
As far as using selectors to format each of the cards, what is wrong with assigning each their own id instead of using pseudo selectors? I am a beginner so I might be wrong here, but for me it would seem easier to just assign each card an id (ex. id="card1"), instead of using combinations of pseudo selectors. Unless there is some disadvantage that I am unaware of, this seems like it is more efficient and avoids over-specificity.
Also, it seems to me like the breakpoint might be set at too large a width. It might be advisable to make your layout responsive enough to shrink down to a more common breakpoint (i.e. 48rem / 768px), or set another "medium" grid layout to avoid having the column layout take effect with larger viewports. This is just my impression though. It's still responsive, but to me the column format doesn't look good at larger viewport widths.
Overall though this solution is spot on to the design, excellent job.
- @Marvel123gWhat specific areas of your project would you like help with?
All I need is overall feedback thank you😊.
P@rmmcfarlinWhile your solution still looks great, the layout of the cards does not match the design. I think you should try using grid to position them with the "diamond" shape, but I think you'd be able to do it with flexbox as well if that's better for you.
The design is responsive and looks good on smaller viewports, but I think you could use relative units for sizing your elements to avoid so many hard breakpoints. While the media queries do work to make your design responsive, using relative units for sizing will trim down on code and keep it to one or maybe two media queries and hard breakpoints.
Overall though, the design still looks good and is responsive.
Marked as helpful - P@nurshWhat are you most proud of, and what would you do differently next time?
I am proud to be able to use CSS Grid and understand how it works. Also, that I learnt something new with respect to responsive images.
What challenges did you encounter, and how did you overcome them?I had a problem with the responsive image. I was using the img-srcset which was working on all browsers except chrome. I had to think about the problem again and I realised I was using the wrong markup to solve it. So I switched to the picture tag and everything worked great.
What specific areas of your project would you like help with?Any feedback is welcome
P@rmmcfarlinGreat solution overall. Only two things I would say are to make sure the green color matches what was in the design file for the button and price color, and maybe avoid setting the margin for the main element. Instead of fitting viewport width and height, the margin creates a set size, so the card isn't centered when the page first loads because there is more space to scroll on the top and bottom. Well done overall.
Marked as helpful - P@librartWhat are you most proud of, and what would you do differently next time?
I learned how to use the
What challenges did you encounter, and how did you overcome them?root: {}selector and media query, but I still need to learn more about media query for responsive web.I still need to learn more about media query for responsive web.
What specific areas of your project would you like help with?I think I would love to get feedback for all of my code from HTML structure to CSS styles.
P@rmmcfarlinLooks great!
- @croochetP@rmmcfarlin
This solution looks pretty close, I think the attribution at the bottom is pushing the design up and off center, maybe try an absolute position for the attribution element to keep it stuck on the bottom of the viewport? Only other thing is the background color of the body is darker, but you can fix this looking at the exact color in the style guide. Nice job overall!
- @Perfect-PatienceWhat are you most proud of, and what would you do differently next time?
I'm most proud that I paid extra attention to the Figma file and managed to get the card to look as close to the design as possible. I noticed details like the vertical gap and drop shadow measurements in the Figma file.
What challenges did you encounter, and how did you overcome them?The main challenge in this challenge for me was finding a way to adjust the widths of components based on the screen size without using the @media query. I researched and learnt of the clamp property in CSS which is what I used to obtain this effect.
What specific areas of your project would you like help with?I want feedback on whether I used variables properly in this project. And on anything I could improve.
P@rmmcfarlinNice job! Looks dead on, nothing to add
- @Jesusguy340What are you most proud of, and what would you do differently next time?
I am proud I was able to do this by myself with help from the web.
What challenges did you encounter, and how did you overcome them?I am just new at css, so some of the topics I haven't covered yet, so I searched them on the web.
What specific areas of your project would you like help with?I would like help with adding mobile design to my css code.
P@rmmcfarlinIt looks like you could benefit from using flex and centering your box / card instead of using absolute units for the margins. This would help make it better on mobile, so the box stays centered with different viewport sizes.
Something like this
body { background-color: hsl(212, 45%, 89%); height: 100vh; align-items: center; justify-content: center; display: flex; flex-direction: column; }
