Design comparison
Solution retrospective
I'm learning javascript right now so I'm really proud about this project.
What challenges did you encounter, and how did you overcome them?I had a problem with the image path, I put the css file in a hierarchy above to catch the image folder.
What specific areas of your project would you like help with?I'd like to know what can i improve in my javascript code, my html too, whatever you thinks I need to improve lmao
Community feedback
- @0xabdulkhaliqPosted 7 months ago
Hello there 👋. Congratulations on successfully completing the challenge! 🎉
- I have a suggestion regarding your code that I believe will be of great interest to you.
MAKING ACCESSIBLE ACCORDIONS 🔴 :
- The best way to go with creating the accordion elements in this challenge would be with the
details
andsummary
elements instead of using plaindiv
element with no semantic meaning.
- They are already fairly accessible and provided a clean, semantic way to create accordion elements. I see you have used the
div
&p
elements for the accordions, but those are not interactive or accessible by keyboard, so not all users will be able to open the accordions to see the content inside.
- MDN's reference is a great place to start learning about the
details
andsummary
elements if you are interested.
- If you have any questions or need further clarification, you can always check out
my submission
for legacy version of this challenge and/or feel free to reach out to me.
.
I hope you find this helpful 😄 Above all, the solution you submitted is great !
Happy coding!
Marked as helpful1 - @R3ygoskiPosted 7 months ago
Olá novamente Daniel, cá estou eu como sempre kkkk. Primeiramente, parabéns pelo projeto, e gostei dessa ideia de colocar o scroll no card ao invés de expandir ele.
Sobre a melhora do JavaScript, sinceramente eu não vi áreas que precisam de ser melhoradas, mas notei uma coisa peculiar, que foi a forma que você nomeou uma das suas variáveis, que foi com um estilo bem similar ao BEM, que foi essa variável
accordion__content
, isso não está errado, mas não é uma boa prática, geralmente utilizamos o camelCase no JS, mas também pode ser usado o snake_case, mas independente de qual forma escolher, tente não alterar ela, se foi camelCase, faça todos em camelCase até o fim. Porque isso ajuda na manutenção e organização do código.Sobre o HTML, a única parte que eu vejo que realmente precisa de um ajuste, seria a parte da semântica do HTML. Como por exemplo:
<div class="accordion__top-text">
poderia ser uma figure, e por aí vai.Sobre o CSS/Scss, pelo visto você pegou o jeito com sass, a única coisa que eu gostaria de falar aqui sobre isso é nesse trecho por exemplo (vale para semelhantes também):
& p { /* Props omitidas */ }
Você não precisaria utilizar o parent selector(&), isso porque o
p
, já está dentro do escopo do seletor pai, então você poderia colocar só op
. Geralmente só vai colocar esse parent selector assim, quando você quer usar um operador, exemplos:& > p
-Que dessa forma você seleciona o filho direto do parent.& ~ p
-Que dessa forma você seleciona os irmãos do parent.
E por aí vai.
Novamente, parabéns pelo seu projeto, você está avançando muito bem, e principalmente parabéns pela sua lógica de JS que por mais que você esteja começando agora, ela está bem concisa e objetiva. Caso algo que eu tenha dito não tenha ficado claro, você já sabe.
Marked as helpful1@mendesdomingosdevPosted 7 months ago@R3ygoski Muito obrigado <3 tô mto feliz de ter conseguido, eu tava tentando fazer a lógica em off o dia todo e tava esquisito kkkkkkk sobre o scroll eu botei porque não sabia fazer da maneira expandida, daí usei o
overflow
e nossa realmente me enrolei com o sass, teve parent até no javascript e eu tava tentando processar como aquilo tudo funcionava, mesmo assim obrigado mais uma vez por dar uma olhadinha no meu projeto.0@R3ygoskiPosted 7 months ago@mendesdomingosdev Não há de que, só uma dica, o seu
.accordion
não estava expandindo porque você definiu um tamanho pra ele por meio doheight
, isso fez com que ele tivesse um tamanho fixo, se você remover aqueleheight:600px
, ele vai ter um valorauto
, e aí ele irá se expandir/retrair de acordo com o conteúdo interno dele. Eu também demorei muito pra me acostumar com esseauto
. X_XMarked as helpful0@mendesdomingosdevPosted 7 months ago@R3ygoski Fiz questão de abrir o site pra testar kkkkkkkk era tão simples assim e eu nem imaginava que era só remover um negócio, eu tenho uma dúvidazinha 🥸 o accordion funciona, mas é possível ter um abre e fecha mais suave? eu tentei usar o
transition
mas não sei se tava usando na propriedade errada porque nada funcionava...0@R3ygoskiPosted 7 months ago@mendesdomingosdev Sim existe, só que, depende muito da forma que você utilizar, como ficaria imenso se eu mandasse um exemplo aqui, eu criei um CodePen, que enviarei aqui para que você possa ter acesso a ele.
Quando for dar uma olhada, veja a parte de
show
ehidden
, pois ambas que definem como otransition
deve se comportar. Pois o transition, ele basicamente cria uma "ponte" entre a forma que um conteúdo está para a que ele vai estar, ele faz uma suavização. Então ao alterar o tamanho da fonte e o tamanho do elementop
, isso faz com que ele faça uma curva suave até o destino.Marked as helpful1@mendesdomingosdevPosted 7 months ago@R3ygoski que interessante, não sabia que dava pra animar assim, vou estudar o código obggg <3
1
Please log in to post a comment
Log in with GitHubJoin our Discord community
Join thousands of Frontend Mentor community members taking the challenges, sharing resources, helping each other, and chatting about all things front-end!
Join our Discord