Looking to hire developers?
@sahand-masoleh
@gbabohernest
Submitted
Hi there 👋
-Anyway, here is my solution...kindly check it out and give me your feedback on best practices and how to improve on it.
Thanks and happy coding ✌✌
@sahand-masoleh
Posted
Hi Ernest,
Regarding your problem, I don't see any :hover selector on your hamburger button, if you have been trying to no avail, it's because your SVG has a fill: none set in it, that takes precedence over everything you write in CSS.
@YelemyahM
Submitted
Hello World !
How can we change color on hover of an svg element ?
@sahand-masoleh
Posted
Hello!
You most likely need to edit your SVG, as any styling defined in there has the highest specificity.
If you want to be able to control the color through your CSS or JS file, a common tactic is to set the fill or stroke in your SVG to currentColor so that it will follow the container.
Also, if you're using an SVG inliner with Webpack for example, you can directly alter fill and stroke provided that they're not defined inside the SVG file.
@JerryWski
Submitted
Hi, I managed to finish this project, but the colour change button was really a big challenge for me. I used the internet and youtube to a large extent. In this project I finally convinced myself to create separate components for my site. Unfortunately, using the sass compiler I had problems firing them up, but I promised myself that I would install Gulp.
I hope you guys like my version. If you have constructive comments feel free to write , good luck !
@sahand-masoleh
Posted
Hey Jerry! Good job!
First things first, you have set a min-height: 150vh on your page, I'm guessing you meant to do 100vh to be able to center the content on the page. But even then, that's the wrong approach, vh units include the address bar on mobile browsers and it will push everything down. Use height: 100% on html and body elements and then you can use percentage units to maximize the main container with min-height: 100%
Regarding the theme toggle, I think a checkbox would've been a better choice than two radio buttons, both for you and for the user. It's a tad bit hard to click on right now.
Marked as helpful
@redpangilinan
Submitted
This is my first solution in this website and I have learned a lot during the development! I am satisfied with how the project turned out although I am aware of my limitations.
The code could definitely use some work and some sections are inefficient so that's what I'm going to ask about.
I would appreciate some opinion that could make the code cleaner, shorter, and more efficient.
@sahand-masoleh
Posted
Hey Red Pangilinan, nice job and welcome!
First things first, your solution looks really good, exactly what the design wanted.
Your footer does look a bit over-engineered, but nothing crazy. What you can do is after you're done, open up the console and turn CSS properties on and off, if it doesn't make a difference it doesn't need to be there.
For example, the container for your social icons has a margin bottom, even though the content are at the top anyways, you can see that turning off that property doesn't change anything.
But don't worry about long HTML, that's the nature of a markup language. There may be some room for optimization, but you'll get there with time.
Edit: By the way, if you want to have an easier time with naming your classes, you may want to look into BEM, or other conventions like it.
Marked as helpful
@NJVS
Submitted
My English is bad. Sorry in advance.
A very simple and straight-forward landing page, nothing much to say but there's something that piqued my interest, The navigation bars inside the footer are arranged from top to bottom, then wrap to the left, instead of left to right, then wrap down.
My initial solution for this is to use flex -> column -> wrap, but to automatically wrap the elements, the the height of the parent element must be set. Tho this works, I still need to set a height or max-height on the parent element for each breakpoints.
But what I want is to auto-wrap the elements like in Flex without setting the height of the parent element. I know that Flex columns work fine, but I just want the height of the footer to be auto-adjusted.
I just want to ask the community if there's another way to do this.
As always, feel free to comment any suggestion/tips. Thaaanks!
@sahand-masoleh
Posted
Hi lieneil! Really good solution!
About your question, you can also use a grid with grid-auto-flow: column and then setting your grid-template-columns to repeat with auto-fit.
Also, don't use height: 100vh on your pages because the moving address bar of mobile browsers is included in it and it will push everything down. Use height: 100% on the "parents" of the element that you want to cover the page, and then min-height: 100% on that element.
Nevertheless, you don't even need that for this page, since it doesn't have a vertically centered element.
One more thing, you don't need to set height: auto on elements, it is by default set to auto.
Marked as helpful
@RemoRemo11
Submitted
I couldn't change the color of the social icons on hover to match the design, so how to make that hover effect with the social icons??
I use the main element to wrap up the main content of the page (except the header and the footer) is that the right use of the main element. Thanks.
@sahand-masoleh
Posted
Hello again Remoan! How are you doing?
It's a challenging layout for sure, that monitor sticking out needs some creativity.
You can't change the color of the SVG because it has a color hard-coded into its code and that overrides anything you define in CSS. Change the fill property to currentColor in your SVG and set the color with CSS with the color property.
Your use of the main component is correct, but like I said about your other project, I would give it a max-width and center it, right now it grows forever and throws your cont element out of center.
Marked as helpful
@KeerthiVasan-N
Submitted
Any Tips On how to align footer section [location tags -icon] and content properly centered might be helpful
@sahand-masoleh
Posted
Hi Keerthi,
You can wrap the content of your footer in a wrapper div and center that coupled with a max-width property.
I would do that for the rest of the page too, because currently your page breaks on ultrawide monitors, it just keeps growing.
Marked as helpful
@mhanak96
Submitted
Trobules that I had:
Overall I thought that this challenge will be much easier - but i learned a lot so I'm happy 😅
@sahand-masoleh
Posted
Dzien dobry Mateusz! Great job!
You can toggle a class on click to change the color, or anything else, and if you give it a transition property, you can animate it too.
You can use a media query and change the size of the container based on view port size.
Same thing, you can toggle classes. Just make sure that your SVG doesn't have colors hard-coded in it or it will override anything you define in CSS. A common technique is to set the fill property of the SVG to currentColor and set the color in CSS.
You can use the Intl.NumberFormat object in JS to format currency and many other things cleanly and natively.
There's nothing wrong with using border, but you can also use the <hr> element, a pseudo element, or even a decorative div, whichever you're more comfortable with.
Marked as helpful
@Tuason066
Submitted
Hi, 👋
I have a question about the GitHub pages. So this solution is built with react and react-router, I am using gh-pages to live my solution but the output(live website) doesn't work correctly because there is an unexpected bug(the images don't show). That's why I used Netlify to live my solution(website).
Thank you so much for helping me out. Happy Coding. 🥳🔥✨
@sahand-masoleh
Posted
Hi Jeffrey, great job!
You have deactivated Github pages for this repo so I can't see what was wrong. But you typically have to set a 'homepage' property in your package.json, otherwise your relative links don't point to the right address.
Usually when everything works but the images, it's a path problem.
@HarmoniaCodes
Submitted
I learned a LOT while completing this challenge! A few things that I could not figure out: How can I load content from the JSON file in a way that javascript can parse? How can I change the styling of the active tabs and dots once the content has been changed via javascript? How can I add animation effects to the tabs/dots content so that the loading of content is more elegant?
@sahand-masoleh
Posted
Hey Nick! Nice job!
First off, don't use in-line HTML event handlers (e.g. <button onclick="someFunction()" />), they are deprecated, see here. Use your JavaScript to listen for the click event and run the function that you want.
This also answers your last question I believe, inside that function toggle CSS classes with animations that you want for the desired elements. I hope I understood your question correctly.
As for loading data with JS, you normally use the fetch API to pull external data. But if your json is local and you're using a bundler, you can also import it like any other ES module.
Marked as helpful
@RemoRemo11
Submitted
@sahand-masoleh
Posted
Hey Remoan, nice job, you almost got it right, here are a few tips that will help you fix the problems:
I have a solution for this challenge, you can take a look at it.
Marked as helpful
@Ivar112
Submitted
My first challenge delving a little deeper into JavaScript. I feel like my JS skills are lagging behind quite a bit compared to my HTML and CSS, so I am especially curious to find out what you think of the JS portion. Anything I could have simplified or taken a different/better approach to altogether?
@sahand-masoleh
Posted
Hi Ivar! Nice job!
Here are a few things I would fix:
@JaneMoroz
Submitted
👋 Hi, everyone!
Here is my solution to the E-commerce product page 🎉
I wanted to practice using Mobx, so the only problem I encountered was the "store" structure. I guess I'll try to refactor it later and create several instead of one 🤔.
So to sum up I used:
🤝 Happy to hear any feedback and advice.
@sahand-masoleh
Posted
Hey there!
I just completed the same challenge! Your solution is very nice, I wasn't expecting strong competition so soon. :D
Here's a few things I noticed:
outline instead of border to avoid shifting the element, if that's unintended.z-index.Marked as helpful
@Ayo-funmi
Submitted
I am still struggling to grasp the full idea of grid layout and any idea on how to make my work better would be highly appreciated. Thank you
@sahand-masoleh
Posted
Hi there!
I don't see any problems in your implementation of a grid layout, is there anything specific that you think needs improvement?
@pqhung3007
Submitted
This is the first time I have worked with NextJS. The challenge itself is also by far the most creative one I've ever dealt with in Frontend Mentor. Quite satisfied but there's a problem I'm confusing 🤔:
Apart from that issue, I'd appreciate other feedback relating to the code comprehensiveness (with Tailwind) or the performance. Thanks and happy coding 🤲💻
@sahand-masoleh
Posted
Hi there!
Images always take time to load, or they may even fail to load, so you need to have a place holder to avoid layout shifts. Put your images in a container and give it a specific size and a neutral color.
Also, you can set a dark color on your body element to avoid the white flashes when switching pages for the first time.
Marked as helpful
@Haidrex
Submitted
Hello,
Tried out SASS with this project.
I feel alright about it, but I did mess up mobile design a bit, still need to practice.
Any feedback welcome :)
@sahand-masoleh
Posted
Hi there!
Your <p/> elements in your testimonials have a fixed width of 38ch, that's why they don't wrap properly.
I suggest giving your menu an absolute position instead of fixed; there's no need for it to move down with scrolling and there's no way of closing it without scrolling back up.
Marked as helpful
@AndreaDev14
Submitted
Hello, everyone! I just finished this challenge. It was fun and I learned a lot. Your feedback would be appreciated
@sahand-masoleh
Posted
Hi there! Congratulations, very nice solution.
Just one tip, disable scrolling when the menu is open, or give the menu a position of absolute instead of fixed, because now I can open the menu and scroll down, with no way of closing it other than scrolling back up.
Marked as helpful
@xingxing-prog
Submitted
I am excited to finish this project by using object, Well, Object is a different concept compared to other programming language. It's very convenient and powerful, I am just dabbing it now. Forgive me, if the responsive part is kind of broken. I am still headache about responsive, which is so different from coding that if it's correct then it will work as you expect, there are so many miles between Ok to good for responsive and other CSS. Welcome any feedbacks. it's a great appreciation to gain any advice about improve CSS and responsive skills.
@sahand-masoleh
Posted
Hi there! Here's what I think:
Don't use 100vh, it will cause scrolling on mobile browsers because of the address bar.
Remove max-width from your body, because you need the body to stretch to be able to center your content. Set your max-width on your <main /> element and give it a margin of auto to have it centered all the time.
You've lost responsiveness because your text element is absolutely positioned and thus removed from the flow of the page. It doesn't care about the other elements. You can control how much your two main elements occupy of the parent flex box using flex-basis and width, preferably with a percentage unit.
Marked as helpful
@Oluwatosin0
Submitted
I just finished working on this project, but I've been having with my js code, it's not running.. I've checked every necessary error, but still can't figure it out. please kindly help out if you can. thanks
@sahand-masoleh
Posted
Hi there!
In your HTML you are trying to read script.js, while the name of your JS file is calculator.js.
@justina-tautvydaite
Submitted
Feedback welcome
@sahand-masoleh
Posted
Hey there!
Great job! I did this challenge recently and your solution is pretty much identical to mine.
Just one thing, you can instruct Netlify to redirect using a _redirects file with /* /index.html 200 as its content in your root folder, so that for example https://space-tourism-info-site.netlify.app/destination/europa won't be a dead link.
Marked as helpful
@Andy-COD
Submitted
Created my first ever react application 😊 🎉. It was a fun process, but I am bothered by the fact that when the site is loaded initially, on route change, the page turns white briefly before showing page content and I was wondering if that's normal. Since this is my first react project, I would appreciate any feedback that could help me improve.
@sahand-masoleh
Posted
Hey mate!
The page turns white because your body element is white. When everything unmounts that's what you're left with.
An easy fix is to set the background color of your body element to something close to all your pages.
Marked as helpful
@yonext24
Submitted
This was the perfect project to do as a first full stack page, i added log in to make comments ^^
1: I dont know why frontend mentor took the screenshot fully gray :( (i already tried generating new ones)
2: I know i should change the <div class='main-container'> to <main> but that would mean change all the styling and im lazy
@sahand-masoleh
Posted
Hey there!
I would love to check it out, but I believe your back end server is down. It says it cannot load comments.
Marked as helpful
@Stephen-Adom
Submitted
@sahand-masoleh
Posted
Hey mate!
Good job!
I suggest putting in some mock comments to show off the design and functionality.
In my opinion users should be able to see the comments without logging in.
Marked as helpful
This project needed a bit more time to complete yet I've learnt a lot while doing it.
I tried changing navigation menu's style for mobile and hide it until hamburger menu button being clicked. I thought this is a better solution than creating whole new menu but I'd like to hear few feedback about this.
@sahand-masoleh
Posted
Merhaba Birkan bey!
There's nothing wrong with creating a new element for the mobile menu, do it however you are comfortable as long as accessibility practices are exercised. For example right now your hamburger button is actually a picture element with a click event listener, and that's not good for accessibility. Try to use a button tag instead which is selectable and clickable natively.
Also, if open, your mobile menu doesn't close when the view expands beyond your break-point. This means that if a user opens the menu and then turns the phone into landscape mode, they will still see the mobile menu with no way of closing it.
Marked as helpful
