Venus
@VenusYAll comments
- @Xandros9Submitted 9 months ago@VenusYPosted 9 months ago
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 tofit-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 withaspect-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 helpful1 - @progAreejSubmitted 9 months ago@VenusYPosted 9 months ago
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 helpful0 - @hossamzayed201Submitted 9 months ago@VenusYPosted 9 months ago
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 helpful1 - @EminAbdullayev1998Submitted 9 months ago@VenusYPosted 9 months ago
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! :)
0 - @Udochi17Submitted 9 months ago
I would love your feedbacks.
Thank you.
@VenusYPosted 9 months agoYou'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! :)
0 - @KTshibanguSubmitted 9 months ago@VenusYPosted 9 months ago
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! :)
0 - @RodRyan19Submitted 9 months ago
Hi! I would love to hear some feedback if there things I did wrong.
@VenusYPosted 9 months agoHey! I had a look at this project as you requested, and it's really good! Almost no issues at all from what I can see.
The site is responsive, and it resembles the design very closely.
The only thing I would suggest is that you should consider using
padding
instead ofmargin
for the whitespace around the edges of the page.This is because using margin makes it so that the site has scrolling at all times, even if the user's screen size is tall enough to fit all the content.
Using
margin
causes this issue because it's not considered in the calculation of the element's width or height, even if thebox-sizing
is set toborder-box
.border-box
only includes padding and border as part of it's width or height.So this in combination with
min-height: 100vh
means that the height of the webapge is always going to be 100vh + whatever the margin is, causing the scrolling issue.I hope this explanation has been helpful, but if not, please feel free to ask me questions, and I'll try my best to clarify.
body { margin: 2rem; ❌ padding: 2rem; } @media screen and (min-width: 800px) { body { margin: 4rem; ❌ padding: 4rem; } }
Other than that, you did a fantastic job at this challenge, and I can see that you're improving a lot! :)
Marked as helpful1 - @Souheib-AlouiSubmitted 9 months ago
Not quite sure if my colors are correct and if media queries are necessary . Any feedback would be welcomed.
@VenusYPosted 9 months agoGreat work on this challenge! You've done well replicating the design, and the site is very responsive on top of that.
Your colours seem correct to me, but if you're worried about them, you can use this image colour picker site to get the exact colours that you need: https://imagecolorpicker.com/
All you need to do is take a screenshot of the thing that you want to extract the colours from, and then paste it directly into that website. No need for downloading and uploading the image, so it's very simple and easy to use.
Alternatively, you can use the 'Color Picker' tool from Microsoft PowerToys, which is an immensely useful tool that allows you to pick colours directly from the screen.
If you haven't got Microsoft PowerToys yet, you can install it from the official Microsoft site right here: https://learn.microsoft.com/en-us/windows/powertoys/
As for the site itself, one minor thing I would suggest is adding some padding to your page:
body { padding: 20px; }
I used 20px as an example, but you may of course adjust the value to your liking.
While this change isn't strictly necessary, it's highly recommended as it makes for a better user experience in most cases.
This is because whitespace is good for the visual balance of a page, and in some cases, it even improves readability.
Other than that, well done once again for completing this challenge!
Hope this has been helpful! :)
1 - @OsmarPESubmitted 9 months ago
I didn't know how to position the background correctly, any suggestions?
@VenusYPosted 9 months agoGreat work on this challenge! It's responsive, and you've done a good job of replicating the design.
I did notice one small thing though, which is that the buttons aren't the same sizes, nor are they square.
This is happening because you're allowing their sizes to be determined by the icon sizes and the padding around them.
To fix this, you could set a hard-coded width and height, and then center the icons inside them with flexbox:
.card__link { width: 50px; height: 50px; display: flex; justify-content: center; align-items: center; }
I also noticed that the content gets partially cut off when you shrink the height of the viewport enough.
To fix this, you could simply add replace
height
withmin-height
for the body element:body { height: 100vh; ❌ min-height: 100vh; }
Other than that, this is a very good solution!
Hope this has been helpful! :)
Marked as helpful1 - @tanasegabrielwSubmitted 9 months ago
My process
- Outline the structure of the page
- Structure the HTML
- Style the CSS
- Fix small details
@VenusYPosted 9 months agoGreat work on this challenge! The card looks almost spot on to the original design.
I noticed that the card gets partially cut off at the top when you shrink the viewport's height.
This is happening because you've set the height of the body element to 100vh.
You can fix this by changing
height
tomin-height
:body { height: 100vh; ❌ min-height: 100vh; }
The card also isn't responsive to changes in viewport width.
If you shrink the viewport width to the point where the card no longer fits on the page in its entirety, you run into the issue where the card gets partially cut off again, but this time on the left side of the card.
This is happening because you've hard-coded the width of the card:
.card { min-width: 21rem; max-width: 21rem; }
This code is essentially the same as if you were to set the width of the card to 21rem with the
width
property, causing the card to be stuck at 21rem regardless of the screen size.You can fix this issue by simply removing
min-width: 21rem
.This will allow the card to grow up to 21rem, at which point it will stop growing.
Other than that, this is a very good solution, and well done once again for completing the challenge!
Hope this has been helpful! If you have any further questions, please feel free to ask me. :)
Marked as helpful0 - @abdellah-abadouSubmitted 9 months ago
Not responsive but similar to the design
@VenusYPosted 9 months agoGreat work on this challenge! You've done a good job of replicating the design of the page, and the layout is spot on.
In order to improve the responsiveness of your page, there are a few things you could change.
The first thing is that you could remove the
height
property of the body element and replace it withmin-height
:body { height: 100dvh; ❌ min-height: 100dvh; }
This sets the initial height of the body to 100vh but allows it to expand if necessary to accommodate the content.
I noticed that a white box appears behind the card at certain viewport widths.
This is being caused by the main tag having a hard-coded height. If you remove
height: 700px
from the main tag, it will get rid of this problem.While the card is responsive the changes in screen width in the sense that it shrinks and grows along with the viewport, at some screen sizes, you can see the layout breaking a little bit, such as the image not spanning the whole width of the card.
For your card to be more responsive to smaller viewport widths, you could make these changes to your code:
.card { align-items: center; ❌ } .card > div { padding: 0 50px; } .card div { width: 80%; ❌ }
align-items: center
was causing the image to not be able to take up the full width of the container at certain viewport sizes. Removing it solved this issue.However, removing it also caused the div element to no longer be centered in the card.
Adding
padding: 0 50px
and removingwidth: 80%
at the same time achieves the same layout that you had before while still allowing the content to be centered.Other than that, this is a very good solution to this challenge!
Hope this has been helpful! :)
Marked as helpful1 - @Darkx-devSubmitted 9 months ago@VenusYPosted 9 months ago
Great work on this challenge! You've done a great job of replicating the design and layout of the card.
I noticed that if I shrink the viewport height enough, the card no longer fits on the page and gets partially cut off at the top.
This is happening because you've hard-coded the height of the body to 100vh.
To fix this issue, you can simply add
min-height: 100vh
to the body element:body { height: 100vh; ❌ min-height: 100vh; }
A very minor thing I noticed was that you wrapped the 'of' in the bottom part inside the span tag as well when it should only be the name.
Additionally, since the creator's name is meant to be a link that takes you to the page of the creator, you should wrap the name in an anchor tag instead of a span tag:
<a href="#">Jules Wyvern</a>
The name of the NFT should also be a link:
<h3 class="heading"> <a href="#">Equilibrium #3429</a> </h3>
You should also be able to hover over the NFT image to get the overlay to appear with the view icon.
You can achieve this in a number of ways, but the best way to my knowledge that works with your HTML markup is this:
1. Make the card responsive:
.component { width: min-content; ❌ max-width: 290px; } .NFT { height: 250px; ❌ width: 100% aspect-ratio: 1/1; }
2. Remove the whitespace underneath the image with
display: block
:img.NFT { display: block; }
3. Add a div tag with the class
.overlay
as a child of thediv.NFT
element:<div class="NFT"> <img class="NFT" src="images/image-equilibrium.jpg" alt=""> <div class="overlay"></div> </div>
4. Position this new element absolutely, and position the div.NFT element relatively:
div.NFT { position: relative; } .overlay { position: absolute; top: 0; left: 0; }
5. Give this new element a width and height of 100%, an opacity of 0, and a transition:
.overlay { opacity: 0; width: 100%; height: 100%; transition: opacity 100ms linear; }
6. Add the view icon as a background image to the
.overlay
element:.overlay { background-image: url([insert path to view icon image file here]); background-repeat: no-repeat; background-position: center; }
7. Change the opacity to 1 on hover or focus:
.overlay:hover, .overlay:focus-visible { opacity: 1; }
8. Change the cursor to a pointer when the user hovers over the
div.NFT
element:div.NFT:hover { cursor: pointer; }
I apologise for the super long message. If it was difficult to digest, or if you have any problems or questions at all, please feel free to message me, and I'll do my best to help you.
Other than that, well done once again for completing this challenge!
Hope this has been helpful! :)
Marked as helpful1