Christian Prasetya
@christian-prasetyaAll comments
- @mahidulkabirSubmitted 3 months ago@christian-prasetyaPosted 2 months ago
Nice job, mate, but if I can fix a little bit.
Don't use div as the container, semantically the card is the main element, so maybe better use
<main class="container">
rather than<div class="container">
as the<div>
purpose is solely for styling only.For me is just that, thanks, cheers mate.
0 - @VitaliySaburdoSubmitted 4 months ago@christian-prasetyaPosted 4 months ago
It's very nice and almost perfect with the solution.
Marked as helpful0 - @jamilplSubmitted about 2 years ago@christian-prasetyaPosted about 2 years ago
Well done finishing this challenge, even though there are some area of improvement can be made.
-
Use the semantic HTML Instead of using
<div class="container">
to wrap the whole content, supposed you can use<main class="container">
. Because with the right and good semantic HTML, you can fix the accessibility of your website, especially for the user with visual impairment, as the semantic HTML will guide them to enjoy the content of your website. -
In your CSS file, try consistently use 1 unit measurement, like rem, or em instead of pixels. The rem and em will help you a lot when it comes to responsive design. So, in my opinion, you should stick with rem or em units.
Overall it's nice and readable code, and also well-structured (apart from the div container wrapper), I see you trying to implement BEM, but try to look into about it further, you can learn more about BEM here.
Can't wait to see another solution submission from you, cheers mate.
Marked as helpful0 -
- @bajra03Submitted about 2 years ago
Just done this challenge, and open for any feedback. Thank you.
@christian-prasetyaPosted about 2 years agoCongratulations @bajra03 for finishing this project wonderfully.
Your code is well structured, semantically correct, and looking good responsively.
Just little bit area of improvement
- Try to apply the measurement using relative units like rem & em, it will help you a lot with the responsive design.
- Try to implement BEM method (Block, Element, Modifier) to create more structured and reusable css code, it will help you a lot in the future.
Once again, congratulations. Matur suksma.
1 - @okida-rafaelSubmitted about 2 years ago@christian-prasetyaPosted about 2 years ago
Congratulations, mate, for finishing this first project. The result is very good also. You've done a great job.
Your code is well structured, and semantically good. But I see you do not implement any media queries for the responsive design. Please kindly try to add media queries in the future, it will help you a lot in the future project you'll create. And also try to implement more external css, it also will help you a lot in the future.
Cheers, mate.
Marked as helpful1 - @zakariabelassriSubmitted about 2 years ago
Feedback would be welcome.
@christian-prasetyaPosted about 2 years agoCongratulations on finishing this challenge. You structure your HTML & CSS code semantically great and well structured.
But of course there's some area of improvement:
- Try to apply responsive design using media queries, I know it's hard to grasp at the first time, but it's worth it for developing your skill in the future.
- Try to use BEM method for creating a reusable css code that will make your life easier. You can check it here.
- Maybe try to learn about Sass/Scss pre-processor for creating more well structured and modern CSS, it really helps a lot. I just started to implement the using of Sass/Scss and really help me to structured my stylingsheet, and creating lots of functions on CSS. Check it here
But overall, I think you did a great job developing this challenge. Hope to see more finish challenge from you.
Marked as helpful0 - @Mourad3rdSubmitted about 2 years ago@christian-prasetyaPosted about 2 years ago
Congratulations for finishing this challenge.
Your result is totally fine, but there's some area of improvement
- Don't forget the semantic HTML
Instead of using
<div class="container">
in the opening content tag, it's better to use<main class="container">
as it's semantically correct, and will help the reader with visual impairment to browse the main content of your website.-
And because this project require us to make a responsive website, it's better if you start using relative unit especially rem and em.
-
And for more best practice, I think you better link all the google fonts and fontawesome cdn through link in the HTML head rather than importing through CSS (personal opinion though).
Overall, nice work, and can't wait for your next project submission.
Marked as helpful0 - @avinashdvSubmitted about 2 years ago
Had a lot of fun & a little bit frustration for achieving the background pixelated shadow. I've tried various combinations and finally settled down to one. Tried the background-blend property in hover state with multiple bg-imgs and bg-color. Overall this project helped me learn something new.
@christian-prasetyaPosted about 2 years agoCongratulations for your solution, it's very nice.
- I see you try to use BEM Method for the CSS styling, which is great, but I think there some area of improvement.
<div class="nftCard__author"> <img class="nftCard__author__avatar" src="./assets/images//image-avatar.png" alt="author image" /> <p class="nftCard__author__about"> Creation of <span class="nftCard__author__name">Jules Wyvern</span> </p> </div>
I think it will be better and will be more easy if you write like this:
<div class="nftCard__author"> <img class="nftCard__author-avatar" src="./assets/images//image-avatar.png" alt="author image" /> <p class="nftCard__author-about"> Creation of <span class="nftCard__author-about-name">Jules Wyvern</span> </p> </div>
especially when you use scss, it will make your life easier, so basically you can use nesting selector from Scss like this:
.nftCard__author { &-avatar {} &-about {} }
which will make your life easier when writing the scss code.
- I appreciate you try to use Scss, but I think you can improve by folder structuring your scss partial file (it will be very helpful), and create component-like scss file.
- Try broadening your scss file with @mixins, function, and many more, to unlock the potential of your scss/sass skill. Sass - Guide
Hope this will help, and I'm sorry if my english little bit bad.
Marked as helpful0 - @ST3V3NP3RRYSubmitted over 2 years ago
This is my first project on frontend mentor I'm just starting to learn HTML and CSS, and trying to get a handle on github and git, so any help or recommendations with code formatting and best practices would be most welcome. Sorry if that's too much of a generalisation.
I've seen some other solutions where developers have used custom CSS properties to provide variables for colours etc is this a best practice for me to consider moving forward?
All comments about how the code could be improved would be greatly received.
Thanks
Steven
@christian-prasetyaPosted over 2 years agoHi Steven, great job, your solution is almost perfect with the design.
You've done a great job with the HTML semantic and accessibility, and your CSS is really great. And you perfectly measure the width and the heigth of the rectangle.
As for the custom CSS variables, Yes, it can help make your life easier in the long run, so you don't have to remember every color you want to use, or the font and font size for your heading, paragraph, etc. But, it came back with the personal preference. For me personally, I use the CSS variable in all my project, so I can focus more on the other solution without worrying what color I've used, what the hex or rgb code, etc.
Once again, well done, and hope to see more challenge solved.
Marked as helpful0 - @COSSART-FRSubmitted over 2 years ago@christian-prasetyaPosted over 2 years ago
Hello if I may helping.
First of all, cheers of finishing this challenge.
But I think you should change the
<div class="card">
to<main class="card">
and adding a alt to your<img>
to help the issue with your code submission. Believe me, I did the same mistake before, and secondly, it will help you in the future to fix the semantic and accesibility of your website in the future.Hope this will help.
Cheers.
Marked as helpful0 - @Feedy99Submitted over 2 years ago
while creating the structure I found it difficult to create a rectangle and bring rectangle in a center. II am unsure about the width and height of the rectangle.
Well i tried my best as a beginner. Overall i had fun working on this project and making a QR CODE. THANK YOU!
@christian-prasetyaPosted over 2 years agoDear Sahir, well done to finish this challenge. Eventhough the code is not basically correct, but it does the job.
Area of improvement:
- Don't forget about the
<!DOCTYPE html>
in the first line of your HTML page (before the html tags). It's the very basic tag to tell the browser the HTML version. That's will help you with the HTML issues. - Semantically, don't use the
<div class="rectangle">
you should use<main class="rectangle">
for semantic reason, and<footer>
tag for the attribution section. That will help you with the accessibility issues. - For the CSS, you code beautifully, very clean, and specificity is correct.
- For creating the rectangle, basically you can add width and height in your "container" (in your
<div class="rectangle">
html code), and set the width and height for the rectangle you want to create, as the content container. - For centering the rectangle, remember that
<main>
or<div>
is basically a block element, so you just can center it usingmargin: (value for x-axis)px auto;
, where the auto will automatically center the content in the page. - Tips for measuring the height, width, margin, and padding, you can use the graphic software program like illustrator, photoshop, adobe xd, or figma for the jpg file, and measure it with line tools. It's better though using the pro version, but myself using the free version, and I do that to measure the value I need.
Hopefully, it can help you in the future. Keep it up, cheers.
0 - Don't forget about the
- @luamunizcSubmitted over 2 years ago@christian-prasetyaPosted over 2 years ago
Nice job, mate.
The code is well structured and semantically great.
Area for improvement, maybe can be more detailed with the margin auto centering, cause in the preview site is little bit off center. As for the CSS margin for the card div, maybe you can join the margin: 0 auto; with margin-top: 100px; -> margin: 100px auto; surely will be better for centering the card in the middle of the page.
Once again, nice job, and let's grow together.
Marked as helpful1