Duy Anh Le
@leanhduyAll comments
- @Kane2881Submitted 6 months ago@leanhduyPosted 6 months ago
Tested on several browsers and the solution look the same on every each of them. Nice solution!
0 - @chrisbabu9Submitted 6 months agoWhat are you most proud of, and what would you do differently next time?
learned from best. adopted new techniques
What challenges did you encounter, and how did you overcome them?so much frustrating stuff.
What specific areas of your project would you like help with?idk
@leanhduyPosted 6 months agoGreat work.
I learned a few useful technique by looking at your styling.
1 - @EduTavaressSubmitted 6 months ago@leanhduyPosted 6 months ago
Good work!
Through my review of your code, I have one following suggestion:
-
Consider creating variables to hold shared values in the css (e.g., colors), instead of hardcoding in each element
color
orbackground-color
- Create variables in the
html
selector: e.g.,--text-main: #ff0000;
- Use the variable. e.g.,
h1 { color: var(--text-main); }
Why: Better code maintability. If there is a requirement change about the color, you will only need to update the value of variable in the
html
selector, instead of finding all elements with same color and update it. - Create variables in the
Marked as helpful1 -
- @TejaswiniBelesheSubmitted 6 months ago@leanhduyPosted 6 months ago
Great work!
The only comment I have is to follow the CSS best practice and implement styles in an external file and use it via <link>, rather than inline styling using style element or style attribute.
You can read more here: https://htmlandcssguidebook.com/css/css-intro/#css-files
Marked as helpful0 - @AleandroPrestaSubmitted 6 months ago@leanhduyPosted 6 months ago
Great work. However, it would be improved by adding media queries to apply the style differences between mobile screen and laptop screen. Currently the solution only for desktop screen.
Marked as helpful0