Vulla Sai kiran kumar• 20
@SaikiranVulla
Submitted
Sarvotham Gowda
@sarvothamgowda
All comments
- Sarvotham Gowda• 210
@sarvothamgowda
Posted
Hi Sai Kiran,
Good start.
- You can center the container by using flex in the body and its properties. Learning flex and grid would help you to solve most of the challenges. Instead of using <br> make use of the margin property to create space between elements.
body { min-height: 100vh; display: flex; justify-content: center; align-items: center; }- As a good practice use:
<html lang="en"> <head> <meta chartset="UTF-8"> <meta name="viewport" content="width=device-width, initial-scale=1.0"> </head> </html>You can learn more about the attributes and tags here https://www.w3schools.com/
- The font and colors are not as per the design. So you really need to read the relevant style files provided as a part of this challenge.
Hope this helps!
Marked as helpful
1 - StephenAlcantara• 140
@StephenAlcantara
Submitted
Sarvotham Gowda• 210@sarvothamgowda
Posted
Hi Stephen,
Good job. You can add a few lines of code to improve it even better. For example:
- Since, you did not define the viewport height the card has flushed all the way to the top.
body { min-height: 100vh; }This will give enough room to center your container.
- For letter-spacing in the span inline element. Defined property is invalid.
span { letter-spacing: 15px; }- For the desktop version the image dimensions and size of the card are different. You can also make use of @media queries to build a responsive mobile card which allows the child items to stack on top of each other.
Hope this helps!
0 - Nuwan Chandrasekara• 10
@nuwanchandrasekara
Submitted
Sarvotham Gowda• 210@sarvothamgowda
Posted
Hi @Nuwan,
Good start. Even though I'm still learning and new to HTML and CSS I would like to share some feedback based on my learnings.
- For the body consider using flex and its properties to align the container to the center of the page. Consider using this:
body { min-height: 100vh; display: flex; justify-content: center; background-color: hsl(212, 45%, 89%); }-
Consider using semantics elements (for eg: <picture> ) which clearly defines its content.
-
I think the path for the image is wrong hence the QR code image is not showing. While I add images I always declare its width and height in the img markup. And use width of 100% and height auto in the CSS to make it responsive. You can also consider using Object-fit: contain; to retain its original aspect ratio and it will fit within the given dimension.
Since, you have declared img as a class="qr-code" use that in your CSS rule.
<img src="<path> alt="QR-code-image" width= " " height " ">.qr-code { width: 100%; height: auto; padding: 1rem; border-radius: 15px; }Marked as helpful
0 - Mohit Jain• 190
@MOHITBILALA
Submitted
Sarvotham Gowda• 210@sarvothamgowda
Posted
Hey Mohit,
Good Job! Your solution is almost close to the design. One thing I noticed is that you have forgotten to add border-radius to the container.
.container { border-radius: 5px; }Happy coding!
Marked as helpful
0 - Abdelrahman Khaled• 280
@Abdelrahman0Khaled
Submitted
how can i sit background imgs better? **how can i make this project with best practice ? ** I am still learning ..all feedback is welcome
Sarvotham Gowda• 210@sarvothamgowda
Posted
Hey, good job!
Especially on aligning the testimonial cards as per the design. The only thing where you can work on is the background image position. Here is my solution.
- mobile version:
body { background: url(./images/bg-pattern-bottom-mobile.svg) no-repeat fixed bottom right, url(./images/bg-pattern-top-mobile.svg) fixed no-repeat top left; }- desktop version:
body { background: url(./images/bg-pattern-bottom-desktop.svg) no-repeat fixed bottom right, url(./images/bg-pattern-top-desktop.svg) no-repeat fixed top left; }Marked as helpful
0
