Skip to content
OverviewSolutions
41
Comments
80

Venus

@VenusYUnited Kingdom1,770 points

Hey there! I'm a self-taught frontend developer with a passion for coding and problem-solving. I love learning and helping out the community whenever I can!

I’m currently learning...

HTML, CSS, Sass, JavaScript, React, Webpack, NPM

Latest solutions

View more solutions

Latest comments

  • Venus•1,770
    @VenusY

    Great work on this challenge!

    I noticed that when the page's width expands beyond 1440px, the card is no longer centered on the page.

    This is because you've set a max width of 1440px on the <main> element.

    By removing this property, you allow the page to span the entire width of the viewport, which keeps the card centered on the page regardless of what screen size the user's device is.

    I also noticed that the card isn't vertically centered because you've set the height of the main element to fit-content, which is causing the height of the main element to take up only as much space as it needs as opposed to the entire viewport.

    If you remove this property and replace it with min-height: 100vh, this will set the height of the main element to 100vh initially and allow it to grow if necessary.

    As a result of this, your card will always be vertically centered on the page.

    main {
  height: fit-content; ❌
  min-height: 100vh;
}

    Finally, on smaller screen sizes, your website isn't very responsive due to the hard-coded widths of your card and NFT image.

    You can make your page more responsive like so:

    .preview-card {
  width: 350px; ❌
  max-width: 350px;
}

.equilibrium {
  width: 302px; ❌
  height: 302px; ❌
  width: 100%;
  aspect-ratio: 1 / 1;
}

.preview-card .ETH-num {
  margin-right: 7rem; ❌
  margin-right: auto;
}

    width: 100% combined with aspect-ratio: 1 / 1 means that the NFT image will take up all the available space within the card while maintaining a square shape, which allows the image to grow and shrink without being distorted or stretched.

    margin-right: auto pushes the two elements as far apart from each other as possible while still allowing the card to shrink if necessary.

    Other than that, this is a very good solution to this challenge!

    Hope this has been helpful! :)

    Marked as helpful
  • Venus•1,770
    @VenusY

    Good job on completing this challenge!

    I wouldn't recommend using vb values on the widths and heights of the star image and submit button the way you have here.

    This is because when you reduce the height of the viewport enough, you eventually run into an issue where the height of these elements becomes far too small.

    For example, the star image becomes so small that you can no longer tell it's a star, and the submit button gets squeezed to the point where the text overflows outside the box because it's so squished.

    To fix this, you can simply change these values to px values instead:

    img {
  width: 15px
}

button {
  height: 35px;
}

    The width and height values that I've used here are examples, you may adjust them to your liking if you plan on implementing this suggestion.

    I see that you've used padding to try and center the card on the page, but this method only works on a very specific screen size.

    An alternative method you could try is using flexbox and min-height: 100vh like so:

    body {
  padding-top: 200px; ❌
  margin: 0;
  min-height: 100vh;
  align-items: center;
}

    margin: 0 resets the margin added by the user agent stylesheet so that it doesn't interfere with the layout.

    min-height: 100vh sets the height of the viewport to 100vh initially while still allowing it to expand beyond that if necessary.

    align-items: center centers the card vertically on the page.

    While testing the buttons on your page, I noticed that hovering over one of the rating buttons also causes the button before it to change its colour as well. Was this intentional?

    Other than that, this is a very good solution to this challenge!

    Hope this has been helpful! :)

    Marked as helpful
  • Venus•1,770
    @VenusY

    Great work on this challenge! You've done a good job of replicating the design of the page.

    I noticed that the card isn't centered properly on smaller screen sizes/viewports because of the default margin added to the body element by the user agent stylesheet in combination with the width: 100vw property you've added.

    To fix this issue, you could use the following code:

    body {
  margin: 0;
  width: 100vw; ❌
}

    margin: 0 removes the margin added by the user agent stylesheet.

    I also removed width: 100vw as it's redundant. The body element takes up the entire width of the viewport by default, so it's not necessary to specify it to do so.

    I also noticed that the entire card, including the text, shrinks when the viewport's height is reduced.

    While this technically makes your page responsive, it isn't ideal for user experience or readability because it means that the font size is based on the viewport's size, causing the text to be unreadable at smaller viewport heights due to how small it is.

    Not only that, because you've set the font size using vh values, the user can't change the size of the font through their browser settings, which is crucial for accessibility.

    It's important to allow users to increase the font size of your page as needed so that they can view the content comfortably, especially for those who have vision problems.

    Finally, there's no indication that the button is a clickable element because there are no changes in styling, which is important to improve the user-friendliness of your site.

    To improve this aspect of your page, you could change the font and background colours on hover, as well as the cursor type:

    .container .card .buttonSection .leftSide button {
  cursor: pointer;
}

.container .card .buttonSection .leftSide button:hover,
.container .card .buttonSection .leftSide button:focus-visible {
  background-color: #9ab22a;
  color: #ccc;
}

    Other than that, this is a very good solution!

    Hope this has been helpful! If you need me to elaborate on anything, please feel free to message me. :)

    Marked as helpful
  • Venus•1,770
    @VenusY

    Good job on this challenge! You've done well replicating the layout and design of the original design.

    I did notice a few minor things, however.

    The div wrapper element that you've put the hero image inside seems redundant from what I can see. You've only set a width and a height on it, both of which you've also set on the image itself.

    You could achieve the desired layout by removing this .photo element and allowing the hero image to be a direct child of the .card element.

    When the page switches to the smaller layout, the hero image gets slightly distorted because the height of the image is hard-coded to 200px, while the width of it is allowed to change with the width of the card.

    One way you could fix this issue is to add object-fit to the image like so:

    .background .card .photo img {
  object-fit: cover;
}

    Setting this property to cover allows the image to fill its container while maintaining its aspect ratio, but it results in part of the image being cut off.

    If you don't like this side effect, then you can try an alternative method, which is to allow the height of the image to change dynamically.

    This would require you to remove the height property on the img element:

    .background .card .photo img {
  height: 200px; ❌
}

    I also noticed that, if you shrink the width of the viewport enough, you run into an issue where the background no longer takes up the entire viewport.

    If you set the background on the body element, then you can avoid this issue:

    body {
  background: url(data:image/svg+xml,%3csvg%20xmlns='http://www.w3.org/2000/svg'%20width='1440'%20height='437'%3e%3cpath%20fill='%23D6E1FF'%20fill-rule='evenodd'%20d='M0%20349.974c218.558%20116.035%20460.05%20116.035%20724.475%200s502.933-116.035%20715.525%200V0H0v349.974z'/%3e%3c/svg%3e), #e0e8ff;
  background-repeat: no-repeat;
  background-size: 100% 350px;
}

    In order for this solution to work properly, you also have to remove the background property on the .background element, as well as remove the .imgdesk element.

    Other than that, well done once again for completing this challenge! I wish you luck on your coding journey.

    Hope this has been helpful! :)

  • Venus•1,770
    @VenusY

    You've done a great job on this challenge! The site is responsive and resembles the design closely.

    While playing around with viewport sizes, I noticed that when you shrink the height of the viewport, you eventually run into an issue where the card no longer fits on the page and you can't scroll to view the parts that don't fit on top of that.

    This is because you've added overflow: hidden to the body element, which prevents scrolling when there is overflow:

    To fix this issue, you can simply remove this property from the body element:

    body {
  overflow: hidden; ❌
}

    Other than that, this is a very good solution!

    Hope this helps! :)

  • Venus•1,770
    @VenusY

    Great work on this challenge!

    When you reduce the height of the viewport until the card no longer fits on the page in its entirety, you run into an issue where the card is partially cut off, and you can't scroll to view the rest of it on top of that.

    This is happening because you've added overflow: hidden to the body element.

    By removing this property, you'll fix this issue and allow the height of the screen to adjust automatically according to the size of the viewport.

    I would also recommend changing the media query so that the page switches to the mobile version a bit earlier, perhaps at 1100px:

    @media screen and (max-width: 1100px) {
  ...
}

    Of course, you can choose whatever width you'd like, but I recommend going no lower than 1100px as this is the last point at which the site still looks good on the desktop version.

    Any narrower and the whitespace on either side of the card becomes too small and hinders user experience.

    For the mobile version, I would recommend making it more responsive to improve usability on all screen sizes.

    Because you've hard-coded the width of the card to 260px, the width of the card initially looks disproportionate to the width of the viewport. This width also doesn't allow screen sizes narrower than 260px to display the card properly.

    To fix this, you can allow the card to grow to a larger width, such as 500px:

    .left {
  height: 446px; ❌
}

@media screen and (max-width: 1100px) {
  .container {
    max-width: 500px
  }

  .left {
    width: 100%;
  }

  .right {
    width: 100%;
    height: auto;
    aspect-ratio: 500 / 413;
  }

  .right img {
    width: 100%;
    height: 100%;
  }
}

    I've set aspect-ratio: 500 / 413 on the .right element as this allows it to grow along with the viewport size while maintaining the aspect ratio of the image so that it doesn't get distorted.

    Another thing you should consider adding is some padding to your page. While this isn't strictly necessary, it improves visual balance which therefore improves the user experience of your page.

    @media screen and (max-width: 1100px) {
  body {
    padding: 20px;
  }
}

    Other than that, this is a very good solution to this challenge!

    Hope this has been helpful! :)

View more comments