@FahimEcho
Submitted
Looking to hire developers?
@DCoder18
@FahimEcho
Submitted
@DCoder18
Posted
Nice work!
I noticed that you're using React. If you don't mind, could you please share some resources where I can learn React too?
Thank you!
Marked as helpful
@petewk
Submitted
Some alignment issues, re-set button submits and clears fields instead of just clearing
@DCoder18
Posted
Hi Pete,
Good attempt. The reason why your reset button submits is because you added submit
to the type
attribute. You don't need this here unless your sending data to an external page. Instead, set it as <input type="button">
.
For the reset button, try using a separate reset function. In that function, you can set all your variables like bill
, tip
, people
etc. to 0
.
@mackieva
Submitted
The one area I'd like to see what others did, the percentage buttons and how that was value was determined when clicked. I just did an if statement checking the ID and setting it there so I'm looking forward to seeing a different way.
@DCoder18
Posted
Hi Ryan, Good job! Regarding your question about the percentage buttons, I used something called data attributes. It allows you to store custom data from within the html itself!
For example, I have the following percentage button with a data attribute I called data-tip-amount
.
<button type="button" data-tip-amount="5" >5%</button>
And then in javascript, I stored the tip amount in a variable using the getAttribute
method.
tip = Number(btn.getAttribute('data-tip-amount'))
Hope you find this helpful :)
Marked as helpful
@mjmorrison10
Submitted
I feel like a lot of the text were supposed to be smaller than 16px. Or should I have made the entire container bigger?
@DCoder18
Posted
Hi Michael,
to answer your question, yes I think increasing the width of your container would definitely help. 16px wouldn't look too big then. I would also suggest making the height of your container auto
and control the height using padding. This would help with responsiveness and you don't have to keep changing the height if you make a change to the width :)
Secondly, instead of using divs for all your elements, I suggest using the appropriate HTML markup such as <h1>
for headings, <p>
for paragraphs etc. This is very important for accessibility and SEO. This will also fix majority of the accessibility issues mentioned in your report.
Here are some ways you can fix those issues:
<div class="container">
to <main class="container">
<div class="attribution">
to <footer class="attribution">
and so on..Here is a helpful resource to understand more about semantic markup: Semantic Markup
I know this is more than you asked for but I hope it helps anyway :)
Marked as helpful
@feliveira
Submitted
@DCoder18
Posted
Hi Felipe,
Great attempt at this project! If I understand correctly, you want the image to be as wide as the project does? By tinkering a bit with google inspect, I found if you make your #card
class width: 100%
, you might achieve what you're looking for. If I misunderstood your question, do let me know.
To fix the accessibility issue mentioned in the report, you just have to move the attribution
code block inside a <footer>
tag :)
Marked as helpful
@vinaypuppal
Submitted
Any feedback is welcome!
@DCoder18
Posted
Your solution looks great Vinay! My only feedback would be to perhaps add some more padding to the top and bottom of the container :)
Marked as helpful