@blurridgeSubmitted over 2 years ago
Latest solutions
React ContextAPI, mobile-first workflow, Flexbox, CSS Grid, module.css
#accessibility#animation#react-routerSubmitted about 2 months agoAll constructive feedbacks are always welcome
React, Mobile-first approach, Flexbox
#reactSubmitted 2 months agoI am looking for a specific feedback on my way of handling layout at different screen sizes.
- Is it common way of handling responsive design, and conditionally rendering components as I did. Is there a better way, and which is it, using best practices?
- I think there might be a small bug on ImageGallery on first render of the component, since it looks like large image is being shown right away, and then there is that flashing effect, which should only take place upon changing the image, it also reffers to Lightbox component since ImageGallery is a part of the Lightbox as well. I am wondering how do I remove this 'buggy' behaviour?
Mortgage repayment calculator -Vanilla JavaScript, Flexbox, Sass
Submitted 8 months agoAny feedback is widely welcome. I am always open to critics/suggestions for improvements etc.
Responsive multi-step form validation HTML/CSS/Vanilla JS
#accessibilitySubmitted 9 months agoThe most useful feedback for me at this time for this project would be as follows:
- How good is my JavaScript code when it comes to best practices?
- Accessibility segment of my code?
- How good is this project written in general, when it comes to all - HTML, CSS and JavaScript?
- Which area(s) I can improve the most?
- Have you also learned anything from my code throughout your review(s)? If you have - please tell me what specifically, I would highly appreciate it
Latest comments
- @Zdravko93Posted over 2 years ago
Hello @Zach !!! Congragulations on your efforts, and you did a good job on this project! Looks very much like the original design. Also, after reviewing your code I have noticed:
-
you have a little bit of a redundant code(specifically in the media query section in my opinion) like .container {...} , button {...} etc
-
in the .container code where you put .container { position: absolute, top.... } you could instead use a more 'modern' solution like using its parent container(In this case <body> element) and put on it something like body { display: flex; justify-content: center; align-items: center} to center a .container div
OR you could also use:
.container { margin: 0 auto; max-width: ...(whichever you want it to be max)}, -
You could put 'font-family....' on body element and ofc if needed, also on button separately, instead of putting it on p, h1, h2 etc, because this way you will be more general with it
-
Also, with selectors that have the same values , in your case h1, h2 you could wrap them like: h1, h2{ font-weight: 900; }
All in all, these are not some 'huge' mistakes you made or anything like that, it's just a suggestion for slight improvement and making a code a little bit more cleaner and easier to read in my personal opinion. And again, you did a good job on this one @Zach! Wish you all the best, and keep coding!!!
0 -
- @NitinSaini007Submitted about 3 years ago@Zdravko93Posted about 3 years ago
Hello Nitin! Congragulations on finishing this challenge. Looks good. Also, try changing the color of the paragraph in the center of the card to more suiting one, closer to the design. And again, good job! Keep coding!
Marked as helpful0 - @carloseag1609Submitted over 3 years ago@Zdravko93Posted over 3 years ago
CARLOSAG1609 This looks pretty much the same as the original! Space between the icons and headings are little bit lower, but that does not really matter, you can change it quickly.I think you did really well, as your css code is very nicely written!! I like how you kept it very clean and neat, and it is obvious that you put some nice effort into building this project.Keep it going CARLOSAG1609 ! :)
Marked as helpful0
