2
\$\begingroup\$

I have put together a To-do Application with the Slim framework on the back-end (API) and a Vue 3 front-end. I added a demo on my YouTube channel.

In the main App.vue file I have:

<template>
  <div id="app">
    <Header 
    title="My todo list" 
    :unsolvedTodos = unsolvedTodos
    />

    <List 
      :todos="todos"
      :dataIsLoaded=dataIsLoaded
      @delete-todo="deleteTodo" 
      @toggle-todo="toggleTodo" />

    <Footer
      :isValidInput=isValidInput
      newTitle = ""
      placeholder= "+ Add new todo"
      validationMsg = "Please add at least 3 characters"
      @add-todo="addTodo"
      />

  </div>
</template>

<script>
import axios from 'axios'
import '@fortawesome/fontawesome-free/js/all.js';
import Header from './components/Header.vue'
import List from './components/List.vue'
import Footer from './components/Footer.vue'

export default {
  name: 'App',
  components: {
    Header,
    List,
    Footer
  },

  data() {
    return {
      apiUrl: "http://todo.com/api",
      dataIsLoaded: false,
      isValidInput: true,
      todos: [],
      unsolvedTodos: [],
    }
  },

  methods: {
    showTodos: function(){
      axios.get(`${this.apiUrl}/todos`)
      .then((response) => {
        this.todos = response.data;
      })
     .then(this.getUnsolvedTodos)
     .then(this.dataIsLoaded = true);
    },

    getUnsolvedTodos: function(){
      this.unsolvedTodos = this.todos.filter(todo => {
        return todo.completed == 0;
      });
    },

    toggleTodo: function(todo) {
      let newStatus = todo.completed == "0" ? 1 : 0;
      axios.put(`${this.apiUrl}/todo/update/${todo.id}`, {
        title: todo.title,
        completed: newStatus
      })
    },

    deleteTodo: function(id) {
      axios.delete(`${this.apiUrl}/todo/delete/${id}`)
    },

    addTodo: function(newTitle){
     const newToDo = {
        title: newTitle,
        completed: 0
      }
     
      if(newTitle.length > 2){
        this.isValidInput = true;
        axios.post(`${this.apiUrl}/todo/add`, newToDo);
      } else {
        this.isValidInput = false;
      }
    }
  },

  created() {
    this.showTodos();
  },

  watch: {
    todos() {
      this.showTodos();
    }
  }
}
</script>

In Header.vue:

<template>
  <header>
    <span class="title">{{title}}</span>
    <span class="count" :class="{zero: unsolvedTodos.length === 0}">{{unsolvedTodos.length}}</span>
  </header>
</template>

<script>
export default {
  props: {
    title: String,
    unsolvedTodos: Array
  },
}
</script>

In Footer.vue:

<template>
  <footer>
    <form @submit.prevent="addTodo()">
      <input type="text" :placeholder="placeholder" v-model="newTitle">
      <span class="error" v-if="!isValidInput">{{validationMsg}}</span>
    </form>
  </footer>
</template>

<script>
export default {
  name: 'Footer',

  props: {
    placeholder: String,
    validationMsg: String,
    isValidInput: Boolean
  },

  data () {
    return {
     newTitle: '',
    }
  },

  methods: {
    addTodo() {
      this.$emit('add-todo', this.newTitle)
      this.newTitle = ''
    }
  }
}
</script>

The to-do list (List.vue):

<template>
  <transition-group name="list" tag="ul" class="todo-list" v-if=dataIsLoaded>
      <TodoItem v-for="(todo, index) in todos"
        :key="todo.id" 
        :class="{done: Boolean(Number(todo.completed)), current: index == 0}" 
        :todo="todo" 
        @delete-todo="$emit('delete-todo', todo.id)"
        @toggle-todo="$emit('toggle-todo', todo)"
        />
    </transition-group>
    <div class="loader" v-else></div>   
</template>

<script>
import TodoItem from "./TodoItem.vue";

export default {
  name: 'List',

  components: {
    TodoItem,
  },

  props: {
    dataIsLoaded: Boolean,
    todos: Array
  },

  emits: [
    'delete-todo',
    'toggle-todo'
  ]
}
</script>

The single to-do item (TodoItem.vue):

<template>
  <li>
    <input type="checkbox" :checked="Boolean(Number(todo.completed))" @change="$emit('toggle-todo', todo)" />
    <span class="title">{{todo.title}}</span>
    <button @click="$emit('delete-todo', todo.id)">
      <i class="fas fa-trash-alt"></i>
    </button>
 </li>
</template>

<script>
export default {
  name: 'TodoItem',

  props: {
    todo: Object
  }
}
</script>

Questions / concerns:

  1. Is the application well-structured?
  2. Could the code be significantly "shortened"?
\$\endgroup\$
1
  • \$\begingroup\$ Were you able to simplify the event passing using v-bind="$attrs"? \$\endgroup\$ Jul 1, 2021 at 22:04

1 Answer 1

2
\$\begingroup\$

Is the application well-structured?

On the whole it seems okay, though see the answer to the next question that means the structure could be slightly changed for the better.

Could the code be significantly "shortened"?

Simon Says: use computed properties

Like Simon suggests: use computed properties - the implementation inside getUnsolvedTodos() could be moved to a computed property, with a return instead of assigning the result from calling .filter() to a data variable. Then there is no need to need to call that method and set up the property within the object returned by the data function.

Promise callback consolidation

The call to axios.get() in showTodos() has multiple .then() callbacks:

showTodos: function(){ 
  axios.get(`${this.apiUrl}/todos`)
    .then((response) => {
      this.todos = response.data;
    })
   .then(this.getUnsolvedTodos)
   .then(this.dataIsLoaded = true);
  },

Those can be consolidated to a single callback - especially since none of them return a promise.

showTodos: function(){
  axios.get(`${this.apiUrl}/todos`)
  .then((response) => {
    this.todos = response.data;
    this.getUnsolvedTodos(); //this can be removed - see previous section
    this.dataIsLoaded = true;
  });
},

While this does require one extra line, it would avoid confusion because somebody reading the code might think the statements passed to .then() should be functions that return promises.

Single-use variables

In toggleTodo the variable newStatus is only used once so it could be consolidated into the object passed to the call:

axios.put(`${this.apiUrl}/todo/update/${todo.id}`, {
  title: todo.title,
  completed: todo.completed == "0" ? 1 : 0
})

If that variable is kept it could be created with const instead of let since it is never re-assigned.

Passing events to parent

In List.vue the <TodoItem has these attributes:

@delete-todo="$emit('delete-todo', todo.id)"
@toggle-todo="$emit('toggle-todo', todo)"

Those seem redundant. In Vue 2 these lines could be replaced with a single line: v-on="$listeners" but apparently that was removed with Vue3. I tried replacing those lines with v-bind="$attrs" but it didn't seem to work - I will search for the VueJS 3 way to do this.

\$\endgroup\$
1
  • 2
    \$\begingroup\$ Any answer that says "Simon says" gets a +1 from me \$\endgroup\$ Jul 1, 2021 at 21:48

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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