Wonuola Alonge• 385
@wonuola-y
Submitted
Reviews please! If there's anyway I can improve this please help out
Caleb Bankole
@BankoleCaleb
All comments
- Caleb Bankole• 230
@BankoleCaleb
Posted
Hi. You did a good job on this project and congratulation on your new job - I saw it on LinkedIn.
First, I realized that not all part of your "go back to notifications" button is clickable. User has to click on the text itself. You can look it up Also, you are logging a message to the console. You should have cleared all your console.log() before deploying.
Thirdly, when you click the "go back to notifications" button, your home page still shows three notifications unread even after one has read it before clicking a notification.
Good job and keep coding. How was your UTME score? Did you attend Adams' College post-UTME class?
Marked as helpful
0 - Nduoyoosung• 70
@Nduoyoosung
Submitted
I worked has had i could to accomplish this task
Caleb Bankole• 230@BankoleCaleb
Posted
Keep up the hard work! It is a very beautiful and responsive site.
Anyways, you could improve the functionality of the pink transitioning button when hovered. Instead of changing the top margin, you could have just transitioned it and it does not have to be up to 25px - it is making it flicker when the mouse it at some point around the button (you can confirm that yourself).
By the way you really did a good job; keep up the journey!
0 - Silas• 350
@Silas310
Submitted
The JS gave me a lot of work, and I've got good insights, but I'm pleased by the final result.
Caleb Bankole• 230@BankoleCaleb
Posted
Well done, you have done good job but I believe it can be only be better. There are a lot of things to make better here, but I'm just going to share a few things.
First, you didn't have to use three different CSS files; you could have just adjusted styles using media query. Also about your script, its is no longer in vogue to use var declaration. Furthermore, instead of having a long chain of "if else" statements, you could have just used a switch block. By the way, most of the things you used js for could have been easily done with CSS.
I hope you didn't find this review criticizing. Keep working and I can't wait to meet you at the top.
Marked as helpful
1 - Lazar Stojanović• 160
@Coolbylaki
Submitted
This project was very easy
Caleb Bankole• 230@BankoleCaleb
Posted
Good job! No doubt, it is a really easy project, but you must always put every little details into account - like centering the paragraph and "star icon", which states otherwise according to the design.
Marked as helpful
1 - Caleb Bankole• 230
@BankoleCaleb
Submitted
I struggled in the collapsing part of the mobile view. Please help me view my code and guide me on how best to do it.
Caleb Bankole• 230@BankoleCaleb
Posted
@Kamlesh0007 please, help me take a look at the code of this project and how I can improve it.
0 - Miguel Olarte• 20
@Olarte910903
Submitted
this is my first project of frontendmentor.
my dificult are flexbox aligns
Caleb Bankole• 230@BankoleCaleb
Posted
Well done on your first project!🎉🎉🥳
First, I'd like to tell you that you don't need flexbox on the div. You only needed it on the body for centering. Also, using semantic HTML could have improved the accessibility of your site. Furthermore, the page is not responsive.
You didn't have to use more than one CSS file as you have done; you could have just used media query. With what you have done, no style is applied on small screen - confirm that.
I hope you find this helpful. Continue on journey; we'll meet at the top!
Marked as helpful
0 - Moh Derajatul ula• 180
@Derajatul
Submitted
Caleb Bankole• 230@BankoleCaleb
Posted
You did it! It is awesome, you are awesome!
Anyways, there is always room for improvement... First, you did not follow the design for the desktop view. Also, you did not spell 'careers' correctly (although that is minor😊) You should also be conscious of the style on the "learn more" button - the hover state.
Again, I must commend you for the wonderful work done. We'll meet at the top!
Marked as helpful
1 - Kamlesh Rajesh Yadav• 5,060
@Kamlesh0007
Submitted
Caleb Bankole• 230@BankoleCaleb
Posted
Nice work!
Although it could have been more efficient if you something like CSS variables for the different color mode instead of changing the style file.
Also, there is a repetition of a code above on line 107. Is there really a need for that?
Marked as helpful
0 - Ayotommy012• 20
@Ayotommy012
Submitted
what do you guys think does it need any other adjustments
Caleb Bankole• 230@BankoleCaleb
Posted
Of course, it needs adjustment. Especially the centering.
Since you have decided to use flexbox on the body like that, you can also change the flex direction so that the two block would stack on top of each other, rather than side by side as it is now.
Also, you did not seem to use the right colors. The text is not readable.
You have done a very good job, kudos. Just a little more touch.
Marked as helpful
0 - James King• 250
@jamesaking02
Submitted
I could not figure out how to get the background images positioned correctly. It was really frustrating, so I just omitted them. I would love some input on how to do it correctly, and then I can resubmit.
Caleb Bankole• 230@BankoleCaleb
Posted
For positioning the background images at the top left and bottom right, you'd use this style:
background-position: 0% 0%;
That's basically the format, I think you should be able to edit the value to suit your need. You could also use negative values. Note that you are putting the style on the body.
background-position: left top;
0 - Wonuola Alonge• 385
@wonuola-y
Submitted
Reviews please
Caleb Bankole• 230@BankoleCaleb
Posted
Hello again, Wonuola. First, I must commend you for taking up the challenge (not everyone has the mind to).
Anyways, there are so many areas to work on. If you don't mind, we can do it together.
To start with, the images are too big that there is scrolling even on large screen. Also, the links are not well styled as they don't seen clickable. I know it is just a demo site, but you can still add basic styles like "cursor: pointe;"
There are other observations, but you can chat me on WhatsApp if you like to link up.
08134690893
0 - Wonuola Alonge• 385
@wonuola-y
Submitted
Feedback and reviews please!
Caleb Bankole• 230@BankoleCaleb
Posted
Good job, Alonge.
Anyways, you can improve the experience by changing the cursor types of the logos, register, login button.
Also, the image is too large for mobile view. A trick you can use is to change the "width" which you set to 100% to "max-width". I promise, it would be a magic!
0
