Zuliatu• 140
@zuliat
Submitted
I think this project was quite easy for me but any feedback on how to make it better would be highly appreciated.
notabhishekrai
@notabhishekrai
All comments
- notabhishekrai• 180
@notabhishekrai
Posted
Hey buddy. It seems that you have forgotten to push the image in github. Thats why there is no QR image only alternative text in your card. Check again.
Also: you can center content with just body{ display: grid; min-height: 100vh; place-content: center; }
Marked as helpful
0 - Francisco Prado• 80
@franciscoprado4
Submitted
Another challenge for this awesome page, any suggestion is welcomed.
Regards to Y'all.
Francisco.
notabhishekrai• 180@notabhishekrai
Posted
pretty good job
however, the background image is messed up in large screen view and mobile view.
few suggestion:
- use mobile first approach and work your way up
- your card is the <main> and your <body> is container in this case, so you don't need that extra div wrapper.
- use body{ display: grid; place-content: center; } that will center the card for you.
- use ::before and ::after in body to add those bubbles (pseudo classes), you can get rid of two more divs that way.
- its good practice to not give height to the elements but in this case it won't do harm so just keep this in mind.
other than that good job. keep stylin !
P.S you can use my solution as reference, its not perfect but i hope you can learn new things;
0 - PLuciano Lima• 1,290
@LucianoDLima
Submitted
I tried again using the BEM format, I think I did it right ? Although I did not use a Modifier at all.
Also it was very hard to figure out a way to properly position the background since I never placed 2 backgrounds at once before.
notabhishekrai• 180@notabhishekrai
Posted
wow you somehow seem to manage that with position but thats a lot of maths. use css grid, trust me its way way way better and cleaner.
good job with the BEM, you can substitute double underscores with just - it will be more cleaner you don't have to follow BEM 100%, just the concept is enough, use - to separate block and element, trust me it will be more cleaner and readable
few suggestions:
- in ::before and ::after, you can add image through content: ''; its done by content: url('path/image.png'); don't use background property in pseudo elements. and use left, right, top and bottom in percentage to position them absolutely as you like.
- i think there is a typo, i suppose you meant to write clip: rect(0,0,0,0); but you have written clip:rect(0,0,0,0,0); (one more 0) remember a rectangle has 4 side >_o
other than those pretty good job. and good job on focusing on accessibility like using .sr-only.
you can also use my solution as reference good day! 2)
Marked as helpful
1 - Diego_• 160
@Dialtor
Submitted
notabhishekrai• 180@notabhishekrai
Posted
pretty impressive. Few suggestions:
- use max-width on the container so that your cards don't stretch in bigger screens.
- "Reliable, efficient delivery" and Powered by Technology are two different headings. I see you have used span tag inside the h1, in my screen (1920px) they come in the same line so use h1 and h2 in respective order.
other than that, pretty good. 1 like for hard work
0 I have finished this one after ages. I struggled so much with getting the background correct but I still haven't managed to do it in a responsive way.
If you could give me a hint as to how to do the background so it works well when responsive I'll let you feed my pet duck.
Thank you
notabhishekrai• 180@notabhishekrai
Posted
pretty good
but you should use ::before and ::after in the body and then set the background color in the body. with before and after, you can position the images as you like. It should solve your responsive issue.
I see you have used those svg as background.
you can use my code as reference https://github.com/notabhishekrai/profile-card-component
although it isn't perfect, i hope you can learn something from it, also you can give your feedbacks.
0- notabhishekrai• 180
@notabhishekrai
Submitted
/D O N E/ Feedbacks are highly appreciated.
notabhishekrai• 180@notabhishekrai
Posted
I was experimenting attribute selector in css ([role="value"]). Hence, the mess.
0 - Dizzy_Sloth• 50
@TotallySly
Submitted
I really struggled in producing the Overlay. I had to look at a lot of different solutions in order to figure out how this worked. After completion, I am now wondering if the hover elements should have been placed within Anchor tags.....
I need to improve my knowledge of accessibility and aria-labels. Position: Relative and Absolute.
I have not used it on a responsive challenge yet, but I am wondering if using 'Max-Width' helps responsiveness? To my ignorant self, it feels like that should limit the amount of Media Queries. But I know nothing.
Thank you
notabhishekrai• 180@notabhishekrai
Posted
Great Job! But you should wrap all the code inside a <main> </main> (replacing div class = "container" with main class="container" should do) and use atleast 1 H1 tag. These two should solve your accessibility issues.
for the hover effect, you can just use position relative to the parent and position absolute to child with top and left 0.
max-width does help in responsive design. If you use max-width of 200px for an image, it will resize when you decrease the screen size but it wont resize(stays the same) when you increase the screen size (to whatever size). Unlike the normal width property.
Marked as helpful
2 - Dennis Griffin• 1,320
@dgdev1024
Submitted
Honing my experience with Next.JS with this project. This submission contains a bonus feature which allows users to create their own QR codes, which will be persisted in their browser's
localStorage.notabhishekrai• 180@notabhishekrai
Posted
Wow! This is crazy dude. I have tested the QR and it works. Although I don't do js, this is very impressive!
0 - abbos• 460
@shomurodovabbos
Submitted
!
notabhishekrai• 180@notabhishekrai
Posted
Impressive! Although I would recommend more padding left and right to the text and border radius of around 17-18 px on the card and around 10 to 12 px to the image. Looks like you have given a fixed height to the card Its not necessary to give height. But looks close enough. Hats off.
0 - MauriceHuang• 30
@MauriceHuang
Submitted
This is my first attempt. Feel free to let me know any areas that I can improve.
Particularly, how I could have written to be clear and align with the best practices.
Thank you!
notabhishekrai• 180@notabhishekrai
Posted
Other than the box shadow, I don't see any flaws in the design. Great work!
use box-shadow: 4px 2px 20px hsl(212, 45%, 89%);
Marked as helpful
1
