
Aditya Singh
@adityas24All comments
- @fiqihalfito@adityas24
Hi Fiqih
I couldn't get what exactly is your problem? Can you tell in detail?
- @Sanjog735@adityas24
Hi Sanjog, i loved your site, great job!
For creating those lines, you can simply give
border-bottom
to the first and the second boxes you have created. That is the best way to do it i think.If you still want to follow any other path, you can use
div
tag and give itwidth
,height
andbackground color
. But why to create tags unnecessarily if you can do it just by adding border-bottom. This would be a bad practice.Do mark it as helpful if it helped you. Happy coding!
- @saswat10@adityas24
Hi Saswat, great effort by you!
I saw that for displaying the person's image, you are using background image. But the image tag is the most appropriate way to display image here. For more information read this stackoverflow answer on background image vs img tag
Hope it was helpful, Happy coding!
Marked as helpful - @dhan5a@adityas24
Hi Dhanya, Great work by you. I found following irregularities in your HTML
1. Instead of writing this
<div class="header"> Improve your front-end skills by building projects </div>
You could have written<h2> Improve your front-end skills by building projects </h2>
It is semantically better code. And not necessary to use
h2
tag. You can use any heading tag according to requirement2. And you should have used paragraph tag here
<div class="text-copy"> Scan the QR code to visit Frontend Mentor and take your coding skills to the next level </div
Instead of above code, you should write
<p> Scan the QR code to visit Frontend Mentor and take your coding skills to the next level </p>
Here right tag to be used is paragraph tag.
Thanks, Happy coding.
Marked as helpful - @spaprocki@adityas24
Hey Szymon, great job , a very well semantic html written by you. I think writing good semantic HTML is harder to master than CSS.
And I would like to suggest you that avoid applying any styles on the body element. Make habit of not doing it from the very beginning. All the best!
Marked as helpful - @Teo-kodehode@adityas24
Hey Teo! you did very well in your first challenge. I would like to suggest you the following....
You could have avoided the
div
you used to wrap theh1
andp
. As you have not used any style on div. Then you can simply avoid using it. It will not affect the semantics and will also decrease the length of the code.So you could simply write
<div class="container"> <img src="images\image-qr-code.png" alt="QR code"> <h1>Improve your front-end skills by building projects</h1> <p class="text">Scan the QR code to visit Frontend Mentor and take your coding skills to the next level</p> </div>
All the best 🙂👍
Marked as helpful - @abdurakhmon-web@adityas24
Hey Abdur great Job!
Very well written markup and very well use of BEM. Now learn about em and rem and start using them. You are ready to rock.
All the best 🙂👍
Marked as helpful - @ayushbhargava22@adityas24
Hey Ayush good job.
I saw that you are using margins to give spaces around the elements inside the card. For example in
<div class="nft-img"></div>
and<div class="owner"></div>
.Do not use margins for this purpose. Use padding instead. Margins are used when we want to give space between two adjacent elements. But to give space around the element always use padding.
All the best 🙂👍
Marked as helpful - @Nikkakko@adityas24
Hey Nika! good job with this one.
On small screens (below 600px) your card has no space around it. You card needs some left and right
margin
around it. We do not set that margin on container as margin on container is set toauto
to center the card. So best way of doing this is that always wrap the container inside some element and give margin to that element, that can be section or div according to your requirement.Here is the code
<div class = "card> <div class = "container"> . . . </div> </div> .card { margin: 0 2rem; }
Hope it helps. All the best 🙂👍
Marked as helpful - @sandeshad100@adityas24
Hey Sandesh! great job with this one.
One mistake i saw is that you are using unnecessary divs in your code. Inside each column you have 4 elements. And for each element you are using a div to contain it. You don't really need to do that.
Instead of writing
<div class="col1 box"> <div class="col1Img"> <img src="images/icon-sedans.svg" alt="sedansImg"> </div> <div class="col1Title"><h2>Sedans</h2></div> <div class="col1Info"> <p>Choose a sedan for its affordability and excellent fuel economy. Ideal for cruising in the city or on your next road trip.</p> </div> <div class="col1bottom"> <button>Learn More</button> </div> </div>
You could have simply written
<div class="col1 box"> <img src="images/icon-sedans.svg" alt="sedansImg"> <h2>Sedans</h2> <p>Choose a sedan for its affordability and excellent fuel economy. Ideal for cruising in the city or on your next road trip.</p> <button>Learn More</button> </div>
Still the code will work fine as every element inside the flexbox becomes a flex item irrespective of that item is inline or block element.
All the best 🙂👍
Marked as helpful - @daKeshra@adityas24
Hey good job!
You can use transform: rotate() property on the arrow and rotate it by 180degree when the arrow is being clicked. You can do this both with css and javascript.
All the best 🙂👍
- @Sathyan-B@adityas24
Hey Sathyan, good job with this one.
Your images are not visible as you have not written the path correctly. Remove the first slash in the images url. Your site would be working fine while working with VS code. But when you host your website, this is not the correct way of giving path.
Instead of writing <img src="/images/icon-suvs.svg">. You have to write <img src="images/icon-suvs.svg">.
All the best 👍🙂
Marked as helpful - @ToniHunter274@adityas24
Hey Oluwatoni, upload your code again.
Your folder structure is not proper. Index.html file should be in the root folder.
All the best 🙂👍
Marked as helpful - @SiR-PENt@adityas24
Good job with grids buddy!
Biggest problem with your web page is accessiblity. Color of your text and headings is very light. Make them bit darker so that they are accessible to everyone. They might be inaccessible to person with weak eyesight. Many other factors come under accessiblity. Google about accessiblity in web dev and you will get more insight into this.
And here i am sharing a great tool to know that color contrast is good or bad and how readable/accessible is your text: https://coolors.co/contrast-checker/112a46-acc8e5
All the best 🙂👍
Marked as helpful - @khanisrar@adityas24
Hey Israr Good Job ! And it's not a landing page buddy.
- @tjohnsey@adityas24
Hey Johnsey great effort!
There is not any specific size of the card. All you have to do is look at the design and get your card looking as close to the design as possible.
Hope it helps. All the best 🙂👍
- @momenkamal221@adityas24
Great Job Kamal!
One mistake i saw in your code is that you are using "ids" everywhere instead of "classes". I know the CSS will still work fine and the web page would look the same. But in real world it is a very bad practice and mostly classes are used for styling purpose. Biggest advantage of classes is reusability of code.
Ids have some very specific use cases. They are very much used when JavaScript comes into action. Google it and you will get more clarity.
So get into habit of writing classes from very beginning itself. All the best 🙂👍
Marked as helpful - @ashleywaudbydev@adityas24
Hey Ashley you did a great job!🙂
I studied your code well and found some points you need to improve on: 🙂
- You are using div element where it is not necessary. Using div there will not create any problem, but you are unnecessarily increasing the length of the code. That will reduce the maintainablity and readability of the code.
For example: <div class="header"> <h1>Improve your front-end skills by building projects</h1> </div>.
Here you do not need to wrap h1 in a div. If still you want to write h1 inside of a header, then use <header> tag instead. As we have to take care of semantics also 🙂
- Similary you don't need to wrap paragraph inside of a div. You can straight away use <p> tag without wrapping it. Afterall everything inside a flexbox will become flex item. You don't need to wrap everything inside a div 🙂
I will recommend you to go through markup written by Daniel. He has written it perfectly i think
https://www.frontendmentor.io/solutions/qr-code-component-using-html-and-css-38XEjHXG2
All the best 🙂👍