Community Feedback

• 0

  This comment was deleted 2 weeks ago

  

  alicepsz980Posted 2 weeks ago

  Reply
• 0

  Tente com px, em vez de rem.

  

  alicepsz980Posted 2 weeks ago

  Reply

  • 0

    Hi @alicepsz98 , thanks for the feedback! I wish the answer were that easy 🙂 border-radius can actually take both percentage and length values (such as px and rem).

    

    emestabillo610Posted 2 weeks ago

    Reply

  Add a comment
• 3

  Hi.

  The issue is because children elements are overflowing so their borders are shown.

  The solution is to remove all the border-radius that you have and leave it only in the main tag + you must use the overflow: hidden property, so children's borders aren't displayed

  main {
      ...
      border-radius: 1.5rem;
      overflow: hidden;
  }

  

  juanmesa2097930Posted 2 weeks ago

  Reply

  • 0

    Hi @juanmesa2097 I've already tried this and it works for the mobile version. The desktop view has that popup that overflows the container, and applying overflow: hidden will cut it up to the point of the container's border, which is about half of the popup. Sigh. Thanks for taking a look!

    

    emestabillo610Posted 2 weeks ago

    Reply
  • 1

    @emestabillo you can still use it in the image-container

    main {
        border-radius: 1.5rem
    }
    
    main .image-container {
        width: 100%;
        border-radius: 1.5rem 0 0 1.5rem;
        overflow: hidden;
    }

    Those would be the only places where you would set the border-radius

    Another possibility could be background-clip: content-box but you still need to replicate the border-radius in the image-container

    

    juanmesa2097930Posted 2 weeks ago

    Reply
  • 1

    @juanmesa2097 Ah I see, this method will also work. Thank you so much!

    

    emestabillo610Posted 2 weeks ago

    Reply
  • 0

    @emestabillo of course, no problem.

    

    juanmesa2097930Posted 2 weeks ago

    Reply

  Add a comment
• 1

  Hey, generally setting border-radius and overflow:hidden on parent container works, but for popup you probably need to use js, coz on mobile hover wouldn't work. Nice solution though

  Cheers, Roman

  

  rfilenko2,000Posted 2 weeks ago

  Reply

  • 1

    Hi @rfilenko , good point! The hover states wasn’t originally called for in the style guide but I added it anyway. The popup active state, on the other hand, doesn’t remove itself once applied on mobile..and that I might have to adjust by js. Thank you for the comment!

    

    emestabillo610Posted 2 weeks ago

    Reply

  Add a comment
• 2

  Emmilie, this looks exactly like the design. I can't even tell the difference! Great job! I did find a few areas of improvement besides the border-radius.

  • The hover states should transition together. Currently, on desktop, when you hover on the CTA, the shared bubble transitions a little later. Onmouseleave, the button transitions quickly and before the shared button. There's no cohesion right now.
  • Missing content box-shadow.
  • The CTA doesn't work well on mobile. Either use an input button and make it pure CSS or use Javascript to toggle the states as the mobile user clicks on the CTA.

  

  shashilo1,090Posted 2 weeks ago

  Reply

  • 0

    Hi @shashilo , great eye as usual! Will you be my mentor?? 😄 You're absolutely right about the CTA. I tried to be slick, and I paid the price lol. I'll revisit the project to include all above comments. Thank you so much!

    

    emestabillo610Posted 2 weeks ago

    Reply

  Add a comment

Give some feedback to emestabillo about their solution