Skip to content
OverviewSolutions
22
Comments
8
Akash Banchhor
@akash02ab

All comments

  • Akash Banchhor•230
    @akash02abPosted 8 months ago

    Desktop View is looking Good!

    Need to optimize for mobile view.

    Few Suggestions:

    • Use consistent CSS naming conventions to make sure the styles are easily readable.
    • Use CSS variable to avoid repeating yourself. (makes it even more useful when you want to change something, you change it at one place)
    • Use semantic html such as main, section, article etc instead of using div's. Instead of <div class="container"> --> <main class="container"> is better of main content of the page.
    • Keep all the styles in one place (make it's easier to debug and good practice while working on a project with team). Instead of using inline style for margin-left it's better to put it in external css i.e. style.css

    Keep Up the good work. Happy Coding!! 👋

    0
  • Akash Banchhor•230
    @akash02abPosted 9 months ago

    Is screenshot broken cause I'm using nested CSS ?

    Can someone please confirm!

    0
  • Akash Banchhor•230
    @akash02abPosted 9 months ago

    Looking good!

    Small suggesting increase the font-size of h1 element.

    h1 {
  font-size: 20px; ❌
  font-size: 22px; ✔️
}

    Happy Coding!! 🎇

    PS: If you using images for your design consider using PerfectPixel extension to get the output as close as the design.

    0
  • Akash Banchhor•230
    @akash02abPosted 10 months ago

    Looking good,

    Some improvements to match the design:

    • Divider SVG is non completely visible and it's position is off. (placement of svg should be exactly in between advice-text and dice-button)
    • Card's background color is off, it should be darker.
    • The hover effect on dice-button is missing.
    • Look at 'no-cache' attribute while fetching the data from AdviceSlip API, In some browsers (firefox) response is cached by default.
    0
  • Akash Banchhor•230
    @akash02abPosted 10 months ago

    Good job! on desktop-view, but mobile-view need some work.

    1
  • Akash Banchhor•230
    @akash02abPosted 10 months ago

    Hi @Deividb2, great work on using semantic HTML elements.

    Few suggestions:

    • Give position: absolute; to the dice button to match the design. (the button is inside the card in your implementation, where it should be floating at the bottom-center)

    • Give higher border-radius to the card.

    • You may want to look no-cache attribute of axios, as Firefox browser store the advice as cache and new advice does not appear on button click, however it works perfectly fine on chrome and edge.

    0
  • Madhavan•340
    @madhavan-tsSubmitted 10 months ago

    I have completed this as my 19th challenge in Frontend Mentors.

    I have used ReactJS and I learned to use the axios API Call in React. This was a good practice for me. Please look at the code and tell me whether the approach is correct or not

    I have learned several things. By making mistakes😅.

    1. We cannot use box-shadow property along with clip-path property.
    2. We cannot use useEffect hook inside Event handlers.

    If you have to correct any mistakes that I have done.... I am happy to recive the comments.

    Advance thanks for the comments 🫡

    Advice Generator App using ReactJS

    2
    Akash Banchhor•230
    @akash02abPosted 10 months ago

    1.In Firefox browser the response is cached thus not showing new "advice" on button click. You may want to check Using JavaScript Axios/Fetch. Can you disable browser cache?

    2.When "Loading ..." text occur the "Dice" button jumps from it's original position, this is due to length of "Advice" text, it can be of any number of lines thus, changing the height of the card. To deal with it you can give fix min-height to the card.

     .advice__box {
   min-height: 30rem;
 }

    you also need to give position: absolute; to .advice__btn-wrap.

    0
  • Akash Banchhor•230
    @akash02abPosted 10 months ago

    The response is cached and the advice doesn't change on Firefox browser. You may want to use :-

    fetch(url, {cache: "no-cache"})
    0