Design comparison
Solution retrospective
All suggestions are welcome!
Community feedback
- @lack21Posted over 1 year ago
Good job π, but I have a suggestion!
- Problem with your code is that advice is not changing, I noticed you have this in your Javascript
const url = 'https://api.adviceslip.com/advice' function getAdvice(){ fetch(url) .then(function(res){ res.json() .then(function(data){ console.log(data) id.innerHTML = `Advice #${data.slip.id}` text.innerHTML = `"${data.slip.advice}"` }) }) .catch(function(err){ console.error('Error') }) }
Replace it with this
function getAdvice(){ const randomNumber = Math.floor(Math.random() * 2); const url = `https://api.adviceslip.com/advice?random${randomNumber}` fetch(url) .then(function(res){ res.json() .then(function(data){ console.log(data) id.innerHTML = `Advice #${data.slip.id}` text.innerHTML = `"${data.slip.advice}"` }) }) .catch(function(err){ console.error('Error') }) }
This should make it work!
Marked as helpful1@cassialityPosted over 1 year ago@lack21 Hello! Thanks for the suggestion, I have no idea about that possibility, I'll practice more to learn math properties. Thanks again, come more suggestions!!
1@lack21Posted over 1 year ago@cassiality It's not about the Math, it's the source you're taking advice from
1 - @0xabdulkhaliqPosted over 1 year ago
Hello there π. Congratulations on successfully completing the challenge! π
- I have other recommendations regarding your code that I believe will be of great interest to you.
HEADINGS β οΈ:
- This solution lacks usage of
<h1>
so it can cause severe accessibility errors due to lack of level-one headings<h1>
- Every site must want only one
h1
element identifying and describing the main content of the page.
- An
h1
heading provides an important navigation point for users of assistive technologies, allowing them to easily find the main content of the page.
- So we want to add a level-one heading to improve accessibility by reading aloud the heading by screen readers, you can achieve this by adding a
sr-only
class to hide it from visual users (it will be useful for visually impaired users)
- Example:
<h1 class="sr-only">Advice generator app</h1>
- If you have any questions or need further clarification, you can always check out
my submission
for another challenge where i used this technique and feel free to reach out to me.
.
I hope you find this helpful π Above all, the solution you submitted is great !
Happy coding!
Marked as helpful1@cassialityPosted over 1 year ago@0xAbdulKhalid Hello! Thanks for your suggestions, I wasn't aware of accessibility, my fault. Come more suggestions! :D
0
Please log in to post a comment
Log in with GitHubJoin our Discord community
Join thousands of Frontend Mentor community members taking the challenges, sharing resources, helping each other, and chatting about all things front-end!
Join our Discord