2
\$\begingroup\$

Task Description:

  1. Create a div-element and take control over it using a Vue-Instance.

  2. Print an array of hobbies into the div-element. Provide some default hobbies.

  3. Add a 'New Hobby' button and a text-input element. So that the user can add additional hobbies.

  4. When a hobby list-item is clicked, it shall be removed from the list.

  5. Add a "Hobby deleted"-paragraph which is only shown when a hobby-item has been deleted at least one time.

  6. Above the list of hobbies add a hobby-counter which shows the current count of hobby-items.

  7. Style the hobby-list depending on whether you have more or less then 3 hobbies in the list.

  8. Outsource your hobbies (the list-item elements) into a component (so that it becomes re-usable).

Here's my solution for the points 1 to 7:

new Vue({
  el: '#app',
  data: {
    hobbies: [{
        name: 'Running'
      },
      {
        name: 'Chess'
      },
      {
        name: 'Cooking'
      },
      {
        name: 'Hiking'
      }
    ],
    newHobby: '',
    deleted: false
  },
  methods: {
    addHobby: function() {
      this.hobbies.push({
        name: this.newHobby
      });
    },
    removeHobby: function(event) {
      let updatedHobbies = this.hobbies.filter((hobby) => {
        return hobby.name !== event.target.textContent.trim();
      });

      this.hobbies = updatedHobbies;

      if (!this.deleted) {
        this.deleted = true;
      }
    },
    setStyle: function() {
      if (this.hobbies.length < 3) {
        return 'pink';
      } else if (this.hobbies.length === 3) {
        return 'orange';
      } else {
        return 'cyan';
      }
    }
  }
});
#app {
  max-width: 800px;
  margin: 0 auto;
}

li {
  margin: 10px;
}
<script src="https://cdnjs.cloudflare.com/ajax/libs/vue/2.5.3/vue.min.js"></script>
<div id="app">
  <div id="hobby-counter">
    <p>Hobbies: {{ hobbies.length }}</p>
  </div>
  <div id="add-control">
    <input type="text" id="newHobby" v-model="newHobby" />
    <button v-on:click="addHobby" id="addNewHobby">Add New Hobby</button>
  </div>
  <div id="deleted-message">
    <p v-if="deleted">Hobby deleted!</p>
  </div>

  <li v-for="hobby in hobbies" v-on:click="removeHobby" v-bind:style="{ backgroundColor: setStyle() }">
    {{ hobby.name }}
  </li>
</div>

Is everything done in a good way and manner?

Where could my implementation been improved?

Any ideas how to solve point 8?

There I get finally stucked.

Looking forward to read your comments and answers.

\$\endgroup\$
3
\$\begingroup\$
  • At the moment all your current hobbies only consists of a name. I think you could use a simple string instead of wrapping it in an object (for point 8 this is not possible though, of course).

  • No need to check if (!this.deleted) before setting this.deleted = true;

  • setStyle should be a computed property

  • You can use @click instead of v-on:click, you can also use :style instead of v-bind:style (just syntactic sugar)

  • I would put the v-if="deleted" on the <div id="deleted-message"> instead of the inner <p> element.

\$\endgroup\$
3
\$\begingroup\$

Like I alluded to in my comment, I considered voting to close your question because point #8 looked like a request for help implementing code that doesn't work. Alas, I will review the working code.

Feedback

I agree with the points in the answer by Simon - especially that there is no point to only setting this.deleted if it is falsey. Below are some other aspects that I noticed.

Rendering a list without a key

See the section key on the List rendering page of the Vue JS documentation1:

To give Vue a hint so that it can track each node’s identity, and thus reuse and reorder existing elements, you need to provide a unique key attribute for each item. An ideal value for key would be the unique id of each item.2

Apparently the elements in your list don't have a unique id, but the array index can be used, using the alternate v-for syntax:

<li v-for="(hobby, index) in hobbies"

That way the index (which is 0-based, since it is a JavaScript Array) will be available for the key attribute.

Removing items in constant time

That index mentioned in the previous section can be passed to the removeHobby() method, which could then use that index to call the JavaScript Array method slice() instead of filter(). As the list grows in size, the filter() approach would take more time, but calling splice() would be quicker because it would know the location of the element to remove. See this working in the snippet below.

new Vue({
  el: '#app',
  data: {
    hobbies: [{
        name: 'Running'
      },
      {
        name: 'Chess'
      },
      {
        name: 'Cooking'
      },
      {
        name: 'Hiking'
      }
    ],
    newHobby: '',
    deleted: false
  },
  methods: {
    addHobby: function() {
      this.hobbies.push({
        name: this.newHobby
      });
    },
    removeHobby: function(index) {
      this.hobbies.splice(index, 1);
      this.deleted = true;
    },
    setStyle: function() {
      if (this.hobbies.length < 3) {
        return 'pink';
      } else if (this.hobbies.length === 3) {
        return 'orange';
      } else {
        return 'cyan';
      }
    }
  }
});
#app {
  max-width: 800px;
  margin: 0 auto;
}

li {
  margin: 10px;
}
<script src="https://cdnjs.cloudflare.com/ajax/libs/vue/2.5.3/vue.min.js"></script>
<div id="app">
  <div id="hobby-counter">
    <p>Hobbies: {{ hobbies.length }}</p>
  </div>
  <div id="add-control">
    <input type="text" id="newHobby" v-model="newHobby" />
    <button v-on:click="addHobby" id="addNewHobby">Add New Hobby</button>
  </div>
  <div id="deleted-message">
    <p v-if="deleted">Hobby deleted!</p>
  </div>

  <li v-for="(hobby, index) in hobbies" v-on:click="removeHobby(index)" v-bind:style="{ backgroundColor: setStyle() }" :key="index">
    {{ hobby.name }}
  </li>
</div>

1 and note that it actually contains a todo-list example, which you could have based your code off of...

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.