Latest solutions
SUNNYSIDE AGENCY LANDING PAGE using CSS Grid, Vanilla JavaScript
#accessibility#bemSubmitted about 3 years agoTIME TRACKING DASHBOARD using CSS Grid, Flexbox, Vanilla JS, and BEM
#accessibility#bemSubmitted over 3 years agoHUDDLE LANDING PAGE (SINGLE) using CSS Grid, Flexbox, and BEM
#accessibility#bemSubmitted over 3 years ago
Latest comments
- @LuisJimenez19@kenreibman
Hi @LuisJimenez19 ! Great work on this.
There are a few ways you could approach this issue. I'm going to give you my take on it. Hopefully you can understand, and it would help!
-
First off, I would change your
card__image--active
into ana
(anchor) tag. -
I would change that into an anchor tag because it makes it more user-friendly to click on the NFT image. It's a little bit of a pain for some users when you have to click on very specific spots of a card/image to open the link. It's better when you just make the whole container click-able and make the actual button (the eye) a decoration, or a "fake button" as I like to call it. The user will still think the eye is a button.
-
You don't need an active class in this situation since you're not using JavaScript. I would remove all the hover states you have right now. I would also remove the opacity and background-color on your card__image--active.
-
Add a
position: relative
to your card__image--active, move your background-image from your card__image div to the card__image--active div and make a pseudo element on that. So it would be:
.card__image--active { ... other styling background-image: url(../images/image-equilibrium.jpg); position: relative; } .card__image--active::after { content: 'url(image)' /* <-- this is your eye image */ position: absolute; top: 50%; left: 50%; transform: translate(-50%, -50%); opacity: 0; visibility: hidden; /* (the top, left, and transform properties centers the eye on the container) */ }
- Then set a hover state on your card__image--active and your card__image--active::after (pseudo element)
.card__image--active:hover { opacity: 0.8; /* this lightens the image */ } .card__image--active:hover::after { visibility: visible; opacity: 1; /* this makes the previously hidden eye icon (opacity: 0) appear when the card__image--active div is on hover by the user and changes it to (opacity: 1) */ }
Basically the logic in this, is that the eye icon is currently hidden indefinitely. Only when the user hover overs the --active container, it will become visible, and at the same time the container behind it has its opacity go down. This works because the pseudo element wont receive the opacity effect and they will work simultaneously.
I hope this helps! I'll be honest, I did not test any of the css when I wrote it. so please get back to me if there are any issues.
Marked as helpful -
- @DaliaMuj@kenreibman
Hi @DaliaMuj ! I'm happy that you enjoyed the project, you did a great job!
I have a few things that stand out to me which I hope would benefit you:
-
The first thing I notice is your background. Most times when you set a background with an image, your image will repeat indefinitely. Sometimes it's great. However in this challenge, to match the original design, you only need one background image.
-
In order to prevent that, in your css, give the
body
where thebackground-image
is being set, another line called:background-repeat: no-repeat
. -
Then give it another line
background-color:
and set the color to the slightly lighter blue that the ReadMe file gave you, and you've got a very similar looking background to the original! -
There are some weird horizontal scrolling in your project which I also fixed by change your
height: 100vh
tomin-height: 100vh
-
I would also give your
body
amargin: 0
andpadding: 0
to reset any default margins and padding that html puts onto your elements. -
Usually you want your site's content to be wrapped in a
main
element. I would change yourdiv
with the class ofcontainer
to amain
element with the class ofcontainer
to make it semantically correct.<main class="container"></main>
-
The two buttons - "Proceed to Payment" and "Cancel Order" are currently
input
tags which are strictly used for form fields, like when you want a user to input their email/password. In this case, you wouldn't want that here. Instead I would use ana
tag (anchor tag) and style them. You did it correctly for the "Change" button. Anchor tags are also more "style-able" if you give it adisplay: block
as well. -
Also remember to style your :hover and :focus states for those two buttons to visually show that it is interactable.
I hope this helps, great job! Keep it up!
-
- @AmritPal91@kenreibman
Hi @AmritPal91 ! Great work :) Your site is nicely done using semantic elements!
Here are some suggestions that I have:
- I noticed that your navigation bar is not properly structured and styled. That's probably why you had a lot of trouble making it responsive, and you were forced to switch to the tablet layout very early. I always recommend using Flexbox to style your navigation. Looking at your site, I don't see any uses of Flexbox, and you will realize how easy it is to align content using Flexbox.
- Take a look at this video for making a fully responsive navigation menu using flexbox:
- Also take a look Kevin Powell's channel for countless flexbox tutorials like this:
If you are serious in learning web development, I recommend making this site again, or choosing a very similar challenge on here using Flexbox.
I hope this helps! I can tell you're on the right track, you just need the right tools :)
- @luis08201@kenreibman
Hey @luis08201 ! Great submission. I love how you pushed yourself in using SCSS. Some suggestions from an industry standpoint:
-
Check your accessibility report that Frontend Mentor offers you, you have quite a few accessibility and HTML issues.
-
Your BEM naming is a little off. You're giving modifier classes too often; Usually to every third word in your element class name. the double hyphen
--
are for modifier events likebtn__large--active
orphoto__img--highlighted
in your case,header--nav
or--link
should be__link
ormain-header__nav
simply put, you should rarely have to use -- -
I see you're using
section
elements which is a great step into a well-structured page. However, you're lacking amain
element that surrounds thosesection
tags to make it semantically correct. You could wrap all your content inside thebody
tag with amain
element. -
I also suggest you put a nice descriptive alternative text
alt=""
whenever you use an image for accessibility purposes. Same goes for your anchor links as well. I see you're getting a lot of those errors in your footer. -
If your image is purely decoration and you absolutely believe that it doesn't need descriptive text for accessibility, you can hide that from screen readers by putting an
aria-hidden="true"
in yourimg
tag. You can read more about that here: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attributes/aria-hidden
I hope this helps, you're doing great!
Marked as helpful -
- @AnazAnoiar69@kenreibman
Professionally and personally I use the clamp() function for truly responsive typography. This will genuinely take your responsive game up to a professional level.
Check out this video: https://youtu.be/U9VF-4euyRo
- @dannyguzman31@kenreibman
Nice job on the submission! Unfortunately the preview image isn't appearing on here, but I looked at your live site and it looks great.
I noticed you were centering the
div
twice with yourbody
andcontainer
. Yourbody
could have been yourcontainer
in this case, and thecontainer
could have been youritems
. In my opinion it was not necessary to do that.I would also recommend when you're centering a
div
to usemin-height: 100vh
instead ofheight: 100vh
to make responsiveness easier in future projects. Setting a fixedheight
like that may bring some issues in bigger projects. I would also stray away from setting a fixedwidth
like100vw
, as well, for parent elements. The setwidth
also applies to containers, or cards as well. I would set amax-width
instead, which is more responsive friendly, and you can also reassign the dimensions in a media query when the screen gets bigger/smaller.As you do bigger projects on here, you should also start giving your elements more "meaningful" names. Always think about someone else reviewing your work, and wondering if they would be able to understand what each line is referring to. If it was a bigger project, I would have no idea what
p1
andp2
is referencing. Start using naming conventions like BEM early, and maybe start putting comments in your code as well, which will definitely help people in this community when they review your code.I hope this helped!
Marked as helpful