@fabarqzSubmitted over 1 year ago
Latest comments
- @avinnoPosted over 1 year ago
Hey! Great job on your solution here!
In regard to your question and some suggestions I have for you for this solution:
- I would try not to specify height on containers or major bodies of content so that you can let the content naturally flow down the page as needed. With regard to your solution code, I am specifically talking about your div.qr-card. For the div.qr-card I would not set a "min-height".
- I would not set your footer position to fixed. I think you are doing this to keep it at the bottom at all times, but you can achieve this a different way to avoid other issues like the footer overlapping your content on smaller screens such as with displaying flex and using the flex-grow property, etc. I challenge you to look into that if you are interested.
- I see you have set widths for certain elements as well. While this may be necessary in special cases, I would recommend not doing this where you have done it and let widths be a natural 100% and use padding and margins to achieve any space you may need. Using exact heights and widths are not inherently responsive, so using units like percentages, flex, and grid will allow your content to naturally grow in height and width as needed with respect to window size/device size.
In direct response to your question:
In summary, I would suggest ONLY setting exact heights or widths if it is absolutely necessary for something specific, for example, an icon or image that does not need to take up a full width or height, but overall large areas of content you can likely let naturally fill the page with restricting it to any specific height or width unless it is a percentage like 100% or if you have a two column flexbox, you may set each to 50%, etc. Of course, these are my personal opinions, but at all costs, I let my page content flow naturally until I have an absolute need to specifically size something (again, such as an icon or image, etc.). This makes for a more responsive page. If you start setting exact heights and widths for elements all over your page, when you resize your window or look at it on a small device, you are going to have overflow problems and it will become a mess.
I hope that helps! Great job on this project! I look forward to seeing more of your solutions and maybe updates to this one if you wish to make them!
1 - @NicorabiSubmitted over 1 year ago@avinnoPosted over 1 year ago
Hello! Nice job on this challenge! Looks great.
A suggestion I have:
- (My project encompassed the same thing before I updated my solution thanks to someone else's comment to me) On your project, if I click "SUBMIT" without making a selection, it still shows the thank you message without a number out of 5. I challenge you to not allow anything to happen unless the user makes an actual selection when clicking submit.
Great work! I look forward to seeing your future solutions!
Marked as helpful1 - @EverezzeSubmitted over 1 year ago@avinnoPosted over 1 year ago
Hey, nice job! I think it is interesting to try things like this without the quick and modern go-to approach such as grid or flexbox.
I remember when I first started learning HTML/CSS back in 2002, flexbox and grid did not exist yet. We used floats and other ways to establish layouts. We had to. So it is cool to see someone try a challenge without grid and flexbox.
I am curious and wanted to ask what your reason was for doing it without grid or flexbox? If there was a reason in particular, or maybe you just wanted to do it to do it.
Also, I would challenge you to vertically center your article(s)! While it's not necessary, that was often a somewhat tricky thing to do before grid and flexbox existed, for me personally anyways. At least it was not always a straight forward thing to tackle depending on your layout, etc. With grid and flexbox, it is pretty automatic on how to center vertically and horizontally. But maybe you purposely just did not center it. Regardless, I like it!
Thank you and nice job!
Marked as helpful1 - @Kaygp27Submitted over 1 year ago@avinnoPosted over 1 year ago
Great job on this challenge!
I don't have any major suggestions.
- If I am being critical though, you could do without your div.container and just utilize the rest as is. The div.container is unnecessary. I used dev tools and deleted your div.container and it all sits the same.
- For accessibility and semantic reasons, I would change your div.grid to a main tag with a class name of grid instead.
- Also, for accessibility, I would include an alt value to your left-side image. I see you included alt but without a value.
- Lastly, I would add these CSS properties to your button.cta so that your icon and button text are vertically aligned centered:
- display: flex;
- flex-direction: row;
- justify-content: center;
Other than those minor suggestions (my own opinions of course), I don't think I can pick it apart any further right now. You did a beautiful job on this one! I look forward to your future project solutions!
Very Respectfully, Aaron V.
Marked as helpful2 - @rahimi-1997Submitted over 1 year ago@avinnoPosted over 1 year ago
Hey! I love your solution to this challenge! Great job!
One suggestion:
- When you click a rating and submit and then hit "Back to Rating" and hit submit again without selecting a rating, it gives you the same rating you first chose. I would make it reset completely so when you click BAC TO RATING, it requires you to click a new rating again before it allows you to hit submit again. OR, if you leave it, I would keep their original rating selected/highlighted so they know it is still on their original selection. I hope that makes sense! Love it!
3 - @llKryptonixllSubmitted over 1 year ago@avinnoPosted over 1 year ago
Hey! Great job on this challenge project!
I had the exact situation with the popup menu. I found myself needs to make a second button as well. I could restructure it to line up right and just set the z-index so the original button shows on top when the popup shows, but like you said, in the end I felt it was not worth it to redo things just for that and instead add a second button to show/hide with the mobile menu. I know that sounds lazy and not a great option or thing to say, but I do know I COULD have done it if needed, but I was able to accomplish the challenge requirements overall. You pick and choose your battles and how to tackle them. With this one, I did it up similar to your comments.
Thanks for making me feel less alone on this one!
Marked as helpful0
