Abhilashpandey
@freakyjonesAll comments
- @Simplyvoda@freakyjones
Hi Vodina,
** congrats on fetching data through API 🎉🎉🎉**
I just saw your code and here is one suggestion for you. Always handle errors on your API calls
fetch('https://api.adviceslip.com/advice').then((data) => { // console.log(data); return data.json(); }).then((adviceData) => { // console.log(adviceData.slip.id); document.getElementById('advice-id').innerHTML = adviceData.slip.id; // console.log(adviceData.slip.advice); document.getElementById('advice-box').innerHTML = adviceData.slip.advice; }).catch((err)=>{ console.log("do whatever you want with this error", err) })
Hope this helps, Thanks Happy coding :)
Marked as helpful - @rimshub@freakyjones
Hi Rimsha congrats on completing the challenge 🎉🎉
your website looks just like the design. while reviewing your website, one mistake caught my eye, the filter by region dropdown list item cursor is not a pointer.
Happy coding :)
- P@Fran-Sanabria@freakyjones
Hi Fran,
congratulation on completing the challenge
I just saw your code and like to give my one suggestion with your permission,
- Rather than using media queries throughout your CSS file you can put all your code related to that responsiveness in one place, It will be much cleaner and you have your particular device code in place.
@media screen and (min-width:950px) { img{ height: 268px; width: 268px; }, h1{ font-size: 21px; } }
I hope it helps, Thanks. Happy coding :)
- @MrEasty94@freakyjones
Hi Michael,
congratulation on completing the challenge
I just saw your code and like to give you one suggestion
- instead of using an anchor tag on add cart, a button tag will be a better option for accessibility.
I hope it helps, Thanks Happy coding :)
- @Paula-Carlech@freakyjones
Hi Paula,
congratulation on completing the challenge
I just saw your code and like to give some suggestions with your permission,
-
you can add another key to every object in the data.json and then map it for every bar so that you can change the color of the bars.
-
Start using semantic HTML in your code. Here is a blog for you to get started with semantic HTML. https://www.simplilearn.com/tutorials/html-tutorial/html-semantics
If you want to know how to do this with only Js you can check out my solution.
I hope it helps, Thanks happy coding :)
Marked as helpful -
- @casserole27@freakyjones
Hello Clewis,
congratulation on completing the challenge. I just saw your code, Here is my one suggestion that may help you in the future.
Use of universal operator to get rid of the default browser Style instead of using margin:0, padding:0 for every device
*{ margin:0; padding:0; box-sizing:border-box }
I hope it helps, Thanks Happy coding :)
Marked as helpful - @Jetyun@freakyjones
Hi Jetyun, congratulation on completing the challenge
I just saw your code, and here are my suggestion,
- Using responsive units (rem, em,vh,vw) in your code will help you to make a responsive website.
- Using min and max will also help you to make a responsive website.
Here is one of my favorite videos regarding website responsiveness (https://www.youtube.com/watch?v=srvUrASNj0s).
I hope it helps. Happy coding :)
Marked as helpful - @lhpellizzon@freakyjones
Awesome, More power to you Luis Pellizzon.
Happy coding :)
- @abymani@freakyjones
Hi Abdul,
congrats on completing the challenge
I just saw your code, I would like to give some suggestions with your permission
- start using data attributes in your code, It will be much more efficient to change open and hide the nav and reduce the use of excessive javascript.
- try using a z-index while stacking multiple components in one place . Here is one of my favorite videos regarding z-index (https://www.youtube.com/watch?v=uS8l4YRXbaw).
hope it helps, Thanks Happy coding :)
- @fsuropaty@freakyjones
Hi, fsuropaty,
congratulation on completing the challenge.
There is two way you can handle image responsiveness
- use it as a background image as per your code it will be like
#image-section{ background-image:URL(image path); background-position:center; background-size:cover; background-repeat:no-repeat; }
My solution also uses this method.
- The second method will be when we make img take a width of 100% so that it can inherit the parent container responsive property. The code will be something like
#imagesection{ width: 10rem; } #imagesection img{ width:100% }
I hope this helps. Happy coding:)
Marked as helpful - @Kapil101527@freakyjones
Hi, Kapil,
congrats on completing the challenge.
I just saw your solution, Here is my one suggestion
Wrap your mobile-bg and parent-bg images inside the individual parent div and give them 100% width of the parent. That can make your image responsive. Check my solution for reference.
Hope this helps. Thanks, and happy coding :).
Marked as helpful - @iOwsla@freakyjones
Hi Owsla,
congrats on completing the challenge.
I just review your solution and it almost looks like the design, Here is some tips that can improve your solution
- Change the width of box content and box image div to 100%. It will make two div equal width.
- instead of using a margin for giving space between parent div and inside elements, padding will be an awesome choice.
Hope it helps, Thanks, Happy coding :)
Marked as helpful - @Ejemeare@freakyjones
Hi stepheigbe
congrats on completing the challenge .
- If your current src for the Image is
\media\product.jpg
change it to./media/product.jpg
hopefully it will work. - For the second Image change the src to
./media/iconcart.svg
Thanks, Happy coding :)
Marked as helpful - If your current src for the Image is
- @margaridalsemedo@freakyjones
Hi Margarida,
congratulation on completing this challenge. I just saw your code and would like to give some suggestions with your permission
-
your border radius is not showing up in desktop view probably because your parent container card is much bigger than your child div card image. Try to give your parent a fixed height and then make your card-image container height 100% of the parent's height. in addition you can give your background image `{background-position:center, background-size:cover}.
-
More space between the card text container and its content will greatly improve your UI.
personally, i like to add
{margin:0,padding:0}
with*{box-sizing:border-box}
for getting rid of default browser styles.hope it helps, Thanks, happy coding :)
-
- @Valhalla-2@freakyjones
Hi kounik,
congratulation on completing this challenge.I just saw your code and would like to suggest two videos for learning more about website responsiveness and box-model, 1.https://www.youtube.com/watch?v=srvUrASNj0s //for website responsiveness 2.https://www.youtube.com/watch?v=-8ORfgUa8ow //for CSS box modal
the 2nd video is pretty long if you don't want to watch the whole video, you can watch only the box-modal section.
hopefully this help, Thanks, and happy coding :)
Marked as helpful - @Ali-Primer@freakyjones
Hello Ali, congratulation on finishing your challenge.
I just saw your code and I would like to give some suggestions with your permission,
- keep your images in one folder. it will help you manage your project in long run.
- use a universal selector (*) to get rid of default browser styles like
*{margin:0,padding:0,box-sizing:border-box}
.
- @Maggus407@freakyjones
Hello Markus, congrats on finishing the challenge.
I saw your code and it's pretty awesome, I tried this challenge a while ago and was having a problem with the flexbox issue. Regarding your image responsiveness, I think there is another approach: making a parent image container and then putting the image inside of that div with a width of 100%. let me know your thoughts. Thanks
Happy coding :)
- @thisisharsh7@freakyjones
Hi Harsh, Congratulation on completing the task . Also here is one of my favorite videos regarding the basics of HTML, and CSS. Hopefully, it will help you master HTML and CSS a lot faster. https://www.youtube.com/watch?v=-8ORfgUa8ow.