This was a quick one. Now that I know how to use git version control, I feel much better about contributing more projects. I'm curious how much improvement the project needs, accessibility wise.
Any constructive feedback is very welcome.
Thanks for checking this out.
@vanzasetia
Good spot on the alt text and I will come back to centering the container properly. Good idea to use flex!
I think rem is great, but I wouldn't say it's dynamic. It's a fixed value that is set in the root. I don't think em is dynamic either, it only resizes relative to the font size of it's parent. In this context I was happy with pixels.
Thanks for the comment about the width, but did you know you can refactor width: 100%; max-width: 600px; into one line?
In this project I used width: min(100%, 600px);
The first value sets the width it should be ideally at a minimum and the second value sets the maximum width.
I do appreciate your feedback, thanks.
Hello, Frontend Mentor community! This is my solution to the QR code component.
I have read all the feedback on this project and improved my code. Due to the fact that I published this project very long ago, I am no longer updating it and changing its status to Public Archive on my Github.
You are still free to download or use the code for reference in your projects but I longer update it or accept any feedback.
Thank you
Well this is inspiring. I'm going to have to revisit this one ;)
Well done for your hard work!
I'm particularly impressed by your use of variables to store the relevant colors in the :root and your clear use of names (.card .qrImg etc). However I have some suggestions for improvement.
Height is not as dynamic as width, because of this you will come to find that height isn't actually as ubiquitous a requirement as width.
Explore naming conventions. You can use something like BEM which allows for orthogonality in your design. For more information read https://en.bem.info/methodology/quick-start/ In your project however you could use BEM like this.
div.card (a block class) img.card__image (a block and element class) h1.card__heading (a block and element class) p.card__text (a block and element class)
Combine this with your very organised alphabetical structure and you'll be in business.
Lastly I'd suggest you research some different ways to use containers. Currently your .container element only seems to be centering the .card element. This is absolutely fine, but you could actually (probably) just center within the body. I recommend you check out a free course by Kevin Powell on flex and responsive web design if you haven't already (https://courses.kevinpowell.co/conquering-responsive-layouts). I learnt a lot about containers and lots of other things, as well as the flex stuff, which you already seem comfortable with.
Hope this helped
Hi Kensie,
You did a really great job here so well done. You made a really good use out of flexbox.
Some suggestions.
Your HTML could be more semantically meaningful. I accept that the main point of this is to make your design look good, but consider swapping the div#container for a section#container and maybe you could wrap the h1 tag with a header..
These little things are good practice and will help make future projects more accessible which is very important. Do a bit of a defragment of your CSS and remove any code not being used.
On classes vs IDs. I think the reason why people suggest classes over id's is because developers can leverage rules of one class across multiple elements. This can be used to make our code more succinct and follows principles of dry (do not repeat yourself) nicely.
It might be worth exploring the BEM structuring method. Even if parts of it seem counter intuitive to you. Learning and implementing a new structuring method - for say a month - is very useful and can be stated in your CV's and interviews. Employers will respect the effort you went to in order to adapt your practice.
Well done for your really excellent work and don't be disheartened by my criticism. I think you nailed the design and that's honestly the most important thing in the beginning.
