Ken• 935
@kenreibman
Submitted
Going to keep updating this calculator, there are a few bugs. Any feedback would be greatly appreciated.
P
Ken
@kens-visuals
All comments
Hey @kenreibman 👋🏻
I looked at your code and I can see what you're missing. You're missing validation, if you take a look at my solution you'll see that you can't input letters, a bunch of zeros, symbols, and invalid value in general. In your case, you're only checking if the input is filled then calculate the number, but you need to consider the edge cases. Other than that, everything looks and works perfectly. Nice job, and sorry for the late response.
Marked as helpful
1- Ken• 935
@kenreibman
Submitted
Wow, this took so long to make. After countless hours of studying JavaScript, watching crash courses and tutorials. I was finally able to create a simple pop-up on a page.
I'm very confident in HTML and CSS now, especially Flexbox. I hope I can have challenges that push me to utilize more CSS Grid. I also appreciate the challenge and countless hours of studying I went through to learn a little JavaScript.
I am still in the learning process of vanilla JavaScript, and I know the code I wrote for the pop-up is not exactly the cleanest. If anyone has feedback on how I can make the code cleaner, or acceptable for the industry, I would greatly appreciate any sort of feedback that you give me out of your time.
Hey @lmaoken 👋🏻
I took a look at your JS as you asked, and here are some suggestions:
- First, it would be really great if you declared variables for all the selections, so you don't have to write
document.querySelector('')each time. For example:
const btn = document.querySelector('.btn__share');of course you should use more descriptive names, this was just a demonstration.
- Next, I suggest creating classes in CSS and give all the styling that should be applied to the button or whatever you're toggling and then in JS just add and remove those classes or even better use toggle method. You can check out my **solution to see how to do it. When, you give styles with
.styleit adds inline styling, and then it's pretty much stuck in your HTML, unless you refresh the page, so it would be a lot better to implement those styles in CSS then play with the classes in JS. - Lastly, you should add `aria-expanded="false" in HTML, and then according to the state of the pop-up change to true, I also have that implemented in my solution.
I hope this was helpful 👨🏻💻 other than that, you did a superb job, I knew you got this and this is just the begging. You've got a lot to do and to practice, but you'll get there, trust me. Cheers 👾
Marked as helpful
1 - First, it would be really great if you declared variables for all the selections, so you don't have to write
- Chaman• 120
@chaman-rawat
Submitted
When I was using Firefox and Chrome "Responsive design mode" I do not get the accurate device width. It was slightly larger than the actual pixel width I set. So is there any method or workaround to get accurate screen size?
Hey @chaman-rawat 👋🏻
I've got some feedback for the project.
- For the icons, add
aria-hidden="true”, because they're for decoration. You can read more aboutaria-hiddenhere. For example:
<img src="images/icon-team-builder.svg" alt="Team Builder">- Avoid, positioning items with just
margins especially when you're trying to center them. Both Flexbox and Grid have things likealign-items,justify-content, etc. you should use them for positioning. - Also, avoid fixed units for layouts such as pixels, try to use rems and percentages for things like
width. - Lastly, to answer your question, there's no way to get accurate screen size from browser DevTools, because sometimes in DevTools things look superb for mobile viewport width, but when you open it on an actual phone some things may not look or work as excepted, but that's pretty rare nowadays.
I hope this was helpful 👨🏻💻 other than that, for the first project you did a great job, well done. Cheers 👾
Marked as helpful
2 - For the icons, add
- Abhijit Sarode• 90
@AbhijitSarode
Submitted
Primary focus of this project was to practise flex-box. Your feedback is most appreciated
Hey @AbhijitSarode 👋🏻
I've got some suggestions to help you fix the accessibility and HTML issues.
- In your markup,
<div class="card">...</div>should be<main class="card">...</main>. This will fix the accessibility issues. Don't forget to generate a new repot once you fix the issues. - The hero image should have a description in the
alttag, something like,alt="happily dancing girl" - For the music icon, add
aria-hidden="true”, because it's for decoration. You can read more aboutaria-hiddenhere. - If
srcsetattribute is not used, it should be removed, otherwise it causes some errors. So the image tag should look something like this:
<img src="/images/icon-music.svg" alt="" aria-hidden="true">- Also, I suggest adding
transition: all 0.2s;to the button and the links, this will make:hoversmoother. - Hero image should have a
display: block;, it removes the line underneath the image. If you want to know what's causing it, check out the 3rd section of this video. - I won't go into details about resetting CSS, but I'll leave this cool article here, which will make more sense than my brief explanation.
- Next, I suggest using
<h2>or<p>instead of<h4>, because headings in HTML have to increase gradually, such ash1, h2, h3….. liis only allowed inulas a list item, so you should remove it from<a>tag.
I hope this was helpful 👨🏻💻 all in all, for the second project, you did a good job. Cheers 👾
Marked as helpful
1 - In your markup,
- CleanCoderK• 160
@CleanCoderK
Submitted
Warmly welcome for any comment
Hey @green-cyber 👋🏻
Unfortunately, we cannot see the site. Perhaps, something wrong with the link or the GitHub pages. Please fix the link, so the community can give some feedback.
I hope this was helpful 👨🏻💻 Cheers 👾
Marked as helpful
0 - dex1989• 20
@dex1989
Submitted
that is my first proect, i just stat to code for the project i use html5 and css i have use visual studio for write the code. Every comment and advice is welcome to improve myself.
Hey @dex1989 👋🏻
I've got some quick tips to help you fix the accessibility and HTML issues.
- First, I suggest using
<h2>or<p>instead of<h4>, because headings in HTML have to increase gradually, such ash1, h2, h3….. - Next, this piece of code
dex1989</a>. --7>--></html>is causing some HTML issues, just because you didn't put the closing comment tag correctly, so the solution would be to either remove the code or remove the comment tags. - Hero image should have a
display: block;, it removes the line underneath the image. If you want to know what's causing it, check out the 3rd section of this video. - For the music icon, add
aria-hidden="true”, because it's for decoration. You can read more aboutaria-hiddenhere. Like this:
<img src="./images/icon-music.svg" alt="" aria-hidden="true”>- Also, I suggest adding
transition: all 0.2s;to the button and the links, this will make:hoversmoother. - Lastly, I won't go into details about resetting CSS, but I'll leave this cool article here, which will make more sense than my brief explanation.
I hope this was helpful 👨🏻💻 other than that, you did a good job for the first project, nicely done. Cheers 👾
Marked as helpful
0 - First, I suggest using
@muhabibta
Submitted
Any tips and feedback are welcome.
Hey @muhabibta 👋🏻
I have some suggestions to help you fix the accessibility issues and some other things.
- In your markup,
<div class="container">...</div>should be<main class="container">...</main>and this will fix the accessibility issues. Don't forget to generate a new repot once you fix the issues. - Next, I suggest using
<h2>or<p>instead of<h4>, because headings in HTML have to increase gradually, such ash1, h2, h3….. - Lastly, let's bring the card to the center of the page, and here's how to do it:
body { font-size: 16px; font-family: "Karla", sans-serif; background-color: var(--Light-gray); min-height: 100vh; display: flex; align-items: center; justify-content: center; }and once you do this, you can remove
margin: 8% auto;from.container.I hope this was helpful 👨🏻💻 overall, you did a nice job, keep it up. Cheers 👾
0- In your markup,
- Ctrl+FJ• 750
@FlorianJourde
Submitted
So, my main question would be : how to use container by the same way bootstrap do ? I mean, I definided some
widthto my page to be as close as the design at possible, but I guess it would be better if my colors continue on right and left, instead of this black background. How can I define a nice wrapper in native HTML/CSS ?Also, I wanted to animate my toggle burger-menu, but I struggled because of
display: block... any suggestions ?However, pretty interesting and real-sized exercice ! Liked it, altrough it was a bit long !
Hey @FlorianJourde 👋🏻
I have some suggestions for the project.
- I recommend using
ulandlifor the bothnavs at the top and bottom, also you could put the social media icons in a list as well. Although they don't look like so, but they are lists and it would be semantically more correct to write them like so. - Next, -for the logo, add
aria-hidden="true”, because it's for decoration. You can read more aboutaria-hiddenhere. alttags for the user's images should be their names, like this:
<img src="images/image-emily.jpg" alt="Emily R.">- Lastly, to answer your question about
width, avoid using fixedwidths that's what causing problem here and add those black lines on the both sides on every viewport width. For such big things like layouts, images, etc. the best choice would relative units, such as percentages.
I hope this was helpful 👨🏻💻 other than that, you did a pixel perfect job, well done. Cheers 👾
Marked as helpful
1 - I recommend using
- Ryan Flores• 60
@RyanFloresTT
Submitted
Any feedback is greatly appreciated! I only worried about the Desktop Version, going to learn more to see what I can do about the mobile version.
Hey @RyanFloresTT 👋🏻
I've got some suggestions to help you fix the accessibility issues and some other things.
- In your markup,
<div class="row">...</div>should be<main class="row">...</main>and<div class="attribution">...</div>should be<footer class="attribution">...</footer>. These will fix the accessibility issues. Don't forget to generate a new repot once you fix the issues. - For the car icons, add
aria-hidden="true”, because they for decoration. You can read more aboutaria-hiddenhere. Also, I suggest putting them in<img>tags, so all together will look like this:
<img src="./images/icon-luxury.svg" alt="" aria-hidden="true”>- Also, I suggest implementing
:hoverstate, which you can find in design folderactive-stateimage, after you implement it you can also addtransition: all 0.2s;to the button and the links, this will make:hoversmoother. -Lastly, I won't go into details about resetting CSS, but I'll leave this cool article here, which will make more sense than my brief explanation.
I hope this was helpful 👨🏻💻 If you want to get better at responsive websites, learn mobile first workflow, there are a thousand of articles about it. All in all you did a nice job, for the second job. Cheers 👾
Marked as helpful
2 - In your markup,
- Orincy Whyte• 20
@oswhytecodes
Submitted
Hi all This is my first challenge, and I think I did pretty decent. all that aside, I can't figure out how to perfectly fit the main image inside the div. You can even see I haven't been able to show the top part of the container either. If anyone can help, I am ready for the feedback. Thank you.
Hey @oswhyteknits 👋🏻
I have some suggestions to help you fix the accessibility issues and some other things.
- In your markup,
<div class="container">...</div>should be<main class="container">...</main>and<div class="attribution">...</div>should be<footer class="attribution">...</footer>. These will fix the accessibility issues. Don't forget to generate a new repot once you fix the issues. - Hero image should have a
display: block;, it removes the line underneath the image. If you want to know what's causing it, check out the 3rd section of this video. - Next, I suggest using
<h2>or<p>instead of<h5>, because headings in HTML have to increase gradually, such ash1, h2, h3…. - For the music icon, add
aria-hidden="true”, because it's for decoration. You can read more aboutaria-hiddenhere. To illustrate:
<img src="images/icon-music.svg" alt="" aria-hidden="true”>- In order to fix the
border-radiusjust addoverflow: hidden;to.container. - Also, I suggest implementing
:hoverstate, which you can find in design folderactive-stateimage, after you implement it you can also addtransition: all 0.2s;to the button and the links, this will make:hoversmoother. - Lastly, let's bring the card to the center of the screen, so I made a couple of changes in
body:
body { background-image: url(images/pattern-background-desktop.svg); font-family: Helvetica, sans-serif; font-size: 16px; text-align: center; margin: 0; width: 100%; background-repeat: no-repeat; background-size: contain; min-height: 100vh; display: flex; flex-direction: column; align-items: center; justify-content: center; background-color: #e0e8ff; }I hope this was helpful 👨🏻💻 other than that, you did a great job for the first project, keep it up. Cheers 👾
1 - In your markup,
@carlosedugoc
Submitted
Hi!!, I'm just trying to improve my css skills. I'd like to know if I did it correctly or if there was a better way to do it
Thanks
Hey @carlosedugoc 👋🏻
I have some suggestions to help you fix the accessibility issues and some other things.
- In your markup,
<div class="container">...</div>should be<main class="container">...</main>and this will fix the accessibility issues. Don't forget to generate a new repot once you fix the issues. - Next, make sure to add
alttext, in this case it should look like this:
<img src="./images/image-jeremy.png" alt="Jeremy Robson">- I won't go into details about resetting CSS, but I'll leave this cool article here, which will make more sense than my brief explanation.
- You've named the classes good enough, but if you're trying to boost productivity and improve CSS, I'd suggest learning BEM convention.
- Also, I suggest implementing
:hoverstate on ellipsis, which you can find in design folderactive-stateimage, after you implement it you can also addtransition: all 0.2s;to the boxes and ellipsis, this will make:hoversmoother.
I hope this was helpful 👨🏻💻 other than that, you did a great job, keep it up. Cheers 👾
Marked as helpful
0- In your markup,
- Meri• 10
@merithekid
Submitted
I've only just started doing projects. How can i clean up my code and any practices you would recommend?
Hey @merithekid 👋🏻
Unfortunately, we cannot see the project, perhaps due to a broken link. Please fix the issue, so the community can give some feedback.
I hope this was helpful 👨🏻💻 Cheers 👾
0 - Richard• 50
@richardtr123
Submitted
Guys, some advice for the responsive of a grid, the size especially
Hey @richardtr123 👋🏻
I have some suggestions to help you fix the accessibility issues and some other things.
- In your markup,
<div class="card">...</div>should be<main class="card">...</main>. This will fix the accessibility issues. Don't forget to generate a new repot once you fix the issues. - I won't go into details about resetting CSS, but I'll leave this cool article here, which will make more sense than my brief explanation.
- Next, the footer should be at the bottom of the page.
- The text in the bottom right block should be in
ul libecause it's a list.
I hope this was helpful 👨🏻💻 other than that, you did a nice job, especially with responsiveness, nicely done. Cheers 👾
Marked as helpful
0 - In your markup,
- Shawnathan McCourt• 170
@smccourtb
Submitted
Open to any and all comments/critiques!
Hey @smccourtb 👋🏻
I have some suggestions to help you fix the accessibility and HTML issues.
- In your markup,
<section class="container">...</section>should be<main class="container">...</main>. - For the car icons, add
aria-hidden="true”, because they're for decoration. You can read more aboutaria-hiddenhere. These will fix the accessibility and HTML issues. Don't forget to generate a new repot once you fix the issues. To illustrate:
<img src="./images/icon-luxury.svg" alt="" aria-hidden="true”>- Also, I suggest adding
transition: all 0.2s;to the button and the links, this will make:hoversmoother. And in:hoveraddcursor: pointer;for the finger cursor. - I won't go into details about resetting CSS, but I'll leave this cool article here, which will make more sense than my brief explanation.
- Lastly, let's bring the card to the center of the screen with flexbox, because using
position: absoluteto position such big stuff can be tricky and may have some problems on mobile viewport width, and here's how to do it:
body { display: flex; align-items: center; font-size: 15px; font-family: "Lexend Deca", sans-serif; padding: 0px; margin: 0px; min-height: 100vh; justify-content: center; } .container { display: flex; width: 38.5rem; color: hsla(0, 0%, 100%, 0.75); }I hope this was helpful 👨🏻💻 other than that, you did a good job, keep it up. Cheers 👾
Marked as helpful
0 - In your markup,
- Dummy_ken• 180
@DummyKen
Submitted
This is my first every challenge taken here so this is not perfect but, I believe this is pretty good. And also in my live website, I happened to use the slate theme from Github pages so the buttons are black and stuff. Nevertheless, I am really glad I took this challenge.
Hey @DummyKen 👋🏻 Ken's here 😅
I have some suggestions to help you fix the accessibility and HTML issues.
- In your markup,
<section class="card">...</section>should be<main class="">...</main>and<div class="attribution">...</div>should be<footer class="attribution">...</footer>. These will fix the accessibility and HTML issues. Don't forget to generate a new repot once you fix the issues. - For the music icon, add
aria-hidden="true”, because it's for decoration. You can read more aboutaria-hiddenhere. To illustrate:
<img src="./images/icon-sedans.svg" alt="" aria-hidden="true”>- Also, I suggest adding
transition: all 0.2s;to the button and the links, this will make:hoversmoother. - I've made a couple of changes in
.card, to fixborder-radiusand some other things, so here it is:
.card { display: flex; flex-direction: row; width: 700px; border-radius: 10px; overflow: hidden; }as you can see, I omitted
heightbecause fixedheightprevents your website from being fully responsive. Also, I omittedmarginbecause I have a different suggestion to help you fix the card in the center.- Now let's bring the card to the center of the scree:
body { background: hsl(0, 0%, 95%); min-height: 100vh; display: flex; align-items: center; justify-content: center; flex-direction: column; }I just used flexbox, which will make sure that on any viewport width, the card is positioned correctly.
- Lastly, I won't go into details about resetting CSS, but I'll leave this cool article here, which will make more sense than my brief explanation.
I hope this was helpful 👨🏻💻 Other than that, you did a great job for the first project, well done. Cheers 👾
Marked as helpful
2 - In your markup,
- dkvel• 50
@Theguydev
Submitted
Any feedbacks or suggestions would be great thanks!
Hey @Theguydev 👋🏻
I have some suggestions to help you fix the accessibility issues and some other things.
- In your markup,
<div class="container">...</div>should be<main class="container">...</main>and<div class="header-container">...</div>should be<header class="header-container">...</header>. These will fix the accessibility issues. Don't forget to generate a new repot once you fix the issues. - For the icons, add
aria-hidden="true”, because they are for decoration. You can read more aboutaria-hiddenhere. For example:
<img src="assets/icon-supervisor.svg" alt="" aria-hidden="true”>- Next,
<p class="main">...</p>should havetext-align: center;, especially on mobile viewport width. - I won't go into many details about resetting CSS, but I'll leave this cool article here, which will make more sense than my brief explanation.
I hope this was helpful 👨🏻💻 all in all, you did a good job with the project, keep it up. Cheers 👾
Marked as helpful
1 - In your markup,
- ajensley• 10
@amber-jenae-ensley
Submitted
This is my first submitted solution! All feedback is greatly welcome. Does anyone have a good tip or strategy for knowing what to use for your sizing units and when? With the Figma file I was able to know exactly what some heights and widths should be, but for responsiveness, should everything be relative only (like rem, em, percentages, and vw/vh)? Or are using pixels, especially for font sizes, usually okay? Thanks!
Hey @amberensley 👋🏻
I have some feedback for the project.
- For the music icon, add
aria-hidden="true”, because it's for decoration. You can read more aboutaria-hiddenhere. - Hero image should have a
display: block;, it removes the line underneath the image. If you want to know what's causing it, check out the 3rd section of this video - Also, I suggest adding
transition: all 0.2s;to the button and the links, this will make:hoversmoother. - I won't go into many details about resetting CSS, but I'll leave this cool article here, which will make more sense than my brief explanation.
- Lastly, let's bring the card to the center of the screen, just add
min-height: 100vhto the body, and you're good to go.
I hope this was helpful 👨🏻💻 to answer to your question, I'd say in my opinion it's good to use relative units for the most of the stuff, because fixed units are not good if you're trying to build a responsive website or even a small component. Other than that, you did a great job with your first project, well done. Cheers 👾
Marked as helpful
1 - For the music icon, add
- Q• 60
@QZheng16
Submitted
Learn decent bit from this challenge. Just have a question about what's the best way to program an responsive website. Is it better to handle the element layout's responsive end using media queries or should I try to make sure that it's mostly handle by for example flexbox. A example will be I used media query to change the flex-direction from row to column on certain elements when it reach an min-width. I also thought that I could of done it using flex-wrap, which will handle the layout when it changes from mobile to desktop. I guess my question is does this matter for best practices sake or is it as long as it works kind of situation?
Hey @QZheng16 👋🏻
I have some suggestions to help you fix the HTML issues and some other things.
- First, instead of
imgit should bepicturein this case, since you're using responsive images. Also, addaria-hidden="true”, because it's for decoration. You can read more aboutaria-hiddenhere. So all of this should look like this:
<picture> <source srcset="images/image-header-mobile.jpg 400w" media="(max-width: 400px)"> <img src="images/image-header-desktop.jpg" alt="" aria-hidden="true”/> </picture>- I won't go into many details about resetting CSS, but I'll leave this cool article here, which will make more sense than my brief explanation.
- To answer your question, in modern day and age CSS allows us to make some responsive layouts without using media queries a lot of the time, but it's not always true. So even though flexbox and grid are here to help us, yet they cannot replace media queries. Because media queries are not only used for layout stuff, but also font-size margin, padding, etc.
I hope this was helpful 👨🏻💻 other than that, you did a nice job for your second project, keep it up. Cheers 👾
Marked as helpful
1 - First, instead of
- Jonathan Aldas• 110
@jonaaldas
Submitted
Did really fast and tried to change the color scheme lol
Hey @jonaaldas 👋🏻
Netlify and I have some bad news for you, perhaps you included a wrong link, therefore, we cannot see your website. Please fix the link, so the community can give some feedback.
I hope this was helpful 👨🏻💻 Cheers 👾
0 - Emmanuel Nanor• 40
@manuellnanor
Submitted
would appreciate reviews on this project. Thanks
Hey @manuellnanor 👋🏻
I have some suggestions to help you fix the accessibility issues and some other things.
- First, in your HTML,
<div class="container">...</div>should be<main class="container">...</main>and<div class="attribution">...</div>should be<footer class="attribution">...</footer>. These will fix the accessibility issues. Don't forget to generate a new repot once you fix the issues. - For the music icon, add
aria-hidden="true”, because it's for decoration. You can read more aboutaria-hiddenhere. For example:
<img src="./images/icon-luxury.svg" alt="" aria-hidden="true”>- Also, I suggest adding
transition: all 0.2s;to the button and the links, this will make:hoversmoother. - I won't go into many details about resetting CSS, but I'll leave this cool article here, which will make more sense than my brief explanation.
I hope this was helpful 👨🏻💻 Other than that, you did a great job, nicely done. Cheers 👾
Marked as helpful
0 - First, in your HTML,
- Saaqlainn• 55
@Saaqlainn
Submitted
Any feedback or suggestion would be appreciated.
Hey @Saaqlainn 👋🏻
I've got some suggestions for the project.
- First things first, since you used two
@media queriesto put min and max width, when I open the website I see it in mobile viewport width. I have to change my screen size to see the desktop layout. What I suggest is to choose either build it with desktop first layoutmax-widthor mobile firstmin-width. - For the car icons, add
aria-hidden="true”, because they are for decoration. You can read more aboutaria-hiddenhere. For example:
<img src="images/icon-luxury.svg" alt="" aria-hidden="true">- Also, I suggest adding
transition: all 0.2s;to the button and the links, this will make:hoversmoother.
I hope this was helpful 👨🏻💻 Other than that, you did a great job, well done. Cheers 👾
Marked as helpful
2 - First things first, since you used two
- Yazdun• 1,310
@Yazdun
Submitted
Hello my fellow developers ! Here is my solution for this challenge with a extra feature :
- I used
gridto position cards. - I added dark and light mode as a extra feature.
And I have some questions :
-
It's first time I'm adding dark-mode to my project and I was very unfamiliar and had to do a lot of research, What are best practices to create dark mode using sass ? Is my approach any good ?
-
This should've loaded initial theme based on user's OS theme, But it seems like It's not working after deployment, does it work for you?
-
Can you switch themes or is my solution broken ? 😐
✅ Let me know your thoughts and feedbacks on this
Hey @Yazdun 👋🏻
I've got some feedback for the project.
- For the icons, add
aria-hidden="true”, because they are for decoration. You can read more aboutaria-hiddenhere. For example:
<img class="card__icon" src="./images/icon-team-builder.svg" alt="" aria-hidden="true”>- Next,
<div class="header"></div>should be<header class="header"></header>
Now what comes to the questions:
- I'm not really experienced with dark mode and how to implement it, so I'll let other mentors to give some suggestions on that.
- When I first opened the website, I was going to tell you to add a feature to load the site based on user's preferences, but then I saw the question. Unfortunately, that option is not working, it just opened in white mode, whereas my theme of preference is dark.
- Overall, you added a cool feature, and it works as expected.
I hope this was helpful 👨🏻💻 Cheers 👾
Marked as helpful
0 - I used
- LeoAcosta• 40
@LeandroA98
Submitted
Feedback will be nice.
Hey @LeandroA98 👋🏻
I have some suggestions to help you fix the accessibility and HTML issues.
- For the image, add
aria-hidden="true”, because it's for decoration. You can read more aboutaria-hiddenhere. Like this:
<img src="./images/image-header-mobile.jpg" alt="" aria-hidden="true”>- Next, instead of
type='./image/png'it should betype='image/png'. - Instead of
<section>, I suggest using regular<div>for a couple of reasons. First, when you use a<section>you have to have a heading, likeh2-h6. Next,<section>is for bigger parts of a layout, such as, contact us about us, image gallery, etc. These will fix the HTML issues, just don't forget to generate a new repost once you fix them.
I hope this was helpful 👨🏻💻 other than that, you did a great job especially with responsiveness, keep it up. Cheers 👾
Marked as helpful
0 - For the image, add
- Cody Lewis Zites• 570
@zitescody
Submitted
Please provide feedback!
What can I do to do better?
Hey @zitescody 👋🏻
I have some suggestions to help you fix the accessibility and HTML issues.
- In your markup,
<div class="container">...</div>should be<main class="container">...</main>and<div class="attribution">...</div>should be<footer class="attribution">...</footer>. These will fix the accessibility issues. Don't forget to generate a new repot once you fix the issues. - Now, there are a couple of attributes that are causing some issues in HTML, I'll leave the list below:
<=`` head=`` role=`img` role=`button`I'm not sure what the first one is, but here's a **link to learn more about
role=""- For the car icons, add
aria-hidden="true”, because they are for decoration. You can read more aboutaria-hiddenhere. To illustrate:
<img src="images/icon-luxury.svg" alt="" aria-hidden="true”>- Next, add this line
border: 1px solid transparent;to yourbuttons, so initially the buttons will have some border and only during:hoverthey will change their color. Once you add it you'll see the difference. - I won't go into many details about resetting CSS, but I'll leave this cool article here, which will make more sense than my brief explanation.
- Lastly, let's bring the card to the center of the screen, and here's how to do it:
body { background-color: var(--verylightgray); font-size: 15px; min-height: 100vh; display: flex; align-items: center; justify-content: center; flex-direction: column; }I hope this was helpful 👨🏻💻 other than that, you did a good job for your second project, nicely done. Cheers 👾
Marked as helpful
1 - In your markup,
