Hi to everyone! This is my first junior proyect, I would like some of your comments to improve the code! Thank you! Eli
Doston Nabotov
@dostonnabotovAll comments
- @ElisaFossemaleSubmitted over 1 year ago@dostonnabotovPosted over 1 year ago
Hey, there! 👋
Congrats on your first junior challenge solution. Here's some suggestions I would like to point out:
- The cards didn't load on the page. I think it has to do with your JavaScript fetching code.
- Opt for better naming convention for your classes. Since
div-small
,div-violet
doesn't carry too much meaning, go with something likecard
,tracking-item
and etc. - Also, I would avoid spamming
<br>
elements to give space between other elements. You can read more about its purpose on MDN Docs - CSS custom properties woud really help optimize your CSS code.
- As the last suggestion, I would recommend reading and following the Frontend Mentor guidelines. Create a
README.md
file and explain what, how and why you did. Good for you and other fellow programmers.
Hope it helps! Good luck on your coding journey!
Marked as helpful0 - @Dimoon2134Submitted over 1 year ago
Hello this is my first project here I tried using css naming conventions and HTML5 semantics I would also like to know how i could optimize my .css file Thanks for your feedback
@dostonnabotovPosted over 1 year agoHey, there!
Congrats on completing the challenge.
Your solution looks great, and I don't have much suggestions on your code.
As of your question, visit CSS guidelines to learn more about CSS and how to write better.
Good luck on your coding journey!
0 - @WesselKonstantinovSubmitted over 1 year ago
When I did the QR code component challenge, I didn't have any trouble setting the dimensions of the component. However, this time it was a bit more challenging. I started out with an explicit width (21.875rem). Then, I switched to the vw unit for the width along with a max-width of 21.875rem. I did this as a workaround in order to make sure the component is responsive on small screens, but I feel that this is not an optimal solution. What would be a better way to get the dimensions of the entire component right without having to resort to vw units?
I would love to hear your feedback and suggestions.
@dostonnabotovPosted over 1 year agoHey, there!
Congrats on completing the challenge.
I really liked your approach on semantic markup and good CSS code.
Here are some suggestions I would like to make:
- As of your question, do not heavily rely on viewport units. Because if the user tries to zoom in or zoom out, it really gets out of control. Instead try switching it something like this:
.profile { width: calc(100% - 2rem); max-width: 21.875rem; margin-inline: auto; }
Or if you want one line:
.profile { width: min(100% - 2rem, 21.875rem); margin-inline: auto; }
100% - 2rem
indicates that it can stretch to 100% full width, but with 1rem extra spacing on each side. I usedmargin-inline: auto
just in case to horizontally center the element.- Speaking of fixed height on your image, I liked the descriptive comment explanation. But, here's the better way you might wanna consider:
.profile__decoration { background-image: url("images/bg-pattern-card.svg"); width: 100%; aspect-ratio: 16 / 9; }
Basically, you rely on your dynamically changing width to determine the height of your image. Find out what ratio it uses and apply it.
Good luck on your coding journey!
Marked as helpful1 - @LiamPerrymanSubmitted over 1 year ago
Please feel free to give me some tips.
@dostonnabotovPosted over 1 year agoHey, there!
Congrats on completing the challenge.
Here are some suggestions I would like to make:
- Move your styles to separate
style.css
file. - Use Prettier. I see that you don't care about formatting and how your code looks. So, let Prettier (VS Code extension) handle it for you
- Add some spacing on top and bottom of your page for small devices. Something like,
padding-block: 2rem;
onbody
element should do it. - Attribution is overflowing on top of other elements on small devices. Either remove it or style it better.
- Read the
README.md
andREADME-template.md
files if you want other developers check and read your code.
Good luck on your coding journey!
Marked as helpful0 - Move your styles to separate
- @basitkoraiSubmitted over 1 year ago
Hello, My Fellow Developers, Hope you're nailing it in your coding journeys. This is my Initial Solution to the Interactive Rating Component
I learned to use setTimout() Method to Run a block of code responsive built it with mobile-first workflow.
Had tons of fun and hard time while coding this project! I have tried to make it as good and accurate as possible, but yet I know there are some accessibility, and responsiveness issues in it. Which I will try to fix in the future.
So Feel free to leave any feedback and help me to improve my solution or make the code better in any way possible.
I'll be looking forward to hear lots of feedback from you guys. ✌️
@dostonnabotovPosted over 1 year agoHey, there!
Congrats on completing the challenge.
Here are some suggestions I would like to make:
HTML:
- Try using radio checkboxes
<input type="radio">
instead of regular buttons. Because you only have to select one rating, it will make your life much easier. Also, less JavaScript code.
CSS:
- I think you're missing
*
selector in your first few lines in your code. - I wouldn't recommend setting
transition
on every element. Use transition property where you need it.
*, *::after, *::before { box-sizing: border-box; }
JavaScript:
- If you've switched to radio checkboxes by this time, you can safely remove your code where you toggle the state of buttons.
Other than these, everything looks good to me.
Good luck on your coding journey!
Marked as helpful1 - Try using radio checkboxes
- @ify47Submitted over 1 year ago
I am desperately in need of your feedback, how did i do?
@dostonnabotovPosted over 1 year agoHi, there! 👋
Congrats on completing the challenge!
Really liked your solution. But, here are some suggestions I have:
- I found a bug with your mobile navigation toggle. When using it on my phone, I noticed that icon and navigation state is switched. In other words, when I open nav menu, icon turns into "menu" instead of "x".
Possible solution: I think it's causing by your JavaScript code, where you've specified the
isIconEnabled
totrue
. I think, by default it should be set tofalse
, because nav menu will be closed when you first load the page.- (In my opinion) You have way too many media queries in your CSS. 3 and 4 queries are way too much to handle as if you are trying to micro managing your website.
Possible solution: If you're having issues with big spacing on larger devices and small spacing on small devices, try better learning the
clamp()
. See how you unleash the power, by not just applying to font sizes, but also in your spacing and other several places.Good luck on your coding journey.
Marked as helpful1 - @Lord-AnarakSubmitted over 1 year ago
This time I've used react-redux for global state management to trigger dark mode in tailwind CSS and created and API using redux-toolkit. I went further and did some of my own logic like when searching instantly the table updates and after applying filter search box will search inside the filtered countries like so. I made it super responsive by using combination of flex and grid. Any suggestions to improve my logic in the home component mostly welcome. #ThankYou
@dostonnabotovPosted over 1 year agoHi, there! 👋
Congrats on completing the challenge!
I really liked your solution. However, here are some suggestions I would like to make:
- It would be great if you could use the logo, "Where in the world", as a link back to homepage.
- When I first visited, my first impression was on loading and attribution. Website looks great. But, updating and aligning the loading part would make it even better.
Good luck on your coding journey.
0 - @Yousef0102Submitted over 1 year ago
Please Tell me Your Suggestions to improve my level
@dostonnabotovPosted over 1 year agoHi, there!
Congrats on completing the challenge.
Here are some suggestions I would like to make:
- If you want to write a robust and better maintainable code, focus on naming things.
Instead of this:
<button class="send">Send</button> <button class="cancel">Cancel</button>
Try switching it to this:
<button class="button" data-type="primary">Send</button> <button class="button" data-type="ghost">Cancel</button>
Even if your project scales (in this case, it doesn't), you don't end writing the same code over and over again.
- Don't nest too much.
main .container .plans .text p {}
That's way too much nesting. Even if you're writing your code in Sass or Less (with less care about the output), your CSS ends up looking like a crap. Also, it can have impact on CSS specificity.
Expect these problems, everything looks good to me.
Good luck on your coding journey!
Marked as helpful1 - @av-guySubmitted over 1 year ago
This is a challenge project from Front End Mentor. I did not strictly follow the design requirements for the project, but nothing critical should be missing. I included some additional accessibility features, such as aria-labels, and I tried to follow most accessibility guidelines. I did not have the time to turn this into a PWA, but I may revisit it in the future and add that support.
This project should work well with Edge, Firefox, and Chrome. I was able to use a trial Browserstack environment, but the time limits meant that I could not do much testing with Safari. If you happen to use Safari and are able to test this, please consider filing a bug report if you encounter any behavior that breaks the project.
I did not write unit tests for this project because it is small. In a production environment, I would have established the list of requirements and then written tests for those requirements. As this is really just a small challenge project, I did not figure it was worth the effort.
The minimum viewport size for this project is around 260px. Anything beyond that and you're pretty much on your own. I figured this would be good enough for most of the available devices out there.
A couple of things I plan on taking care of in the future is a theme for people who prefer higher contrast and also a general cleanup of the redundancy between the theme properties from the light and dark theme respectively. There are also some performance issues that have been highlighted by Lighthouse that I could definitely take care of in the future.
@dostonnabotovPosted over 1 year agoHi, there!
Congrats on completing the challenge.
Everything looks good to me. But, I have one suggestion.
I would appreciate if you could lower the opacity of the border of the
<input>
element or at least change the color to something less bright. It's standing out too much in both the dark and light theme and it would be great if it were less distracting to eyes.As for the viewport, I am not sure it's sure if it's a good practice to care too much for below 300px. 320px is my minimum go-to viewport.
Good luck on your coding journey!
1 - @aesy2Submitted over 1 year ago
I'm pretty sure the code I wrote here is pretty messy, although I wanted it to work and worry about clean code later, so I'd have 2 questions.
- What can I make cleaner here?
- What is the proper way to center a box in a middle of a whole page? I feel like doing it via flexbox is not the proper way.
@dostonnabotovPosted over 1 year agoHi, there
Congrats on completing your challenge. Here are some suggestions I would like to make:
- To make your code cleaner and robust, use CSS custom properties to avoid repetition. For example, saving your color palette in variables can help speed up your performance writing code.
- Avoid using
#id
selectors in your CSS. Theid
attributes are most commonly used for JavaScript. Stick only with classes. It will help you in the long run. - There are several ways of centering techniques. But, what I consider a good practice is with the
grid
:
.element { display: grid; place-items: center; }
If you see no effects, try setting
height
ormin-height
values.hsla(241, 72%, 46%, 0.678)
. As 0.678 for the alpha value, it seems like you are going for the pixel-perfect design. Personal preference, of course. But, I wouldn't suggest doing so.- Finally, check out Kevin Powell's video on YouTube and see how he did this challenge. Trust me, you will learn a lot from him.
Good luck on your coding journey!
Marked as helpful0 - @0xabdulkhaliqSubmitted over 1 year ago
👾 Hello, Frontend Mentor Community,
This is my solution for the Huddle Landing Page with Single Introductory Section.
- It's been the first time using
Tailwind CSS
along withyarn
as package manager 🛠️ - Used
Prettier
code formatter to ensure unified code format ⚙️ - Scored
99.125%
on Google Pagespeed Insights! 🤩 - Solution with
99.9
Percent Accuracy 🎯 - Layout was built responsive via mobile first workflow approach 📲
- Had a lots of fun and pain building this challenge ! 🥲
Now for the questions :
- Regarding, css optimization for production. I tried tailwindcss's
--minify
for css code reduction. but it ended up with removing the manual style i wrote oninput.css
- But surprisingly the
apply
directives won't get affected but manual css does - At last i used an online css minifier for production, So i want to know why
--minify
purges manual css oninput.css
file - And, This is for the first time i using
Tailwind CSS
so apologizing for to many arbitrary values to attaining so called Perfection - Finally, Feel free to leave any feedback and help me to improve my solution (or) make the code clean!
.
👨🔬 Follow me in my journey to finish all newbie challenges to explore solutions with custom features and tweaks
Ill be happy to hear any feedback and advice !
@dostonnabotovPosted over 1 year agoHey, there! 👋
Congrats on completing the challenge. I really liked your attention to details in your HTML and CSS. Also, I do have suggestions as well:
HTML:
- It's better use
aria-hidden: true
on decorative images and icons, making it unreachable for screen readers. You can read more here on MDN Docs - Also, setting an arbitrary size for images
<img width="709" height="506" >
might not be a good idea since they can cause weird overflows on small devices
CSS:
- I see that you used fancy link in your attribution section. But, the way you implemented might impact the GPU performance. In other words, it's very costly to use
width
property when animating things. Also, you might have noticed not smooth, but quite rough animation on hover. I would rather switch totransform or scale
properties.
This code:
.attribution a::before { width: 0%; } .attribution a:hover::before { width: 100%; }
can be turned into this:
.attribution a::before { transform: scaleX(0); transform-origin: < left | right >; } .attribution a:hover::before { transform: scaleX(100%); }
It might not boost your website performance, but taking care and preventing this issue can help you in the long run.
Good luck on your coding journey!
Marked as helpful1 - It's been the first time using
- @leozizzSubmitted over 1 year ago
Following the challenge, I tried to do the best I could within what I know and with a little research, but I believe I can improve in the future. Can you give me tips on how to improve the code in general?
-
I used CSS Grid, Flexbox, Media Queries and CSS Variables to make the page organized and responsive, but I'm not sure if I used them in the best way.
-
I slightly changed the color palette suggested in the style-guide to be more like the original design.
-
I also added a validation with JavaScript that is not in the challenge to check if the user chose one of the ratings before sending the submit.
I intend to refactor the code, mainly the JavaScript as soon as I get more knowledge in the language. Do you have any tips, what do you think?
@dostonnabotovPosted over 1 year agoHey, there! 👋
Congrats on completing the challenge! Clean and well-structured code! But, I've some suggestions.
Let's start with HTML. If you also want to care about the accessibility in your website, here are some suggestions:
- Instead of using the regular buttons,
<button class="rating-button">1</button>
, you could use radio inputs,<input type="radio" name="rating">
, making it easier to rate only one of them. Read more here - Wrap the rating state inside the
<form>
element. It can really help those who navigate with their keyboard.
For JavaScript part, if you had implemented radio input I mentioned earlier, you wouldn't need this piece to code:
ratingButton.forEach(button => { button.addEventListener("click", function() { /* other code */ }); });
Because you could easily check the value of radio input when the submit button is clicked.
As for CSS, everything looks good to me.
Hope it helps! Good luck on your coding journey!
Marked as helpful1 -