Skip to content
OverviewSolutions
15
Comments
27
Candela Bramucci
@bramuccci

All comments

  • Candela Bramucci•660
    @bramuccci

    Hi Morgan! It's easy, just remove the margin-bottom from the buttons and add margin-top: auto. Also, I think you should make a div that represent the card. Inside, have three divs for the sections. Something like div.card>div.card__division*3. Then, you could put border-radius to the parent div and overflow: hidden to make the child follow it. To finish with, increase the line-height on the paragraphs. Wish this help you!

    Marked as helpful
  • Candela Bramucci•660
    @bramuccci

    Hi Naveen! Awesome solution, I love it. I just had a question, why do you choose using ::after instead of brackground-image? Is it more easy that way or have something to do with accessibility?

  • Candela Bramucci•660
    @bramuccci

    Hola Santiago! Ese error en tu desafío anterior de debe a que no usaste ningún h1 en tu html. Como es una tarjeta aislada creo que la decisión de un h2 es correcta. Solo recuerda que cuando hagas una página web entera tenés que incluir un y solo un h1. Sobre este desafío, tengo varias sugerencias:

    • Tu breakpoint es de 1100 px. Es innecesariamente grande y deja a las pantallas de 1024px y 768px (laptop y tablet) de lado. Te recomiendo usar 600px y leer este artículo
    • En el body pusiste font-weight: 400px;
    • Tu card está envuelta solamente por el body. Una tarjeta es un único elemento. Deberías usar un elemento (article, por ejemplo) para toda la tarjeta y usar divs para dividirla. Mi estructura sería article>div*3>h3+p+button
    • Usar flexbox en el body te va a dar problemas en cuanto quieras agregar otros elementos

    Espero que estos tips te ayuden :) Keep coding!

  • Candela Bramucci•660
    @bramuccci

    Hi Amila! Keep in mind that you need https or local host to use fetch. Do you try using the vsc extension live server? It's what I used. If you want, you can see my JavaScript code here. Also, would you like to share your old code? About the rest of the page, I believe you did it fantastic. Just is a little to small.

  • Candela Bramucci•660
    @bramuccci

    Hi Attila! I see a little problem. When I put an incorrect email all the errors shows up, even if the other fields are correct. This only happen with the email. All the other functionalities are well done. Oh and the text of "try it free..." isn't center on the mobile version.

    Overall, good solution :)

    Marked as helpful
  • Candela Bramucci•660
    @bramuccci

    Hello rubbersouls, congratulation for your first challenge in this platform 😊 It's amazing to see new people. I'm surprise, your solution is very close to the original design. Here are the things I think can help you:

    • Don't use flex (or grid) in your body. You can see that this give you problems, like the need of using position: absolute in the attribution (which in screen of 1024px overlaps with the card). If you want to center use margin: auto, although I wouldn't put in the body but in the section.
    • No need to use a separate class for the width. Just put it in the .card
    • There's absolutely no need of use so many divs inside the card. Just put the content like .card>img+.card__content. Maybe you want the img inside of a div, that is also ok.
    • I write .card__content because this is helpful for the padding. Add padding to that div and you will not have to adjust every element manually.
    • Instead of a @media you can use in the card: width: 90vw; max-width: 50rem (just an example, change those values).

    I... write a lot. This is only to help, I really think that you did a great job. Keep coding!

    Marked as helpful
  • Candela Bramucci•660
    @bramuccci

    Hola Ricardo! Acá van algunos tips:

    • Pon el width directamente en la card (container), y no en el main.
    • Podés usar font-family: 'Outfit', sans-serif; directamente en el body y así no repetir líneas.
    • No hace falta anidar tantos divs adentro de la card. Yo haría .card>img+h1+p y daría un padding a card. De esta forma no hace falta ajustar cada elemento manualmente.
    • Te recomiendo aprender de medidas em y rem.

    Veo tu código de css muy ordenado, sacando algunas líneas innecesarias mejoraría aún más. Felicidades por tu solución :)

  • Candela Bramucci•660
    @bramuccci

    Hi rummer! If you see your solution on a screen of 1024px (like mine) you will see that the buttons get out of the card. I think I can help with that.

    • First, do *{ margin: 0, padding: 0} . With that you can work better with the spacing.
    • Remove the grid-template-rows of your container. Just leave the grid-template-colums: 1fr
    • Put the background in the cards, and no in the div inside them. Then, add a padding. With this structure you can adjust the elements like header, img, etc.
    • I see that you use react. I recommend to go back to the bases with only css and html. Keep coding!
    Marked as helpful
  • Candela Bramucci•660
    @bramuccci

    Hi! Congratulations for your solution, the responsive is good and the layout seems correct. Here are some tips:

    • You could use semantic better. Use elements like header and footer. I recommend the structure header+main>(section*3)+footer.
    • Try naming your classes in a more descriptive way, like "benefits", "testimonials," "gallery" or what you think is better.
    • Follow header order; I see a header 1 and 3, but not a 2
    • The nav is a little too small.

    Overall I think is a great solution! I hope this helps you.

  • Candela Bramucci•660
    @bramuccci

    Hola! Felicidades por tu solución, muy parecida a la original. Acá van algunos detalles que te pueden servir:

    • main no debería ser directamente tu card, en ves de eso usa un div o un article ( main>article.card )
    • Dentro de .card yo tendría .card__header y .card__content, de esta manera podés darle un padding y un gap al contenido, así no tendrías que ajustar cada elemento manualmente.
    • Olvidaste de poner sombras (en la card y en el botón azul)
    • Olvidaste la imagen de fondo

    Espero ayudar 😊

    Marked as helpful
  • Candela Bramucci•660
    @bramuccci

    Hey this looks really nice! Here are some picky details that I would change: -The cards are typically articles, not sections (see https://developer.mozilla.org/en-US/docs/Web/HTML/Element/article). So is better to do main>article>div (each part of the big card is a div) -Instead of btn-one, btn-two, btn-three, you could use variables. Put a '--color-accent' and change for every div

    But these are minor things, you did great 😃

    Marked as helpful
  • Candela Bramucci•660
    @bramuccci

    Hi! For me your solution sees great with flex (maybe I would the media querie to a lower min value) but as you say, maybe it's more elegant with grid because the use of that extra div. I got this way: first, remove that div

    .cards{
display: grid;
grid-template-columns: 1fr 1fr 1fr;
grid-template-rows: auto auto;
}
.card__supervisor {
    grid-row: 1/3;
}
.card__calculator {
    grid-column: 2/3;
}
.card__karma {
    grid-column: 3/4;
    grid-row: 1/3;
}

    Feel free to try it! Your solution is great and I hope this help you :D

    Marked as helpful
  • Candela Bramucci•660
    @bramuccci

    Hi Louie! Your form only checks if the input has any value, so if I type something like a space it will be considered valid. This in the real life has no sense. Also, you use the html email validation which it's no secure because the users can modify the html and erase that attribute. I recommend to learn about regular expressions and form verification (for the later you can see https://developer.mozilla.org/en-US/docs/Learn/Forms/Form_validation). I hope this help you! happy coding

    Marked as helpful
  • Candela Bramucci•660
    @bramuccci

    Hi Zack! I saw your code and also think it's redundant 😅. You don't need to do different h2 and p for every situation, instead put a single h2.current time , p.previous-time (or something like that) and then modify it in JS. Now, put an addEventListener to the buttons daily, weekly, monthly(I think they should be li) and call a function. That function can edit the time frames using a for cycle. You can use template string for every context (like currentTime.textContent = data[index].timeFrames.${context}.current).

    I hope I have explained well, don't doubt in asking if you want. Remember when you are coding, don't repeat yourself. This is great for a first challenge summited, keep going~

    Marked as helpful
  • Candela Bramucci•660
    @bramuccci

    Hello! This is really nice, almost identical to the original. I think you should give to .content__error a display: none and change later with JS (because it give to much space in the mobile version). Keep coding 🤗

    Marked as helpful
  • Candela Bramucci•660
    @bramuccci

    wow, the animations are amazing 😦 I love this! The only I noticed is that the card in small devices (less than 375px) isn't very responsive, but great solution anyway

    Marked as helpful
  • Candela Bramucci•660
    @bramuccci

    Hi Leon, this solution looks amazing! I see it in the newsletter and I think it's great. In fact, I'm studying it so thanks for the readme!! The dark and light theme looks amazing too, just a question: The theme preference is not saved in the localStorage? When I reload the page, is always dark. Please keep coding! This is awesome.

    Marked as helpful
  • Candela Bramucci•660
    @bramuccci

    Wow, this is really incredible! The design is indistinguible from the original and the responsive is magnificent, you use the grid layout so well. I really like this so much, please keep going 💖. And for the dots... maybe you can put a mini menu who cover the card with fake options? You can use addEvenListener click and mouseover.