Hello guys, please advice. I think my css file could be simplify, but I am struggling with padding and margin. I try to set media query with @media only screen and (min-width: 375px), but it does not work on Iphone 6/7/8 (display as desktop design), so I set to min-width 768px. I use Viewport resizer to check it.
Josh
@Ao-chiAll comments
- @startoverflowsSubmitted almost 2 years ago@Ao-chiPosted almost 2 years ago
Hi~ Congrats on finishing this challenge!
Some tips to make it better for both mobile and desktop versions.
- For the
<html>
and<body>
elements, you could give it aheight: 100%
. - To keep the card component always on center you can get rid of your widths and heights in your
.wrapper
class like this:
.wrapper { height: 100%; display: flex; flex-direction: column; justify-content: center; align-items: center; background-color: var(--Cream); background-color: var(--Cream); } @media only screen and (min-width: 768px) { .wrapper { flex-direction: row; } }
For accessibility and usability here are some tips.
- You can change your
<div class="wrapper">
to<main class="wrapper"
. It will help screen readers and other assistive technologies understand the **structure and hierarchy ** of the content on the page. - Also you can replace
<div>
with headings such as<h1>
to give emphasis on the main topic of the content. You could do this to your.preview
class, like this"<h1 class="preview">Gabrielle Essence Eau De Parfum</h1>
- For the Add to Cart you can remove the
<h1>
element to and just wrap the icon and the text inside a<button>
like this:
<div class="add-to-cart"> <svg width="15" height="16" xmlns="http://www.w3.org/2000/svg"> <path d="M14.383 10.388a2.397 2.397 0 0 0-1.518-2.222l1.494-5.593a.8.8 0 0 0-.144-.695.8.8 0 0 0-.631-.28H2.637L2.373.591A.8.8 0 0 0 1.598 0H0v1.598h.983l1.982 7.4a.8.8 0 0 0 .799.59h8.222a.8.8 0 0 1 0 1.599H1.598a.8.8 0 1 0 0 1.598h.943a2.397 2.397 0 1 0 4.507 0h1.885a2.397 2.397 0 1 0 4.331-.376 2.397 2.397 0 0 0 1.12-2.021ZM11.26 7.99H4.395L3.068 3.196h9.477L11.26 7.991Zm-6.465 6.392a.8.8 0 1 1 0-1.598.8.8 0 0 1 0 1.598Zm6.393 0a.8.8 0 1 1 0-1.598.8.8 0 0 1 0 1.598Z" fill="#FFF"></path> </svg> Add to Cart </div>
That's all! If you have anymore questions fell free to contact me. Happy Coding~
Marked as helpful1 - For the
- @Godstime01Submitted almost 2 years ago
I don't know how to fix the hover effect on the contact link in the navigation section. i tried using backdrop blur effect but it didn't work
@Ao-chiPosted almost 2 years agoHi~ Congrats on finishing this challenge!
One way to solve the hover effect on contact link is by making a new variable with lighter opacity of your
--white
color like this:--light-white: hsl(0, 0%, 100%, 0.4);
. Now you can use this as the background color for hover effect.._links:last-child:hover { color: var(--white); background-color: var(--light-white); }
Also to make your links accessible for screen readers, it's advisable to put discernible text like this
<a href="#">Home<a>
. But if you'll only put an icon or image you should addaria-label
attribute. For example:<a href="#" aria-label=""Website logo> <img src="images/logo.svg" alt="icon"> </a>
If you have any questions you can reach out to me. Hope this helps~ Happy Coding!!
Marked as helpful1 - @franklpjrSubmitted almost 2 years ago
This project was so amazing, but i still have some difficult with the "hamburger" menu. Could someone help me? Thanks a lot!
@Ao-chiPosted almost 2 years agoHi! You did great doing this challenge!
I saw your navigation menu on mobile version and it looks like it is not on top of the body. To make it appear on top you could add
z-index: 1
to your nav-menu class.Some tips for best practices.
- You should wrap your main contents into
<main>
element for more accessibility. - Add an alt attribute to your
<img>
elements like this<img src="/images/client-databiz.svg" class="clientimg" alt="hero banner">
- You should not wrap a
<button>
inside an<a>
element and vice versa. It is for accessibility practice. You can use either one of them. I you want the button to have a cursor pointer, you can just add this on your css stylingcursor: pointer
Hope this helps! Happy Coding~
Marked as helpful0 - You should wrap your main contents into
- @jjfuentes13Submitted almost 2 years ago
Found issues trying to import google fonts, what changes or mistakes did I make?
What are some best practices to have divs be centered?
@Ao-chiPosted almost 2 years agoHi! For the google fonts, I think you did it right, it's just that Fraunces font is nowhere to be found in google fonts (at least on my end).
As for centering the div there are several ways and this is one of it. I checked your code in chrome dev tools and found some improvements to make. First, you can add a
height: 100%
to both your<html>
and<body>
elements. Then you could get rid of the margins on your container class<div>
like this:.container { grid-template-rows: 0; grid-template-columns: repeat(2, 1fr); width: 600px; height: 450px; }
After that you can add this to your body styling:
body { background-color: hsl(30, 38%, 92%); margin: 0; height: 100%; display: flex; align-items: center; justify-content: center; flex-direction: column; gap: 10px; }
This is one way you could do it.
Also I found that you nested an
<a>
element in your button. you can get rid of the<a>
also for it is not a valid markup as per HTML5 Spec Document from W3C.For the
<img>
you should add an alt attribute like this<img class="cart" src="images/icon-cart.svg" alt="cart icon">
Hope this helps! Have a nice day~
Marked as helpful0 - @CodeXsubhamSubmitted almost 2 years ago
Can someone correct my mistake ...when i was trying to position logo of eth and clock just beside stats and timing ...its was not coming just beside them.
@Ao-chiPosted almost 2 years agoHi! I looked on to your solution using the browser's dev tools and I find that you assigned
display: inline-block
to your parent div small-eth and days. The display property is not inherited by its children that's why the icon and text doesn't go side-by-side.To fix that you can add the
display: inline-block
to your child elements like this:.small-eth p, .days p { display: inline-block; }
and you could reduce the padding on the
.days p { padding: 95px }
to.days { padding-left: 67px px;}
.Hope this helps~
Marked as helpful0 - @augustine-a8Submitted almost 2 years ago
How to implement the background overlay when the mobile nav bar is open
@Ao-chiPosted almost 2 years agoHi Augustine!
Just some tips on the overlay. To solve this you can add another <div> element and give it a class of "overlay" or whatever name you'd like. Something like this:
<div class="overlay"></div>
and style it on css with this:.overlay { display: block position: fixed; top: 0; right: 0; left: 0; bottom: 0; background-color: rgba(4, 2, 15, 0.6); visibility: hidden; opacity: 0; z-index: 1; } .overlay.active { visibility: visible; opacity: 1; }
You can toggle the active class with javascript every time you open the nav on mobile. Hope this makes sense and wish I could help! Happy coding~
Marked as helpful0 - @overuseofremSubmitted almost 2 years ago
I found trying to make the page properly responsive really difficult although the end-product is OK. I built it mobile-first and whenever I go from mobile to desktop, the navigation moves outside the page. Haven't found a good way to deal with it yet and it's due to the buttons I used for the mobile navigation. Also, the overlay that happens when the mobile navigation is on screen is weird, the rest of the background darkens but images and buttons remain bright. I haven't figured out a way to fix it, it's the first time I've ever tried javascript. If you have any advice of feedback, I'd be happy to here them!
@Ao-chiPosted almost 2 years agoHi Francine, Congrats on finishing the challenge.
Just some tips on the overlay. I found that you are only changing the body's background color that is why it looks like the images and other text are on top of it. To solve this you can add another <div> element and give it a class of "overlay" something like this:
<div class="overlay"></div>
and style it on css with this:
.overlay { display: block; position: fixed; top: 0; right: 0; left: 0; bottom: 0; background-color: rgba(4, 2, 15, 0.6); visibility: hidden; opacity: 0; z-index: 1; } .overlay.active { visibility: visible; opacity: 1; } @media (min-width: 992px) { .overlay.active { visibility: hidden; opacity: 0; } }
You can toggle it with javascript every time you open the nav on mobile. Hope this helps! Happy coding~
Marked as helpful2