Skip to content
OverviewSolutions
10
Comments
17
Brian Schooler
@superschooler

All comments

  • Brian Schooler•440
    @superschoolerPosted over 2 years ago

    Hi Andrija, great job on this! The only suggestion I have would be to change the step in your inputs from 0.01 to 1, especially on the Number of People section. Most people don't go out to dinner with a fraction of a friend, so all it does it make them click 100 times to move the count up by one. I can see how decimals are useful on the total bill, but it would still take forever to get the bill amount right going $0.01 at a time. Just my thoughts! Awesome work 👍

    Marked as helpful
    1
  • Sushant nande•30
    @sushant2313Submitted over 2 years ago

    This was so challenging. For the first time I tried the mobile first and desktop. When I did mobile version it went smoothly. but I was stuck on desktop version for so long. Still couldn't figure some things out as you can see on the site and github. so can anyone please tell me how can I do desktop version for this. I've been on this same challenge for past 2-3 days but today I thought I'll just ask here since I couldn't do it. so tell me somethings I should try to make desktop version? or should I start from desktop first ? any suggestions and criticism are highly appreciated! thank you.

    Stat Preview Card

    3
    Brian Schooler•440
    @superschoolerPosted over 2 years ago

    Hi Sushant,

    Great job on the mobile version - it looks very good! A couple of suggestions I have:

    • Make the media query larger so it doesn't transition until the screen is wide enough for it to look decent - maybe around 1000px.

    • Add the following CSS so the image fills the right half of the screen in your media query:

    .image {
	height: 100%;
}

.image img {
	object-fit: cover;
	/* Prevents the image from distortion */
}

    The issue with your .third-col element is that every item is individually put in containers. You can see the gridlines in this image. Changing your code to the following should fix it, since it keeps the children of your new <div> element together:

    <div class="third-col">
  <div>
      <p class="nums">10k+</p>
      <p class="stats">COMPANIES</p>
  </div>
  <div>
      <p class="nums">314</p>
      <p class="stats">TEMPLATES</p>
  </div>
  <div>
      <p class="nums">12M+</p>
      <p class="stats">QUERIES</p>
  </div>
</div>

    You may need to work on the padding / margins after that, but it should do the trick!

    Marked as helpful
    1
  • Bola Okun•10
    @BolaOkun23Submitted over 2 years ago

    Hi Front-End Mentor Fam!

    This is my first project submitted! Would love any feedback you can offer!

    Areas I was unsure of is my use of Flexbox, and whether there was alternative way to reach the result?

    What use of Semantic HTML would be standard practice in a project like this, and is it necessary as it seems quite small scale?

    What are other best practice could I use to improve? and any recommendations for extra reading?

    (Extra Question - Best method to not go crazy when hitting an obstacle? Lol)

    Thanks :)

    QR Code using HTML and CSS

    1
    Brian Schooler•440
    @superschoolerPosted over 2 years ago

    Great work on your first project, it looks awesome!

    I notice you have .main-container and only one child inside of it - .qr-box. Because there's nothing else inside of .main-container you can do away with it and just style body. I personally love flexbox and use it wherever possible. Grid would be an OK solution as well but I find flexbox to be a bit simpler unless the layout truly requires a grid.

    I think you did great with the HTML and CSS. You could use main instead of div for .main-container if you wanted to and don't need a div to hold your image. You could simply give the image the .img-cont class to style it easily.

    It's not necessarily wrong, but I'm curious why you chose to use H3 for the heading, as opposed to H1, H2, or any other number? Best practice is to only have one h1 on a page, so if this project were in another website it definitely makes sense to defer to a smaller heading. In the context of this project, There's only one area with heading text so it might make accessibility a bit easier by using H1. Again, this isn't a big deal (as far as I know), I was just curious if there was a reason behind it.

    I noticed you used pixels for sizing everything. If you want a great resource to start out with, I'd recommend Kevin Powell's REM & EM explanation video on YouTube. This guy's got a lot of great content and a free course to help learn the basics of responsive CSS, unless you've got a great resource you're already using. The main benefit to using rem or em over pixels is that if a user has their default font-size enlarged in their browser settings, your website will scale with it.

    As far as getting stuck, read carefully through the code pertaining to your problem, take a walk, watch a YouTube video, and just poke around with it. I'd definitely suggest having a friend who's into web development as well to bounce things off of, or join slack groups (like the one Frontend Mentor has) to post questions if you can't figure it out after you've given it a few good shots.

    Hope this helps, if you have any other questions let me know!

    Marked as helpful
    0
  • Fernando Pérez•320
    @fermopSubmitted over 2 years ago

    Well, this challenge was really difficult to me. I had to study a few things I didn't know, such as icons and the position property. The widths are so challenging to me. I can't figure out how they work to make a responsive page. What do you guys think? feel free to see my code and add some suggestions at this site, I will thank you :)

    Huddle landing page with a single introductory section

    1
    Brian Schooler•440
    @superschoolerPosted over 2 years ago

    Hey Fernando, great job on this project! It looks great on both mobile and desktop.

    Can you clarify where you need help with the widths? I see you've got the width of the container set to 80% on the desktop view, but you also have a max-width of 1200px which is what keeps the image and text to not change sizes. Regardless, it looks great how you have it.

    One suggestion I have would be to use rem instead of px for your font-sizes. The root font size in browsers is usually 16px, so using rem will allow the text to scale if a user manually set their web browser to show text as larger or smaller.

    Here's a great video explaining rem and em: https://www.youtube.com/watch?v=_-aDOAMmDHI

    Marked as helpful
    0
  • Albert Garcia•20
    @albertgarcia1324Submitted over 2 years ago

    Initially had issues with splitting the card to have an image on one side and text on the other side, it was fun to figure out though! Also, aligning everything for product description and getting everything to look as close as possible to the jpg as possible. It's not perfect but I still enjoyed it!

    Responsive Product Page Sale

    2
    Brian Schooler•440
    @superschoolerPosted over 2 years ago

    Hi Albert,

    Building on what @ayushdotdev suggested, if you simply add .card-img { width: 50% }, the card would only be 25% the width of the container since you have it wrapped in another div. You should be able to get rid of the div sandwiched between .card and .card-img since it only holds one item (.card-img).

    Additionally, to make it responsive on a mobile device, you can try the following:

    @media (max-width: 550px) {
	.card {
		flex-direction: column;
		height: auto;
		margin: 24px;
	}

	.card-img {
		object-fit: cover; /* Or Change to Smaller Image */
		border-radius: 3% 3% 0 0;
		height: 300px;
	}

	.card-text {
		width: 100%;
		height: auto;
		padding-bottom: 24px;
	}
}

    This is a pretty simple solution that doesn't quite make it perfect, but is a great start! Nice work on your project 👍

    Marked as helpful
    1
  • Leo Silva•40
    @leozizzSubmitted over 2 years ago

    Following the challenge, I tried to do the best I could within what I know and with a little research, but I believe I can improve in the future. Can you give me tips on how to improve the code in general?

    I used CSS Grid and Flexbox to make the page organized and responsive, but I'm not sure if I used them in the best way. Do you have any tips, what do you think?

    Huddle landing page - Using CSS Grid and Flexbox

    2
    Brian Schooler•440
    @superschoolerPosted over 2 years ago

    Hey Leo, awesome job on the project and great use of CSS Grid. Using Grid can help you save a lot of unnecessary <div>s in the code over flexbox, and I see you used it wonderfully.

    Because you did so well, I only have one small criticism. I'd probably add:

    .illustration {
	align-self: center;
}

    This will keep the picture centered vertically next to the text when your media query hits so it's not way up next to the logo on iPad-sized screens. I'd probably also change the media query to at least 700px since the image is so small between 577px and 700px/

    Marked as helpful
    1
  • Brian Schooler•440
    @superschoolerPosted over 2 years ago

    Hi Gabriel,

    I noticed on larger screens there's a large gap between the image and text since you have a fixed height on your image. Something you might do to combat this is use the following css:

    .product-img {
	width: 100%;
	height: 100%;
	object-fit: cover;
}

    Even with this, the product doesn't look great on all screen sizes. The easiest solution to this would be to add a max-width to main so it will remain static at the largest visually appealing size. If you wanted to get more advanced, you could use a clamp to grow the text with the screen, but I don't think that's necessary with this project.

    Additionally, I'd add a max-width on your mobile product as well around ~425px. Any larger than that and the image dominates until the media break.

    Hope this helps!

    1
  • Brian Schooler•440
    @superschoolerPosted over 2 years ago

    Remove min-height on .main. 70vh is not necessary and will only distort things or made the top and bottom padding inside the container very large.

    Same with .main-image - you gave it a height of 40vh, but it's forced to stretch vertically with this which causes it to be less visually appealing. Instead, you might try width: 100% to make sure it stays tight inside the container but keeps its vertical dimensions. If the container is smaller than the image, however, this may be unnecessary unless you notice it overflowing.

    With that done, to ensure it stays centered vertically on the screen, you can use:

    body {
	/* This will keep the body at least the height of the viewer's screen, allowing you to center anything inside of it vertically. Notice I use min-height rather than height to avoid overflow on different screen dimentions: */
	min-height: 100vh;
	display: flex;
	/* Center Horizontally: */
	justify-content: center;
	/* Center Vertically: */
	align-items: center;

}

    Hope this helps!

    Marked as helpful
    0
  • Mathias Podesta•10
    @mapodestaSubmitted over 2 years ago

    The most difficult challenge in this exercise was the responsive.

    Maybe , I will try to improve the css (classnames or how to structure the css)

    So this is my question, how improve the css in this exercise or how we can change to improve this

    Kind Regards

    Product preview card component

    2
    Brian Schooler•440
    @superschoolerPosted over 2 years ago

    Hi Mathias,

    A couple of quick suggestions that might improve your design:

    The word PERFUME at the top can be made closer to the original design by adding the following CSS:

    .css-t1nuxs {
	text-transform: uppercase;
	letter-spacing: 0.25em;
}

.css-t1nuxs span {
	font-family: Montserrat, sans-serif;
}

    Change media query to ~1,100px - between about 500px and 1,100px the page is a bit distorted.

    Add max-width to the container to limit it to about 1400px. Anything larger than that begins to add a lot of white space to the bottom of your Add to Cart button.

    Hope this helps, let me know if you have any other questions!

    1
  • Abhilashpandey•270
    @freakyjonesSubmitted over 2 years ago

    There are two things-

    1. making equal width boxes with CSS flexbox. 2. I find it hard to understand when to use em, rem and when to use px.

    I would like to ask about the best practices for CSS units.

    Responsive component using flexbox

    1
    Brian Schooler•440
    @superschoolerPosted over 2 years ago

    Hi ABHILASHPANDEY,

    There’s a YouTuber named Kevin Powell who has awesome videos on all things CSS. Here’s one that explains the differences in px, em, and rem very well.

    Basically, never use px. Use rem for font-sizes. Use em for padding and margins.

    https://youtu.be/_-aDOAMmDHI

    Marked as helpful
    1
  • Yosef•500
    @YositySubmitted over 2 years ago

    This one was quite easy but I'm still unsure of what is the "right" way of responsive designing. Like how can I tell that this will work on mobile screens or not. Thank you for your feedback in advance

    Responsive design of QR-code component page

    1
    Brian Schooler•440
    @superschoolerPosted over 2 years ago

    Hi Yosef, great job! The easiest way to tell if it works on all screen sizes is to open developer tools (right click the webpage then select Inspect) and then slide the pane horizontally to compress and enlarge the site. If you hit a point where text overflows or it looks bad, you can use media queries or change other properties to fix it.

    Marked as helpful
    0
  • Brian Schooler•440
    @superschoolerPosted over 2 years ago

    Hi Aboyhasnoname, this is a great start! You've got all the colors and general layout right, but it's missing a good bit of CSS to look like the preview and to be responsive on mobile devices.

    If you haven't already, I'd take a look through freeCodeCamp.com's curriculum to see if it can teach you a bit more about CSS that maybe you haven't picked up or been taught yet.

    Otherwise, I'd suggest doing a bit of research on the following and trying it out with this design:

    • border-radius to curve the edges of the element
    • letter-spacing to spread out the letters in "PERFUME"
    • Remove the "text-align: center" property on all the text - the demo has it all aligned left, with the exception of the button
    • Media queries to make the image change on a smaller screen and perhaps flexbox and flex-direction: column to stack the image on top of the text on the smaller screen
    • Change the text color of the button to white
    • Change the price text to green
    • Move the price text with the strikethrough so it's spaced a bit more to the right of the green price

    Let me know if you have any specific questions I can help with!

    Marked as helpful
    0