Emmanuel Oloke
@EmmanuelOlokeAll comments
- @Mateo-Le-FurSubmitted about 2 years ago@EmmanuelOlokePosted about 2 years ago
Hello Mateo, trust you're doing great. Great job on completing this project, it looks really clean and professional, kudos.
I do have some observations that I wish to bring to your attention;
-
When a user clicks on a country card to show more details about the country and then refreshes the country details page, the user is taken back to the Home page as opposed to still being on the country details page that they refreshed on.
-
On the country details page, there's no state to handle when the country has no Border Countries to display, you can have a div with a text that says "No Border Countries" for example.
-
Also on the country details page, for countries with Border Countries, the design from Frontend Mentor did show that the country names be displayed in full and also be clickable in order to navigate to the corresponding country, but at the moment only the acronyms of the countries present in the "borders" property returned by the API are being displayed.
I also had this problem while attempting this challenge some weeks back. A way I was able to solve the issue was to loop over allCountries and also loop over the "borders" property array and check where the borderCountry matches the allCountries.cca3 property, then you can now return the allCountries.name.common value for that country in a separate array. Then use that array to display the border countries.
That worked for me, you can then go on to add a link to the div that navigates to that particular country's details page. This will fulfill the original design specifications.
Just thought to bring these to your notice and also share my experience building the same challenge. Hope this has been helpful. Enjoy!
Marked as helpful0 -
- @tchydySubmitted over 2 years ago@EmmanuelOlokePosted over 2 years ago
Hello Tochi, fantastic job you've done with this challenge, it really looks good. Kudos!
One observation I've made is that there is no colour change on hovering over the rating number elements. As per the style guides provided, the colour is supposed to change to orange on hover to depict the active state.
I just thought to bring that to your notice. Once again, good job on the challenge.
Marked as helpful1 - @olaniyivictorSubmitted over 2 years ago
I was not really sure of the active state own.
@EmmanuelOlokePosted over 2 years agoHey Victor. Great work on this challenge. Everything looks really good. Big kudos!!
One observation I'll like to bring to your notice though, the Active State for the "Equilibrium #3429" text has not been implemented yet. I'm sure it was just an oversight but just thought to bring it to your attention and point it out nonetheless.
Awesome job completing the challenge once again. Peace!
0 - @olaniyivictorSubmitted over 2 years ago@EmmanuelOlokePosted over 2 years ago
Hello Victor, great job you have done on this challenge.
Just one minor observation though, the header text is without spaces. That is, the "Get insights that help your business grow" text.
Just wanted to bring that to your attention.
Great job once again. Peace!
0 - @olaniyivictorSubmitted over 2 years ago
I am unsure of the mediaquery of phone own.
@EmmanuelOlokePosted over 2 years agoHello Victor, great job completing this challenge, keep up the good work.
There are a few observations I've made while going through the preview site though. I think the mobile responsiveness can be improved upon. Elements on the page can be better positioned to align more with the mobile design file provided in the challenge. I think using appropriate @media queries can take care of this problem, the right spacing and positioning should do it.
Also in the first section of the page, the Get Started button and input field could use some spacing and margins.
In addition, in the Get early access section, the text and form are supposed to be displayed side by side on the desktop view as opposed to the current arrangement.
Finally, I think the colour of the Fylo logo in the Footer section should be changed to white, to improve the contrast, the current colour is really difficult to see at the moment.
Once again, good job on completing the challenge. Enjoy the rest of your week.
0 - @CaioPaulin0Submitted over 2 years ago
all feedback is good :)
@EmmanuelOlokePosted over 2 years agoHello @CaioPaulin0, great work on this challenge, especially the mobile view. Looks really clean, however, there are some observations I've made that I'd like to bring to your notice:
-
The contents are bigger than the provided design file recommendations. Just a few adjustments in your HTML and CSS will fix this.
-
Clicking the thumbnails on the main page opens up the modal which I don't think conforms with the intended performance, it should instead change the main image to the larger version of the thumbnail image clicked. Also, the active state on the clicked thumbnail isn't implemented.
-
On the modal, there are no next and previous buttons. Also, the active state of the selected image in the thumbnail isn't implemented.
These are just some observations and improvements I think can make the solution look even better. Once again, great work on the challenge. Thanks.
1 -
- @webzmaSubmitted over 2 years ago
Me resultó un poco difícil cambiar el comportamiento de la imagen principal del producto. Me entretuve mucho haciendo este proyecto.
@EmmanuelOlokePosted over 2 years agoHello @WilberkLedezma, great work on this challenge, you've got the mobile view implementation absolutely spot on.
Just some observations I made and will like to bring to your notice:
-
When I view the solution on a widescreen monitor, the whole page content is aligned to the left of the screen. I think central alignment on the page will look much better.
-
On the Lightbox thumbnails styling, according to the design files, there should be some opacity on the thumbnail image as well as an orange border/outline when a particular thumbnail is clicked. With you solution, the opacity and border show only onClick and disappears afterwards.
-
When I increase the item quantity and click on the Add To Cart button, the cart icon badge in the navbar only changes to 1 instead of the quantity selected. I think using State Hooks and ContextAPI in ReactJS can help solve this issue.
Overall, awesome job with the challenge. Thanks.
Marked as helpful0 -
- @Hutchii299Submitted over 2 years ago
I really enjoyed this challenge as it allowed me to do a lot with vanilla javascript and practise the MVC pattern. I managed the product quantity through a state variable in the model JS file and also each part of the UI managed itself through their own view component JS file. I used a controller JS file to connect the views to the state variable and control and functionality involving multiple views and parsing state back and fourth. This made for a clean solution and by using grid I was able to make an easy responsive design without too many media queries.
@EmmanuelOlokePosted over 2 years agoHello Hutch, great work you've done with this challenge. Everything works as it should and it's fully responsive.
One little observation I've made though, in the header cart dropdown, when I click on the delete button to remove items, the item quantity in the "controls" class between the plus and minus icons doesn't change back to 0.
Just thought to bring that to your notice so you can make the necessary changes. Once again, awesome job. Kudos!
Marked as helpful1 - @Adrian397Submitted over 2 years ago
Any feedback welcome :)
@EmmanuelOlokePosted over 2 years agoHello Adrian, really awesome job you've done with this challenge I must say, well done!
I really love the mobile view menu implementation, especially that sick transition, awesome job with that.
Just some observations I'd like to highlight; when I clear the cart using the delete button in the navbar cart dropdown, the checkout button still shows, which is contrary to the design file provided. Also, the number of items doesn't change back to 0 once the cart has been cleared. I think you can handle this with React's useState hook.
Another is when I view the website using developer tools, there are some messages that show up in the console as I navigate through the lightbox/modal images. Maybe some leftover console.log debugging messages😅
All in all, really great job you've done. Kudos!!
1 - @olgak169Submitted almost 3 years ago
Feedback is welcome, let me know what you think!
@EmmanuelOlokePosted almost 3 years agoThis is an excellent implementation of the design. Well done!👍🏽
Marked as helpful0