@ziko0
Submitted
Will be glad to hear your opinions
Looking to hire developers?
@RubenSmn
@ziko0
Submitted
Will be glad to hear your opinions
@RubenSmn
Posted
I see that you made a custom line instead of using the provided svg. That looks really good!
I did find some things you could improve upon:
The icon in the button is very small.
A transition effect on the shadow to create a more fluid effect.
Increase the usage of semantic elements, currently you're building a lot with div
elements. These are a great way of getting to look semantic elements the way you want.
For example your div
with the container class could be a main
and you could switch the span
and p
to a heading tag (h1
, h2
).
I hope you find this useful!
Marked as helpful
@yeldynov
Submitted
Looks like Advice Slip API
is currently down, but it was ok when I was building the project, so the code is valid.
@RubenSmn
Posted
Hi Nico, I just looked at your solution for the Advice generator app and the desktop version looks really good. But when I switch to mobile the width
of the div
is not responsive.
I see in your code you're using Tailwind CSS and have a specific desk
break point. You could add a default width
of something like 80%
to the container div.
<div
className="w-[80%] bg-c-dark-grayish-blue relative m-5 p-5 rounded-xl flex flex-col justify-center items-center gap-5 desk:w-[33%] desk:gap-8 desk:px-10"
>
...
</div>
I also used Tailwind CSS for this. If you want reference something take a look at my solution
Marked as helpful
@leonpahole
Submitted
This time I tried to use Tailwind to make the site pixel-perfect. I am loving Tailwind more and more because I feel like it makes me more productive.
@RubenSmn
Posted
Hi Leon, I just looked at your solution for this challenge. Nice job with the theme using the users preferred color scheme! Overall great UI.
I did find some things you could improve upon.
When trying to remove the whole word from the search input I was unable to do so. When arriving at the last letter the word just popped back in place. Probably something to do with your state.
A nice feature might be to let the user know that the play button has been clicked, maybe even change the icon.
Marked as helpful
@Mooonika90
Submitted
Any suggestions are more than welcome:)
@RubenSmn
Posted
Hi Mooonika, I just looked at your code for this challenge and you did a nice job on separating the CSS and JavaScript from the HTML. As you project expands this becomes more important so starting early is a good practice!
I did found some minor issue with the body which does not start at the top of the page. There is a empty space due to the margin of the main
element. This makes the page scrollable which is not a big issue but you could fix this by instead of top margin on the main
add it as top padding on the body
.
Hope you find this helpful and happy hacking!
@Camobeast
Submitted
It took me a long time to figure the grid layout, as this is my first time working with CSS Grid. Is there a better way to set the layout up?
@RubenSmn
Posted
Hi Jakub, nice work on this Notification page challenge. I looked at your code and found some things you could improve upon.
A good practice is the separation of concerns, currently you have all your code in the same file. You can move the JavaScript and CSS to their own file which makes your code more readable. Once your project becomes bigger it is easier to search for something.
A nice feature you can add here and practice JavaScript even more is to try and add functionality to click on individual unread notifications to make them read.
Hope you find this useful, happy hacking!
Marked as helpful
@Mike-is-coding
Submitted
I had a lot of difficulty using typescript in this project, but I got more comfortable using it towards the end as I started to understand the errors and my mistakes. Tailwind was also very difficult to use for me, it seemed very counterproductive. Overall, I think I learned a lot from this project and I'm really glad I completed it. Please leave any comments on anything I can improve on thank you.
@RubenSmn
Posted
Hi Mike, I just had a look at your solution for this challenge which looks really good visually.
I have some suggestion I want to share with you.
The useEffect
in your index.tsx has no dependency array. Which means it will run every render. You can fix this by including the theme
in the array.
In your QuerySection
you're passing some unused state dictData
.
I would suggest to move the header to a new component where you can handle the theme
and font
this leaves the QuerySection
with just the code for the actual query section.
Instead of sending all the theme state (theme
and setTheme
) down to the other components. Create a toggleTheme
function which you can pass down.
Hope you find this useful and happy hacking!
Marked as helpful
@RoboXGamer
Submitted
Any feedback is welcome!
I am still an early beginner in web development. I want to center the component in the center of the page but was unable to so any help would be great.
@RubenSmn
Posted
Hi RoboXGamer, nice job on this new challenge!
To align your element in the center you could use flexbox.
Here is a code snippet you can use, the min-height
makes sure we set the body to the whole vertical screen height. And then we apply the flexbox styles. justify-content
centers the items horizontally and align-items
centers them vertically.
body {
min-height: 100vh;
display: flex;
justify-content: center;
align-items: center;
}
Hope this is helpful and happy hacking!
@nxtime
Submitted
I was having trouble trying to setup a debouncer, to prevent heavy requests on the api, but I got it working, and way better than what I was expectating.
@RubenSmn
Posted
Hi Marcos, great UI and the great use of Framer Motion. I myself am trying to learn it because I think it is very powerful and can make great animation as you showed.
I really like that the url reflects the state of the input, very handy and good feature for the UX.
I did found some things that I think you could improve upon.
This was my first time learning Typescript and using it in a React project. Any best practices to be followed for tsc?
@RubenSmn
Posted
Hi Aravind, overall great work with this multi step form challenge. I did find some minor details that you could improve upon.
When selecting a yearly plan instead of a monthly the prices don't update to the yearly price.
When you hover over the addons it would be nice to show a cursor: pointer
to more indicate that these are clickable
Marked as helpful
@Diyar2222
Submitted
Hi there, the form validation and also mobile responsiveness parts were somehow difficult. I spent on them several hours. Overall, the project is very interesting. Any feedbacks and suggestions will be appreciated. Thanks.
@RubenSmn
Posted
Hi Diyar, nice work on this multi step form. I really like the feedback you get when selecting a plan and addon, the blue looks good.
I did find some minor things you could improve on with the UI/UX
@ayushprojects
Submitted
Hello everyone i have completed result-summary task by frontend mentor. please review my code and comment where I made mistakes.
@RubenSmn
Posted
Hi Ayush, great job on the challenge. I found a few things you could improve upon.
You could improve this by adding a hover effect as well as a cursor: pointer
.
You can build this by using media queries. A recommended approach is to first build the mobile version and then make it responsive to adjust to desktop sizes.
Marked as helpful
@AnnangBudiS
Submitted
i builld this challange with react+tailwindcss
@RubenSmn
Posted
Hi Annang, great work on this challenge. I had a look at the code and found some things you could improve on.
Instead of a Api
component a more recommended/best practice in React is by using a custom hook.
// api
const useApi = (url) => {
const [data, setData] = useState(null);
const [loading, setLoading] = useState(false);
const [error, setError] = useState(null);
useEffect(() => {
setLoading(true);
axios
.get(url)
.then((response) => {
setData(response.data);
})
.catch((err) => {
setError(err);
})
.finally(() => {
setLoading(false);
});
}, [url]);
return { data, loading, error };
};
// app
const App = () => {
const { data, loading, error } = useApi("...");
};
You're currently reloading the whole page when clicking on the button. This is not really the React way of doing things.
A solution for could be to introduce a new return prop from the api hook, a re-fetch function. Here we move the fetch functionality from the useEffect
into its own function and call the function in the useEffect
. When returning we can return a new prop called refetch
with our new function.
const useApi = (url) => {
const [data, setData] = useState(null);
const [loading, setLoading] = useState(false);
const [error, setError] = useState(null);
function fetchData() {
setLoading(true);
axios
.get(url)
.then((response) => {
setData(response.data);
})
.catch((err) => {
setError(err);
})
.finally(() => {
setLoading(false);
});
}
useEffect(() => {
fetchData();
}, [url]);
return { data, loading, error, refetch: fetchData };
};
The usage of this in the app is as simple as switching the newDatas
function for the refetch
one.
const { data, loading, error, refetch } = useApi(
"https://api.adviceslip.com/advice"
);
<button onClick={refetch} className="">
...
</button>;
Marked as helpful
@lgorvin
Submitted
Took me a while to complete, I did all the design first and then worked on the calculations for the total afterwards in hindsight I should have done it the other way round.
I used tailwind as it makes writing CSS much quicker but as a result my code looks pretty messy.
I still need to work on the total calculations because currently if the user changes add ons the total doesn't go back down.
@RubenSmn
Posted
Hi Luca, great work on this multi step form! I was impressed by the animation on the check icon! I did find some things that could be improved.
UI/UX improvements:
State/JS improvements:
(Monthly)
text. But the total price still has the previously selected plan price.I hope you find this helpful, happy hacking!
Marked as helpful
@Igraziella
Submitted
I've been on this challenge for a while now and still can't get around how to get an even border radius for all sides. I'm quite lost there. Any help on how to handle this will be really appreciated.
@RubenSmn
Posted
Hi Obialor, I was looking at the code for the border-radius
as you mentioned.
This goes from top left to top right to bottom right and finally to bottom left. Some more detailed information can be found on MDN docs.
You can implement this by using border-radius: 10px 0 0 10px
for the image and border-radius: 0 10px 10px 0
for the message. If you do it this way its recommended to not place the border-radius
on both elements at once since they need to have different values.
Another way you could solve this is to wrap both the img
and .message
in the .container
and apply styling to the container. This way you don't have to apply the border-radius
to the individual elements.
.container {
border-radius: 10px;
overflow: hidden; /* This makes sure the image has also rounded corners */
}
Of course this approach will need some additional styling to adopt the new layout.
Marked as helpful
@frank1003A
Submitted
In this project, my main objective was to make the code more accessible and conform to semantic standards. I initially considered using the "eval" function, but realized that it could be dangerous and is generally not recommended. As a result, I had to come up with a new approach for implementing the calculator's functionality, which I found quite enjoyable. Additionally, I was able to apply my recently acquired skills in regular expressions to the project.
@RubenSmn
Posted
Hi Frank, just played around with your calculator.
When calculating something like 5+6
which result in 11
after that I wanted to immediately calculate 4*2
but instead of that the input did no clear and I got 114*2
, not sure if this was something intended but I would like to mention it.
From that great work!
Marked as helpful
@Monzer65
Submitted
The code has issues with negative numbers. I didn't add a +/- button to the calculator for the sake of consistency with design.
@RubenSmn
Posted
Hi, just played around with your calculator.
I did however find that you'll have to exactly click on the theme button in order to change the theme. Maybe it is a nice feature to also be able to click the number of the theme you want to change to.
When calculating something like 5+6
which results in 11
after that I wanted to immediately calculate 4*2
but instead of that the input did not clear and I got 114*2
, not sure if this was something intended but I would like to mention it.
Marked as helpful
@ssenyondo67
Submitted
Any comment is strongly appreciated
@RubenSmn
Posted
Hi, I did some playing around and found that you're able to pass in a negative value in the input. When clicking Add to cart the cart gets a -amount label.
When I tried to add a extra item after one was already in the cart the count did not increase. Instead it changed to the newly added amount.
Once there are multiple items in the cart the delete functionality does not seem to work that well.
Marked as helpful
@Wagner-Goulart
Submitted
Hi Guys !
One more Challenge acomplished. This time I struggle with de image Hover Effect. It took me some time to figure out, bur I made.
@RubenSmn
Posted
Hi Wagner, looks really great and the hover effect works perfect! I had some issues with the hovering as well. I solved this using a extra container div the add the color there.
You could even improve this component further by adding some color for the day indication (3 days left) and the credit (Creation of) currently the text is a little hard to read.
Marked as helpful