
Azhar
@azhar1038All comments
- @oyeyinkaojora@azhar1038
Hi Oyeyinka,
For navbar try checking out tutorial by Kevin Powell in youtube. It is not exactly for this but he has shared some important tips and ideas that will help you.
Marked as helpful - @VijayalakshmiGanesh@azhar1038
Hi Vijayalakshmi, congratulations on completing the challenge!
For this challenge, positioning those two circles is the most difficult part. You almost got it correct but try positioning those relative to the card itself instead of body.
For centering the card, you can try using flex or grid instead of using absolute positioning. You may check this article How to Center Anything in CSS Using Flexbox and Grid
Hope it helps :)
Marked as helpful - @ShahbaazX786@azhar1038
Hi Shaik, congratulations on completing the challenge!
I see you have used fixed width for text wrapper and image, this is causing the overflow issue in small mobile view. Try using
width: 100%
instead of fixed value. You will also need to adjust your button to make its width relativeAlso there are 2 different images provided for mobile and desktop. Try using
<picture>
to display different images based on media query directly from html.Regarding using bootstrap for grid, why don't you just use CSS Grid!
Hope it helps :)
- @krishna-cyber@azhar1038
Hi Krishna, Nice work!
I have few suggestions:
- Try to use radio button to create the ratings instead of using span as it can cause accessibility issues.
- When the width of screen is around 700px, the component gets squeezed and then expands around 600px
- Background of star icon on top left becomes orange upon hovering thus hiding the star
Hope it helps :)
Marked as helpful - @Abdul-Hamyd@azhar1038
Hi @Abdul-Hamyd,
You need to create a default repo named Abdul-Hamyd.github.io and do some settings for github pages to work.
You may check the docs Github pages
Hope it helps :)
- @gustavoagoncalves@azhar1038
Hi @gustavoagoncalves,
May I know why you didn't uploaded compiled css because for the live site you need css?
It makes it difficult to give feedback because not everybody will clone the repo to check.
- @DevWanderson@azhar1038
Hi Wanderson, congratulations in completing the challenge!
To center the card, I see you tried to use flex but then went with transform. You can simply do this:
body { display: flex; height: 100vh; width: 100vw; align-items: center; justify-content: center; }
This will bring your card to center.
I also see that your cart icon is not visible. Your
src
should beimages/icon-cart.svg
but you have used/images/icon-cart.svg
. The front/
takes to root i.ehttps://devwanderson.github.io/
.In the challenge two different product images are provided, one for mobile and other for desktop, you can use
<picture>
to specify which image to load on which screen using media query.You should also make your card more responsive by using
min-width
ormax-width
instead of fixed width. You should update your media query too so that you switch to mobile view before it compresses to much. Remember the width provided in style-guide are just for reference related to the picture provided or the width that frontendmentor uses to generate screenshot. So, don't just restrict on using those numbers.Please update your code link too. It gives 404 currently.
Hope it helps :)
Marked as helpful - @ali007-depug@azhar1038
Hi Ali, congratulations on completing the challenge!
You can try using
<picture>
and load the image based on media query instead of loading both images, something like this:<picture title="Perfume bottle"> <source media="(min-width:40em)" srcset="./images/image-product-desktop.jpg"> <img src="./images/image-product-mobile.jpg" alt="Perfume bottle"> </picture>
I also noticed this media query:
screen and (min-width: 375px) and (max-width: 400px)
Now because of this in screen smaller than 375px, your design falls back to desktop mode and breaks. You don't need thatmin-width
and also update your max-widthYou are using fixed height and width on images, try to use
max-width
for image so that your card can be more responsive in smaller screen.Your button is also overflowing your card, so you need to update your spacing.
Hope it helps :)
- @jvmcpheron@azhar1038
Hi Jane, wonderful work there!
Positioning the popup in desktop was probably the most difficult part of this challenge (at least for me 😅). For x-axis I used
%
value instead of fixed amount along withtranslateX
andcalc
to position the popup.First you will have to position that section w.r.t that bottom footer where the share button is present, then do:
.popup { left: 100%; transform: translateX(-50%); }
This will move the center of popup to the right boundary. Then do some calc to negate the padding and half of the share button size to move it left to the center. Check out my solution: Article preview component
Another accessibility tip, the extra close button that you have added, try to make it a button instead of div and give it an
aria-label
if you are using some fancy characters which screen readers may not understand.One small issue I noticed with mobile view is the size of card changes when share menu opens because author part and share menu have different height.
Hope it helps :)
Marked as helpful - @iDominate@azhar1038
Hi Ahmed Ghanem, congratulations of completing the challenge!
I see that you have broken the heading in to four
h4
, this is not good. You should use only one heading and let the content wrap automatically. Same goes for thep
Then the share popup that appears on hover is impossible to click. It goes away as soon as you try to move the cursor. Try using clicks instead of hover.
Your card position seems to be jumping to a different location when screen size is around 550 - 1000px.
You should try to change the size too as it looks small and use relative size like min-width or max-width instead of fixed height and width.
Hope it helps :)
- @HunterMcGrew@azhar1038
Hi Hunter McGrew, congratulations on completimg this challenge!
I noticed one small issue in mobile view, when you click on the share button, height of card changes. This is due to different height of share menu and author details.
An accessibility tip is to use
button
for share icon instead ofdiv
. Since this button contains image instead of text, make sure to give anaria-label
and setaria-hidden
to the child image. Also you have used section unnecessarily. There is no need to put an image inside section and your header and paragraph can go in same section.Try to use
hn
tags for heading instead of paragraphs.Hope it helps :)
- @lekdup@azhar1038
Hi Tenzin, nice work there!
I noticed one small problem, when you switch to mobile view at around 1000px, the cards become too wide. You should add some appropriate
max-width
to them or even better if you introduce a new design for medium size screen like a 2x2 grid.Hope it helps :)
Marked as helpful - @AniketSaini98@azhar1038
Hi Aniket, nicely done!
I noticed that you have used
@media screen and (max-width: 1440px)
to handle the sizing, so in larger screen > 1440px, your card is taking whole width and breaking the design. You should not use such media query. It is present in design document to tell that frontendmentor uses that to generate images.Another problem is your eye image that appears on hover, it is not responsive, so in smaller screen it does not cover whole picture. You can wrap your images in
div
and position it absolutely w.r.t that inner div instead of main container.Hope it helps :)
Marked as helpful - @MahdiRezaeiDev@azhar1038
Hi Mahdi, Great work there!
I noticed some accessibility issue in your solution.
- Unnecessary use of
header
. I don't think you should use header in those cards. They are more like quotes so maybe just simple hn tag should be sufficient. And you can useblockquote
tag too instead ofdiv
for those cards too. - The star images are just decorative, so just hide them from screen readers using
aria-hidden
. I will suggest to also hide the avatar images.
If you are using chrome, I will suggest to turn on Accessibility Tree view. It can really help to visualize how screen readers will see your content.
Marked as helpful - Unnecessary use of
- @Xarkdagor@azhar1038
Hi Sameer, congratulations on completing your first challenge!
For centering stuff, using transform is a nice way, but we have better options now. Try using display flex or grid. For flex, you just need to do:
.wrapper { display: flex; align-items: center; justify-content: center; }
You can find lots of articles and videos about flex and grid. Here are some links from css-tricks:
- https://css-tricks.com/snippets/css/a-guide-to-flexbox/
- https://css-tricks.com/snippets/css/complete-guide-grid/
Also I will suggest not to use flxed height and width as they make it less responsive, you should try to use
min-width
,max-width
etc. or you can try new css logical functionsmin()
,max()
,clamp()
: https://web.dev/min-max-clamp/Handling images is really difficult, so I usually make them
display: block
and wrap with extra div if required.Hope it helps :)
Marked as helpful - @Aloy100@azhar1038
Hi Alloysius,
You can use github pages. Just create a repository as
<your-github-username>.github.io
and you can use it to host the challenges. I will suggest to create a folder per challenge and then you can access them like<your-github-username>.github.io/challenge-name
You may check the official docs for more information: https://pages.github.com/
Another option is Netlify
- @jwalczak94@azhar1038
Hi Joanna, great work!
One thing I noticed is when I open in mobile view, the position of circles is not correct. If you check the design you will notice that the circles are always on left and right side of card. To achieve this, I positioned the first circle from bottom right and second from top left, like this:
background-image: url(images/bg-pattern-top.svg), url(images/bg-pattern-bottom.svg); background-position: bottom 37vh right 52vw, top 53vh left 47vw; background-repeat: no-repeat;
Hope it helps :)
Marked as helpful - @AniketSaini98@azhar1038
Hi Aniket,
Your QR Code component is not visible. It seems you have pushed your code to
master
branch but the default branch being served ismain
. You will need to change the settings or push tomain
branch instead.Marked as helpful