Had fun building this, I'll like to hear your reviews. You'll love playing this game, added some random funny(mean) comment on the game, check it out!
Nemanja Dragun
@ndragun92All comments
- @chukwudobe-MicahSubmitted over 1 year ago@ndragun92Posted over 1 year ago
Looking good!
Well done. You made an achievement just by completing this complex challenge.
UI-wise I think it could be a bit more refined in a way to center circles a bit more as well as to change the position of notifications. One of the notifications appears at the right of the screen which shows a scrolling bar from time to time. Since it is a fixed position in terms of screen size maybe you can reposition items and disable the horizontal scroll to get rid of that effect.
Since u have made this already in vanilla javascript maybe as a next challenge you could try to rebuild same version of this in a framework such as Vue.js or React :)
Keep up the good work!
0 - @dumaaasSubmitted over 1 year ago@ndragun92Posted over 1 year ago
Hello,
Great job for completing the challenge. I would like to give a few tips regarding Vue as a framework that could help reduce your code in the future. For example, for Tip percentages, you could use something like
<div v-for="tip in [5, 10, 15, 25, 50]" :key="tip" class="tip-block" :class="{ active: selectedTip === tip }" v-text="`${tip}`%" @click="setSelectedTip(tip)" >
Only this would reduce your code for a lot of lines and you are kind of safe with numbers and what you pass + this syntax is really easy to extend or even replace a hardcoded array with a dynamic array if you are getting it from API
Other things are the shorthand of writing the if/else operator
${{ tipAmount ? tipAmount : "0.00" }} // Current ${{ tipAmount ?? "0.00" }} // New
Another bonus validation that you could make is to prevent people from writing numbers such as 010 as well as preventing adding characters that are not numbers
Also, you could wrap the element in <main> tag
Overall good job :)
Marked as helpful1 - @El-CassegrainSubmitted over 1 year ago
I lack of JavaScript Basics, and it was a real challenge do get it through. I know I can do better in code point of vue. I would be glad to get some reviews.
@ndragun92Posted over 1 year agoHello,
Great job! Congratz on finishing the challenge
I want to point few things here. I have seen your SubscriptionForm.vue and since this is a multi-step form and uses v-if/else conditions that means that some of the components do not need to be present there on the initial page load.
Instead of importing them like
import Step2 from "./SubscriptionParts/Step2.vue";
you can lazy load them to boost performanceconst Step2 = defineAsyncComponent(() => import('./SubscriptionParts/Step2.vue') )
Also, v-for should not use the index as a key (this is least recommended). Use the name or something unique if it is unique if not you can mix the title and index like
:key="`${plan.name}--${index}`"
v-for="(plan, index) in plans" :key="index"
Another thing regarding click/change events. I have seen that you mix v-on:click and @change I would recommend using the same syntax in code such as @click and @change to make it a bit better.
UI related forms could be centered in the middle of the screen
Overall good job! Keep it up!
Marked as helpful0 - @riansyhSubmitted over 1 year ago@ndragun92Posted over 1 year ago
Hello,
Great job! Well-made challenge!
I want to point few things here. I have seen your MainPage.vue and since this is a multi-step form and uses v-if/else conditions that means that some of the components do not need to be present there on the initial page load.
Instead of importing them like
import SecondForm from "./SecondForm.vue";
you can lazy load them to boost performanceconst SecondForm = defineAsyncComponent(() => import('./SecondForm.vue') )
Also, v-for should not use the index as a key (this is least recommended). Use the title if it is unique if not you can mix the title and index like
:key="`${title}--${index}`"
v-for="(title, index) in titles" :key="index"
Overall good job! Keep it up!
Marked as helpful0 - @TjasaZilSubmitted over 1 year ago
Used Vue3 and Tailwind for frontend, axios to make the API calls, Leaflet for map.
@ndragun92Posted over 1 year agoHello,
Great job completing this challenge.
Few things I did spot. There is
w-screen h-screen
in HTML which causes a horizontal slider to appear. I would suggest swapping that withw-full h-screen
to get rid of that scrollAlso, I know this is a test project but I would still suggest using
.env
variables for firebase credentials so no one else can use yours since they are currently visible by anyone in github. Also once things like that get submitted to git even if you delete them later, they will be present in git history.Also since it is Vue 3 I would suggest trying to use composition API and composables Another thing is that
this.countries.location && this.countries.location.region
can be shortened with optional paramsthis.countries.location?.region
Overall good job! Keep it up!
Marked as helpful0 - @rStrzelczyk98Submitted over 1 year ago
- mobile-first workflow 📱
I would greatly appreciate any feedback 🚀
@ndragun92Posted over 1 year agoHello,
Congratz on finishing the challenge.
Overall good job. There is just one single thing I would point out. Since you have to delay getting new data from API. On click, I would put some :active styles or loading styles until you get new data. Because at the moment when you click the button to generate data a few times sometimes it feels like nothing Is happening behind the scene.
Take care and good luck getting other solutions done!
Marked as helpful0 - @MphoolaSubmitted almost 2 years ago
It was an interesting challenge. I have practiced many staff through this project
@ndragun92Posted over 1 year agoHello,
Congratz on finishing the challenge.
I would give one suggestion. Regarding the filter dropdown instead of hardcoding values in the code since those values are already provided in
.json
file you could filter and extract unique regions and use dynamic v-for for the filter.Also UX wise some hover stats would be nice on clickable elements.
For the global store, I would recommend Pinia for Vue 3.
Also for some logical things, I would leverage composables from Vue 3.
Overall good job!
Marked as helpful1 - @nouarahSubmitted almost 2 years ago
I uesd vuejs and uikit in this project to practice vuejs more.
feel free to suggest anything for enhancement
@ndragun92Posted over 1 year agoHello,
First of all congratz on finishing the challenge.
I would like to give some tips while using Vuse as a framework.
When you make components and use them it is highly recommended to name them with two words Instead of
import card from '@/components/card.vue';
could beimport card from '@/components/ElCard.vue';
The main reason for this is to avoid collision with HTML5 native tags which are usually single words. What can happen is if you create a component that is or will be an HTML tag it will break your initial implementation.
Take care
Marked as helpful0 - @uvdevelop26Submitted almost 2 years ago
hello, I'm learning vue.js, this challenge was perfect to start practicing... if someone has a review please feel free to comment.
@ndragun92Posted over 1 year agoHello :)
Good job with completing the challenge!
I would like to point few things.
For example, in Vue, you could use a big advantage of what the framework is already offering. In order to show/hide tooltips you could have used a reactive variable and mouseover/leave set variable to indicate which tooltip to show. Another approach is the full CSS approach that would make this possible too without relying on JS.
One tip for smaller stuff and better code readability you could use something like
<span class="day text-soft-brown" v-text="spend.day" />
Take care
Marked as helpful0 - @crThiagoSubmitted over 1 year ago
The main objective of this challenge is to test the typescript with vue 3 and vuetify 3.
Firstly, since I used vuetify 3, the resolution is not pixel perfect.
Second point, I had a problem creating the project build, as I never worked with typescript I changed some types
@ndragun92Posted over 1 year agoHello,
Good job on getting this done :)
I would like to give advice on what to avoid doing in vue. When you use
v-for
as a:key
you should never use index. It should be something unique to the data you get such ascountry.id
orcountry.name
When doing a search watch I would highly advise using something as debounce rather then setTimeout
Also for code readability, I would use async/await
Also to refactor some logical stuff you could leverage Vue 3 Composables
Take care :)
Marked as helpful1 - @EgorVadikSubmitted over 1 year ago
Real-time comment section using next.js and Firebase feel free to use it as you wish login is done anonymously with no username or password
May have some bugs, feel free to tell me them and how to improve my work Thanks
@ndragun92Posted over 1 year agoHello,
Good job :) I want to give one UX tip. Once you submit the message, in my opinion, it is better to clear input rather than retain current text in the field. One bonus task could be to automatically increase textarea input while writing a message if you exceed the current textarea height
Take care
Marked as helpful1 - @AtishJoottunSubmitted over 1 year ago
Hello. Feedback of my work is highly appreaciated. Thank you in advance.
@ndragun92Posted over 1 year agoHello,
Congratz on finishing the project.
Few things I would like to point out while checking UI.
- Buttons do not have cursor-pointer which should be there to indicate that buttons are clickable (not just hover states)
- Code-wise name of classes for example class="Rating-question" is a bit strange to see it like this. Maybe you could use something like the BEM naming convention https://getbem.com/naming/
- For rating buttons you could have tried to use input type='radio' and then style it
Hope it helps
Marked as helpful2