Design comparison
Community feedback
- @UnidadePosted almost 2 years ago
I looked your javascript code and I found that you a are making more fetch calls than the necessary, I inspected this with the network section on devTools. This is an example that are present in your components:
{fetchLocation(ip).city}, {fetchLocation(ip).country}
The response of this fetch call is a data object that have all the properties that you are accessing, you can be more efficient if you store the fetch data in a variable and access them as need. For example, this only make a fetch call.
const data = fetchLocation(ip) {data.city}, {data.country} ...
This is important because you don't want to make unnecessary call to the server, they can bottleneck your app, you are using more of your api call quotes and isn't efficient at all.
Marked as helpful1@msabdalaalPosted almost 2 years ago@Unidade Hi Dario, thx for clarifying this point I didn't really notice this at first, but I will take it in consideration, thx so much for the tip as well
0@UnidadePosted almost 2 years ago@msabdalaal You're welcome, I think what you are doing with your fetchLocation is a perfect use case for react context, you are trying to share the same data across your components, If you implement the same idea but using react context I think your code will look a lot cleaner. I forked your repo and implemented react context, if you want to read my version with context: https://github.com/Unidade/IP-Address-Tracker If you are fine with my changes I can make a pull request.
Another thing, your input placeholder text has ''or domain'' but isn't possible to insert a valid domain like: ''www.google.com'', either your API can handle domains, so, I think is better to put it off.Marked as helpful1@msabdalaalPosted almost 2 years ago@Unidade actually I didn't know how to use React Context but since you think it's better, I read your version and you're more than welcome to make a pull request. and I will start learning context for feature projects asap
0 - @tabetommyPosted about 2 years ago
Your solution looks really good especially the improvised box at the right showing the route planner linking to google map. It will be really look closer to the given design if you adjust the UI containing the IP address informations to the center of the border between the map view and background image at the top. You could use position relative for parent container and position absolute for child container to achieve this. Also, frontend mentor platform reports an error on your iframe element. something about attribute loading not allowwed. Honestly i don't know much about that but you can check.Otherwise i think your solution looks really good, functional and responsive
Marked as helpful1@msabdalaalPosted almost 2 years ago@tabetommy thx so much for the feedback glad that you liked my solution, and thx for the postion relative tip really helpful .. me nethier i don't know much about the loading thing but i will figure it out.. thx again ❤️❤️
0
Please log in to post a comment
Log in with GitHubJoin our Discord community
Join thousands of Frontend Mentor community members taking the challenges, sharing resources, helping each other, and chatting about all things front-end!
Join our Discord