Abubkar Ahmed• 460
@abubkar-ahmed
Submitted
Rolando Parada Puig
@RolandoParadaPuig
All comments
- Rolando Parada Puig• 200
@RolandoParadaPuig
Posted
Your image its not responsive, try to set a heigth:auto on it, and for the active states or the menu slide (on mobile) try to put some transition to get a "smooth" more natural sensation
Marked as helpful
0 - Thibaut Lefevre• 120
@malboyoo
Submitted
Taking any suggestions, thanks!
Rolando Parada Puig• 200@RolandoParadaPuig
Posted
i like it but if you would like to improoveit a little bit you dont need the count down function, apiCooldown, the async/await have the "cooldown" on its own, try this:
/* here you change the btn classes */
rollBtn.classList.add("roll-btn-cd"); rollBtn.classList.remove("roll-btn-ok"); rollBtn.disabled = true;/* here you call the API */
generatedAdvice();/* then you change the btn class again */
rollBtn.classList.add("roll-btn-ok"); rollBtn.classList.remove("roll-btn-cd"); rollBtn.disabled = false;*/ the function finish */
this the "right" way of dooing it, so the app doesn't have that "strange" behaviour when you allready get the data from the API, that delay at the end feels weird. oh and on your css add some transitions so it have more... smooth feeling on the btn
Marked as helpful
0 - Jack Tubbenhauer• 170
@jtubbenhauer
Submitted
Not sure how to handle the short delay after the new quote has been displayed. There should probably be some feedback letting the user know they can click the button again.
Rolando Parada Puig• 200@RolandoParadaPuig
Posted
if you whant to handle the delay between calls you could add some kind of loading icon For example: on the begining of the call you could display=none for the text and display=block for the icon with some animation, this way you will have some kind of "interactive" way of showing your public that something is happening using your code here =>
const handleClick = () => { (dismount the text here) (mount the icon with some animation here) fetch("https://api.adviceslip.com/advice") .then((res) => res.json()) .then((data) => { quoteText.innerHTML = data.slip.advice) (dismount the icon with some animation here) (mount the text here) }; };
this is an example and i think it will work if you tri it like this
Marked as helpful
0 - jake• 140
@jakewebd
Submitted
all tips appreciated!
I wasn't sure how to make a <pre> responsive so the mobile design isn't centered as well with the description and such.
Rolando Parada Puig• 200@RolandoParadaPuig
Posted
very good job! i like it! if you want some advice try to use the "mobile first" design pattern this is a free course that help me alot: https://courses.kevinpowell.co/view/courses/conquering-responsive-layouts/ 😉 and read the report section and try to fix some of the issues that you have, it will help you to learn alot about semantics and best practices!
on your css you can use variables to help you make fast changes on larger apps that have repitted styles on diferent elements for example you could put on variables your background colors try to learn about css virables here: https://developer.mozilla.org/en-US/docs/Web/CSS/Using_CSS_custom_properties 👀 good luck with your developer journey! 👍🙌
Marked as helpful
1 - MioMauro• 400
@MioMauro
Submitted
I thought I could put js in as well, but my beginner level did not allow me to. Anyway, I am getting better at development. any advice is always well appreciated.
Rolando Parada Puig• 200@RolandoParadaPuig
Posted
i like your solution! try to keep improving it as you learn more and more and try to use more semantic tags, its a html5 convention and it´s a good practice
Marked as helpful
0
