@moibrahem98Submitted almost 4 years ago
any feedback about my best practice
Renszo Camacho
@RenszCamachoAll comments
- @RenszCamachoPosted almost 4 years ago
Hiya ππ» moibrahem98.
Good job mate πππ. You have done a fantastic job on this challenge π, place properly the background image is quite tricky.
Just a few suggestions IMHO.
-
Add an alternative text to your images.
<img class="person" src="images/image-victor.jpg" alt="victor" /> -
I would add a CSS reset, to avoid some annoying margins.
* { margin: 0; padding: 0; box-sizing: border-box; }- To place the background-images:
body { background-image: url(../images/bg-pattern-top.svg), url(../images/bg-pattern-bottom.svg); background-position: right 50vw bottom 50vh, left 50vw top 50vh; background-repeat: no-repeat; background-color: hsl(185, 75%, 39%); display: flex; justify-content: center; align-items: center; width: 100%; height: 100vh; }- To center the card.
.card__body { width: inherit; height: inherit; display: flex; justify-content: center; align-items: center; }Happy codingπ§βπ»
1 -
- @RamosNicoSubmitted almost 4 years ago
This is the first challenge I've ever done, felt pretty good.
Both Background's .svg are placed in order to look good at widths: 1440px and 375px through media queries, but as soon as the screen gets bigger they will move all over the place, not sure how to fix it.
Also, I can't manage to truly align the card's footer with the main body, the statistics are a bit more to the right compared with the name, although I centered all of them. I could center them manually with some right-padding, but does anyone know how to fix it without going that way?
@RenszCamachoPosted almost 4 years agoHello, ππ» RamosNico.
Well done my man, in your first challenge πππ. Lovely job on this challenge, place properly the background image is quite tricky or at least it was for me.
I've been digging into your code. And this is how I would do it.
- I would delete the images from the Html. and places it in the Css.
body { background-color: #19a2ae; background-repeat: no-repeat; font-family: "Kumbh Sans", sans-serif; background-image: url(../images/bg-pattern-top.svg), url(../images/bg-pattern-bottom.svg); background-repeat: no-repeat, no-repeat; background-position: right 50vw bottom 50vh, left 50vw top 50vh; }Regarding the alignment of the card's footer with the main body, I see it well. They have a display: flex. justify-content: space-evenly.
I hope, it helps. Happy codingπ§βπ»
2 - @edshuliSubmitted almost 4 years ago
Hello everyone, can you please give me an advice about the Html part, especially about the card bottom. I did it with the <table> element, would you do it another way ?
Thank you for your time !!
Have fun :)
@RenszCamachoPosted almost 4 years agoHello, ππ» edshuli. Well done my friend πππ. Lovely job on this challenge, place properly the background image is quite tricky.
Just a suggestion in my humble newbie opinion. π
I've been checking your code, I would approach that, like so.
- I think you can avoid those media queries. Having a wrapper, wrap card, and card-bottom, and in your CSS, I would do this to center the card.
.wrapper { display: flex; flex-direction: column; justify-content: center; align-items: center; height: 100vh; }-
It would still not be fully centered, so you have to remove the margins from the card.
-
To position the background-images properly and make them responsive, I would do this on your CSS body.
background-position: right 50vw bottom 50vh, left 50vw top 50vh;- Add an alternative text to your images.
<img src="/images/image-victor.jpg" alt="victor crest">- The id global attribute defines an identifier (ID) which must be unique in the whole document. Its purpose is to identify the element when linking (using a fragment identifier), scripting, or styling (with CSS). And you have more than one with the same name.
I hope, it helps.
Happy codingπ§βπ»
1 - @varisDogukanSubmitted almost 4 years ago
I don't have any experience with naming classes. Can you help me for this?
@RenszCamachoPosted almost 4 years agoHi, do-Va.
There are only two hard problems in Computer Science: cache invalidation and naming things β Phil Karlton
I recommend you, learn BEM too, it will make your life easier. You can check this video. You Probably Need BEM CSS in Your Life (Tutorial) or check the BEM website about naming Naming
I hope, it helps.
Happy codingπ§βπ»
2 - @rizfirsy-ghSubmitted almost 4 years ago
If you found any problems around it please tell me, I'll fix that ASAP.
@RenszCamachoPosted almost 4 years agoHiya ππ» RizkyFirman.
Good job mate πππ. You have done a fantastic job on this challenge π, I like the animation cards and itβs very responsive π―.
I have been digging into your code, you have used css-grid. Cool.
You have a horizontal scroll on a small and medium screen. I would put on the body an
overflow: hiddento fix that.You got many
<div>. Just a suggestion in my humble newbie opinion. π-
You could have a
<main>tag involving your code. The<main>element represents the dominant content of the<body>of a document. -
Your cards could be
<article>instead of just<div>
I hope, it helps.
Happy codingπ§βπ»
2 -
- @h-harshSubmitted almost 4 years ago
i was not able to set up the background 2 circles in a responsive way, they break when screen size moves
@RenszCamachoPosted almost 4 years agoHi, harsh.
I've been digging into your code. And this is how I would do it.
- I would give your card width, height, and a box-shadow.
.card-box { width: 353px; height: 376px; background-color: white; border-radius: 1rem; overflow: hidden; box-shadow: -30px 50px 50px #00000030; }-
I would delete the images from the Html. and places it in the Css.
-
Now place the card in the center, so in the main it would do something like this.
.main { width: 100%; height: 100vh; display: flex; justify-content: center; align-items: center; }- Finally to the body I would place the background-images.
body { background-image: url(./images/bg-pattern-top.svg), url(./images/bg-pattern-bottom.svg); background-repeat: no-repeat, no-repeat; background-position: right 52vw bottom 38vh, left 49vw top 51vh; }Hopefully, it helps.
Happy codingπ§βπ»
1 - @mohapatraanuragSubmitted almost 4 years ago
Could setup the circles for 1440px and 375px but when checked other resolution the bottom circle moves around. Would appreciate if anyone can give feedback on this.
Working with sass for the first time and with HTML and css after a while.
@RenszCamachoPosted almost 4 years agoHello, ππ» mohapatraanurag. Well done my friend πππ. Lovely job on this challenge, place properly the background image is quite tricky.
Just a suggestion in my humble newbie opinion. π
-I've been checking your code. And I would approach that, like so.
-Try to remove all unnecessary comments.
-Remove the
bg-patternfrom the Html, this way you avoid those Html issues. And in your sass file_global.scssI would place the bg images.body { background-image: url(../images/bg-pattern-top.svg), url(../images/bg-pattern-top.svg); background-repeat: no-repeat, no-repeat; background-position: right 52vw bottom 38vh, left 49vw top 51vh; }I know, there are many ways to place background properly, But that's how I solved it.
I hope, it helps.
Happy codingπ§βπ»
1 - @Ivet89Submitted almost 4 years ago
Hola :)
@RenszCamachoPosted almost 4 years agoHola, Ivet89. You have done a fantastic job on this challenge π, and itβs quite responsive π―.
You have used bootstrap 4. why not bootstrap 5.
I have been checking your code, and you got many
<div>. Just a suggestion in my humble newbie opinion. π-
You could have a
<main>tag involving your code. The<main>element represents the dominant content of the<body>of a document. -
Your cards could be
<article>instead of just<div>
Happy codingπ§βπ»
1 -
- @gam98Submitted almost 4 years ago@RenszCamachoPosted almost 4 years ago
Good job mate πππ. You have done a fantastic job on this challenge π
Just a suggestion in my humble newbie opinion. π
It would be great some margin-bottom on the mobile version.
Happy codingπ§βπ»
1 - @domihustinovaSubmitted almost 4 years ago
Hi everyone ππ» This is my first Frontend Mentor challenge. Until now, I was mostly focused on JS and neglecting my CSS skills so I've decided to work on those a bit, too. I had also zero experience with SASS before I started this task so I guess that was the biggest challenge for me :) I would appreciate any feedback!
@RenszCamachoPosted almost 4 years agoHi ππ» domihustinova. Well done πππ. You have done a fantastic job on this challenge π, I noticed that you did desktop-first, and itβs quite responsive π―.
Happy codingπ§βπ»
1 - @yossefAlatterSubmitted almost 4 years ago
- Hello everyone
- that is a good challenge with nice functionality i also do the animation bonus
- i create my own methods to do that function and use it to make the timer and the animation take a look to the code time CompleteTimerPieces Components
- also i make it responsive as much as i can
- i will be very happy by your feedback
@RenszCamachoPosted almost 4 years agoHiya ππ» yossefAlatter. Well done my friend πππ. You have done a fantastic job on this challenge π
Happy codingπ§βπ»
2 - @FilipHanzlikSubmitted almost 4 years ago
Hello! A feedback would be highly appriciated and would help me improveπ. Also i had a problem setting shadows in hover state. Basically after aplying a hover state for all cards, only the first one works correctly. If anyone knows what i did wrong, i would really appreciate if he let me know.
@RenszCamachoPosted almost 4 years agoHi FilipHanzlik.
Good job mate πππ. You have done a fantastic job on this challenge π, I like the animation background and itβs very responsive π―.
It took me a while to find the shadow's bug. π
The issue is your layout and how you placed the backgrounds, what is above your cards.
So to fix that just put a
.bottom-background{ z-index: -10 }Hopefully, it helps.
Happy codingπ§βπ»
3
