ReeAya• 40
@ReeAya
Submitted
Hi. I'm a newbie and gave this NFT card a good try. I would love any feedback on areas to improve or better ways to build it. Thanks in advance!
Saul
@xsaul
All comments
- Saul• 1,260
@xsaul
Posted
Hello, ReeAya nice job to be your first challenge! There are better ways to center the content instead of using position relative. I would recommend "margin: 0 auto" and you can modify the " 0" depending on which distance you wanna for the top and bottom.
The card should be bigger so you should check out your height and width values.
Also, you forgot to add a shadow to the card to make it look nicer.
And if you removed the info from the footer would be better if you directly delete the footer tag.
Check those details and your work will be much better!
Marked as helpful
0 - Xiomara Arzola• 270
@Xiomiau
Submitted
I don't have the pro version, sometimes the measurements don't stay the same, but I do my best. :D
Saul• 1,260@xsaul
Posted
Hola Xiomara hiciste un gran trabajo! Solo tiene un pequeño detalle tu diseño. En la versión de escritorio la imagen está en la parte superior del contenedor cuando debería estar a la izquierda como si fuera el diseño para moviles. Deberías checar ese detalle, tal vez pusiste mal un media query o algo similar.
Pero a excepción de ese lo hiciste muy bien, felicidades!
Marked as helpful
0 - Luiza Uszacki• 250
@LuizaUszacki
Submitted
I tried to make this project more accessible but I'm still a little confused with aria so... Please let me know if there is something that I did wrong or if there is something that I can improve.
Saul• 1,260@xsaul
Posted
Oi Luiza seu trabalho ficou muito legal, parabéns! O aria label é usado pelo leitor de tela para ter mais acessibilidade para as pessoas com deficiência visual ou se o navegador tem problemas para carregar o DOM.
Marked as helpful
1 - Praise• 80
@Omoh123
Submitted
I would love some feedbacks on properties i did not make use of well. Thank you
Saul• 1,260@xsaul
Posted
Hello, Praise I did a few changes to improve your project. To the image-text <div>, I changed the padding to 12px and the width to 40% like that the card will look wider.
To the image, I changed the width and height to 100% so it can fit better in the container and look bigger.
To the paragraph, I added a padding of 0 for top and bottom and 1rem for left and right like that the text will be centered.
And don't forget that you have to use a specific font!
Check out the values I told you and tell me what you think!
0 - Maria Dennise• 30
@dennisemed
Submitted
Saul• 1,260@xsaul
Posted
Oi Maria seu projeto ficou muito bom, parabéns! Só tem um pequeno erro seu codigo.
1.- Você não pode usar: link="#" em um <button> 2.- Para apontar para um endereço diferente no HTML você precisar usar: " href="" "
Mas além disso seu trabalho ficou bom!
0 @httpsemilly
Submitted
Hello, everyone!
I'm still at the beginning of my programming study journey and this is the first project I create. I had a lot of difficulties making it responsive, but I think I got it.
Feel free to give me any feedback or comments. Thank you!
Saul• 1,260@xsaul
Posted
Hello Emilly, it is an amazing job! Just a comment: if in your <main> you use a height of 100vh you wouldn't need to add a media query. But is just a tiny detail. Congrats!
0- Mariana• 320
@marianaceci
Submitted
Hi, everyone! I struggled a little to fit the screen=375px; I used media query, but maybe it would've been easier some other way... And I need to pratice more Bootstrap... I tried to use it, but gave up because I was taking too long... But looking at some solutions I finally understood how to use the position property :D
Looking forward to your feedback! Thanks!! Ps.: english is not my first language, so I'm sorry for any mistakes here...
Saul• 1,260@xsaul
Posted
Oi Mariana seu desenho ficou muito bom, parabéns! Só tem um problema, vc deveria remover o "overflow: hidden" do body porque em telas de tablets e telefones o usuário não poderá fazer scroll e por causa disso não vai olhar todo o design.
Marked as helpful
1 - Rodney Foster Jr• 30
@T000bias
Submitted
I am not sure that the spacing on my text is the same as in the JPG. How can I go about fixing this? Why is my "Challenge by" section so low from my project? How can I fix this?
Saul• 1,260@xsaul
Posted
Hello Rodney! I've been checking out your code and there are a few things to improve.
I added a width value to the card of 290px. Like that the card has less white space on the sides.
Nor the title nor the paragraph need padding, so if you remove it can look more similar to the original design. Also for the paragraph, I changed the line-height value to: 1.3.
Answering your question, to make the position higher of the text "Challenge by..." just add a margin-top with a negative value, I think -5rem looks good. But if you do that you would need to check it out in the media queries and be changing the value according to the screen size
But in general, you did a nice job, congrats!
Marked as helpful
1 - João Lucas• 10
@JoaoOVER
Submitted
Só iniciante no estudo de html e css, ser poderem me da alguma dica ou fala alguma coisa que eu fiz errado no código eu agradeço '-')
Saul• 1,260@xsaul
Posted
Oi João, seu desenho ficou perfeito! Só no margem, se são os mesmos valores não precisa escrever duas vezes, "150px auto" faria exatamente a mesma coisa. E falando do margem eu acho que com "50px auto" ficaria centrado na tela e seu site não teria a scroll bar. De novo parabens é um ótimo trabalho!
Marked as helpful
0 - ntandomagana• 90
@ntandomagana
Submitted
I haven't been practicing for almost over a year now bc of mental issues, but we are back. Visited this challenge I downloaded a while back and it was so hard having to remember a few things. I watched and followed a video. I feel like starting from scratch. Please be nice :)
Saul• 1,260@xsaul
Posted
Hello ntandomagana! Your JS code isn't working because in your HTML you wrote "" I think you should remove the "./" because isn't uploaded in a different folder.
Just a few things you can improve in your CSS:
Remember always when you do a button add a cursor: pointer. And also, your entire right section can be wider and would look better.
But in general, you did a pretty good job congrats!
Marked as helpful
0 @Mixhetl
Submitted
You are using the <figure> tag and the <article> tag incorrectly. Since its characteristics do not lend themselves to that project. Are not necessary.
I think I could improve some classes so as not to duplicate my CSS
Please friends, inspect my code and be harsh in terms of criticism, only then will I improve. Thanks so much.
Saul• 1,260@xsaul
Posted
Hola Luis, en general hiciste un gran trabajo! Chequé tu CSS y le hice unos cambios para hacerlo más parecido al diseño original. Puedes revisarlo y decirme tu opinión!
Tu carta yo la veía un poco más larga entonces le reduje la altura y le aumenté el ancho. También le cambié el margen para que de esa manera tu sitio no tenga una barra para hacer scroll.
.card{
width: 325px; height: 475px; margin: 50px auto;}
Con los cambios hechos a la carta la imagen se veía mucho más grande que el texto, entonces también le reduje el ancho.
.img{
width: 95%;}
Usando el margen de esta manera ambos parrafos van a estar alineados. Usando "20px auto" centrarías el segundo parrafo.
.title{
margin: 20px;}
Y hablando sobre tu duda. En este diseño cada elemento tiene diferentes propiedades entonces no hay manera de crear clases que se puedan reutilisar.
Marked as helpful
0- Alexioos95• 20
@Alexioos95
Submitted
Saul• 1,260@xsaul
Posted
Hello Alexioos, you did a nice job! For your text, is really simple you just need to change the font-weight to 600.
And about your shadow what you wrote is "box-shadow: 0px 30px 15px 30px hsl(215, 70%, 10%)," so you made a mistyping and is just change to ";" and like that your shadow will be displayed.
Marked as helpful
0 - PGlenda Joyce Reed• 920
@Glenda9031
Submitted
It took me four days to complete this project! I first used a bunch of 'divs' which made everything out of alignment. I'm still confused about when to use 'divs'. It was only when I removed the 'divs' that everything lined up. I also had confusion around using 'flexbox' and resizing the image. I would say that resizing the image took me 3 1/2 days! Please give me feedback!
Saul• 1,260@xsaul
Posted
Hello, Glenda! I've been looking at your code and the paragraph is out of the container, so you should add an <div> inside your <main> and this div will be your "white box" and like that you can adjust the height and width to any size you want to.
Basically, everything you put inside a <div> gets like inside a box so when you removed all of them all the elements got "free" to move and you could align them. The flexbox property is used to choose the direction of the elements that are inside your <div> if you use flexbox then you need to use "flex-direction" with that property you can decide the orientation of your elements. And about the size of the images just use the properties "width" and "height" and use px as a unit of size.
I hope I could explain it well. 😅
Marked as helpful
0 - Hadeer Khaled• 10
@hadeer-khaled
Submitted
Saul• 1,260@xsaul
Posted
Hello Hadeer, I see that the image is kinda big. So it would be better if you make the container's and the image's width smaller so your design can look more like a card. But in general, you did a great job!
0 - Han Silver• 10
@HanSilver227
Submitted
How can I minimize the number of classes and div in my code? The webpage does not display the image after uploading to the live site, please any suggestions on how to fix this issue?.
Saul• 1,260@xsaul
Posted
Hello Han, your site is not working because you added a theme on Github pages. You should remove it because with that will never show the project you actually did.
Marked as helpful
1 - Tan Yi Hao• 40
@tyihao
Submitted
Feedback is more than welcome! This is my first attempt at coding HTML and CSS and it was great fun. I have learned a ton and would love to hear how I can further improve it.
Saul• 1,260@xsaul
Posted
Hello Tan Yi Hao, you did an amazing job! Just remember that the number of the headings is in order of importance. So in this case instead of <h3> you should use <h1>. Also if you will use only one font write it in the body, like that will be applied on all your site. Congrats for your design!
Marked as helpful
1 - Tenzin• 140
@lekdup
Submitted
i am starting with HTML5 and CSS3, all feedback and comments are welcome. it's with great honor to learn from others! thanks 😁
Saul• 1,260@xsaul
Posted
Hello Tenzin! There are a few things with your design. First, the images are not displaying and it is because in your HTML file you are grabbing the images from a folder called "images" but the images you uploaded one by one and not in a folder. Also below the card is duplicated the text so you should remove it. And finally, I think would be better if you make the height and width way much smaller because it is a card and not a full-size element.
Work on that and your design will be much better! 😃
Marked as helpful
0 - Veronica Valdez• 130
@veronicajujuy
Submitted
This is my fourth challenge, I would be grateful if you could give me feedback
Thank you!!
Saul• 1,260@xsaul
Posted
Hello Veronica, you did a nice job! I think the button's shadow should be with less opacity maybe 30%-35% would be better. Also, you should put the bottom section inside the same div and display it with flex or grid instead of displaying it as 2 different sections. And remember always add a hover effect and cursor pointer to the buttons. By the way, I tried to look at your repository and it says the site isn't available so you should check about it.
0 - Marilena Sanchez• 80
@MarilenaSG
Submitted
Hello!
My third challenge and I think the most accurate of them. I got stuck with the quotations background, but solved it reviewing some code and it helped me a lot.
My biggest goal was to make a media query file and get the responsive with only to lines of code. I felt great doing cleaner code than before.
Hope you like it. Love this community. Regards. Mary
Saul• 1,260@xsaul
Posted
Hola Marilena hiciste un gran trabajo felicidades! El único detalle que veo es que 1024px es un tamaño de laptop entonces ahí debería de seguir siendo display grid. Yo creo que el display block sería mejor en 768px. Saludos!
Marked as helpful
0 Hi everyone, I am open to feedback. I would be very grateful if you could check my code and suggest how I can improve CSS.
Saul• 1,260@xsaul
Posted
Hello Rita you did an amazing job! The only thing I see different is that the text of "info container" is in the center and should be on left. Adding a negative margin-left will be enough.
Marked as helpful
1- PCarlos Castillo• 360
@CharlieCastleWeb
Submitted
Saul• 1,260@xsaul
Posted
Hello Carlos you did a great job! Answering your question what you need to do is add a border: transparent and like that it won't grow up. Also, I advise you to add a transition when you hover so it can look nicer and smoother.
Marked as helpful
0 - Iván Serrano• 20
@IvanSerranoGit
Submitted
Saul• 1,260@xsaul
Posted
Hello Iván! To achieve the right color in the image you should add .img_container::after{ content:""; background-color: var(--accent); opacity: 0.6; width:100%; }
Marked as helpful
0 - Melissa Villegas• 120
@melissavi08
Submitted
Saul• 1,260@xsaul
Posted
Hello Melissa you did a fantastic job! The only thing I could say is to add a transition on the button's hover so it can look smoother. But in general, it looks terrific.
0
