Skip to content
OverviewSolutions
6
Comments
3
DJ Drakos
@dj-drakos

All comments

  • DJ Drakos•150
    @dj-drakosPosted almost 2 years ago

    Hey Carlos, great work on this. I have a couple of notes for you to take this to the next level.

    • Most important: Check out the accessibility report and fix the issues in your code that are causing warnings.
    • In the mockup, the background covers the entire viewport. Try working with the background-size property.
    • Pay close attention to the color and direction of the shadows on the card and the button.
    • The Annual Plan option should have a background.
    • Your solution looks pretty small compared to the design mockup. Consider making the desktop version larger.

    Overall, great work! Nice work using pseudoclasses and nested element selectors.

    Marked as helpful
    1
  • Brett•60
    @b-capraSubmitted almost 2 years ago

    My biggest challenge in building this project was handling how to close active accordion components whenever another is opened, as having multiple expanded will cause them to overflow the card. I'm happy with the solution I've implemented, though as I've been learning React, I think using 'useState' could have accomplished this a bit more elegantly in terms of the order of how components are rendered.

    FAQ accordion with animated transitions

    1
    DJ Drakos•150
    @dj-drakosPosted almost 2 years ago

    Hey Brett, this looks amazing! Pixel perfect.

    Your hunch to use React's useState hook is a good one. The collapsible panels are a pretty common React Component pattern called Accordions. They can be highly customizable, and have accessibility, server side rendering, and SEO considerations built in "under the hood." Check out Material UI and Radix to get a sense of what you can do with them.

    Marked as helpful
    1
  • Greatemmet•20
    @GREATEMMETSubmitted about 2 years ago

    it was difficult working with the desktop image, as i was trying to fit it into a 500px height container. i set the height of the image to be 100% but it ignored the height container and maintained its predefined height. Tried different values, went back and forth my code to see of i made any mistakes but i found nothing. i later had to set the container to take the height of the image. if there is a way around this, i'd really love to know.. All feedbacks are welcome...

    responsiveness was done with flexbox

    3
    DJ Drakos•150
    @dj-drakosPosted about 2 years ago

    Hey @GREATEMMET, good work on this.

    I took a peek at your repo, and noticed you're now using the width property to control the image size, which seems to be working! Nice.

    Without playing around with your code, I'm wondering if the issue you're having has to do with how CSS treats the height property. If the parent element (.desktop-img) doesn't have an explicitly defined height, CSS will ignore the height you set on the child (img). That explicitly defined height could be set on the .desktop-img div, or be inherited from its parent... Lots of different approaches.

    CSS-Tricks has a good post that goes more in depth.

    Marked as helpful
    0