Skip to content
OverviewSolutions
3
Comments
12

Nemanja Dragun

@ndragun92Home office340 points

Full Stack Developer Usually working with Adonis/Laravel/Feathers and Vue/Nuxt/Angular/React.

Latest solutions

Latest comments

  • Nemanja Dragun•340
    @ndragun92Posted about 2 years 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
  • Nemanja Dragun•340
    @ndragun92Posted about 2 years 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 helpful
    1
  • Nemanja Dragun•340
    @ndragun92Posted about 2 years ago

    Hello,

    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 performance const 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 helpful
    0
  • Nemanja Dragun•340
    @ndragun92Posted about 2 years 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 performance const 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 helpful
    0
  • Nemanja Dragun•340
    @ndragun92Posted about 2 years ago

    Hello,

    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 with w-full h-screen to get rid of that scroll

    Also, 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 params this.countries.location?.region

    Overall good job! Keep it up!

    Marked as helpful
    0
  • Nemanja Dragun•340
    @ndragun92Posted about 2 years ago

    Hello,

    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 helpful
    0
View more comments