Ritik Kumar
@itswebcoderAll comments
- @satyammodi1212Submitted almost 2 years ago@itswebcoderPosted almost 2 years ago
Hi there 👋. Good job on completing the challenge ! I have some feedback for you if you want to improve your code.
HTML:
Use the <main> tag to wrap all the main content of the page instead of the <div> tag. With this semantic element you can improve the accessibility of your page. Use the <footer> tag to wrap the footer of the page instead of the <div class="attribution">. The <footer> element contains information about the author of the page, the copyright, and other legal information.
Instead of using pixels in font-size, use relative units like em or rem. The font-size in absolute units like pixels does not scale with the user's browser settings. This can cause accessibility issues for users who have set their browser to use a larger font size. You can read more about this here. To center the component in the page, you should use Flexbox or Grid layout. You can read more about centering in CSS here. You can read more about centering in CSS here. body { min-height: 100vh; display: grid; place-content: center; background-color: hsl(212, 45%, 89%); } I hope you find it useful! 😄 Above all, the solution you submitted is great!
Marked as helpful0 - @satyammodi1212Submitted almost 2 years ago@itswebcoderPosted almost 2 years ago
Your solution is right as well as. Your biggest silly mistake was you don't have use order list and unorder list.
<ol></ol> <ul></ul>. Yup, your text heading is not in accurate position. I hope you find it useful! 😄 Above all, the solution you submitted is great!Marked as helpful0 - @satyammodi1212Submitted almost 2 years ago@itswebcoderPosted almost 2 years ago
This is accurate but in this code there will be issue in your height. I hope you find it useful! 😄 Above all, the solution you submitted is great! Replace <div class="main-body"> with the main tag, v <div class="heading"> with <h1>, <div class="par"> with <p>, <div class="text"> with <h2>, <div class="btn">with <button> and <div class="cancel"> with <a> add aria-hidden=" true" to the <a> to fix the accessibility. issue. click here for more on web-accessibility and semantic html .
Marked as helpful0 - @satyammodi1212Submitted almost 2 years ago@itswebcoderPosted almost 2 years ago
Your soulution look like nice but in this solution there will be some issue. You will solve this solution in your solution there will be silly mistakes. You will be use box shadow in your solution as border. Box-shadow: 0 0 20px #cdcdcd; just like that.
Marked as helpful0 - @satyammodi1212Submitted almost 2 years ago@itswebcoderPosted almost 2 years ago
HI! Your solution looks nice though there are a couple of things you can improve which I hope will be helpful! 😊
I would also add some transitions for active states (when colors change on hover). It creates more interactivity and makes the project look cooler. Active states can be done on buttons, links, titles which act like links, or anything else, you choose.
You can read more about it here, in case you haven’t done much of it: https://www.w3schools.com/css/css3_transitions.asp
The image with the alt "car-icon" should be used more as a decorative image, rather than being attached to any content. Also, you wrote alt only on two images and the third one is empty.
Alt attribute for the image is important in order to specify alternative text for the image in case it will not be displayed. Using alt attribute is good for not only accessibility but also SEO and for situations when the image is loading too slowly. If the image is just for decoration you can still write an alt attribute but leave it empty, such images don’t need any alt tag but you will need to also add aria-hidden=“true”. What aria-hidden does is that it removes the entire element from the accessibility tree.
If otherwise, you need to use an alt tag to describe the image. To write an alt tag you need to describe the content and purpose of the image and try not to use words like “picture of” or “image of”.
Finally, for the images to match width you can also do:
.icon { width: 30%; height: 80%; }
Marked as helpful0 - @satyammodi1212Submitted almost 2 years ago@itswebcoderPosted almost 2 years ago
Its look like nice. Something issue in size of your solution.
Marked as helpful0 - @AlanLopReySubmitted almost 2 years ago
I had a lot of difficulties when making the navigation icons, I couldn't make them look like in the challenge but I still did the best I could, if anyone knows how I could improve or if you have articles that have studied them I would appreciate it.
@itswebcoderPosted almost 2 years agoThat's also nice bro. I'm always trying and finally i have done it.
0 - @Arjun7478Submitted almost 2 years ago