2
\$\begingroup\$

Simple Vue.js component, where you can add a wrestler via an AJAX endpoint, and remove a wrestler from an AJAX endpoint.

Any notes on whether I’m doing this the “Vue.js way” or tips for improvement are greatly welcomed.

The HTML template portion of the component:

<template>
    <div>
        <p v-if="loading">Loading&hellip;</p>
        <div v-else>
            <ul class="list-inline">
                <li class="list-inline-item" v-for="(wrestler, index) in wrestlersSortedByName" v-bind:key="wrestler.id">
                    <button type="button" class="btn btn-outline-danger" v-on:click.prevent="removeWrestler(wrestler, index)">
                        <span class="btn-text">{{ wrestler.name }}</span>
                        <span class="btn-icon" aria-hidden="true">&times;</span>
                    </button>
                </li>
            </ul>
            <div class="form-group">
                <div class="input-group">
                    <input type="text" class="form-control" autocomplete="name" required ref="name" v-bind:disabled="busy" v-model="name" v-on:keydown.enter.prevent="addWrestler" />
                    <div class="input-group-btn">
                        <button type="button" class="btn btn-primary" v-on:click.prevent="addWrestler" v-bind:disabled="busy">
                            <span class="btn-text" v-html="submitText" />
                        </button>
                    </div>
                </div>
            </div>
        </div>
    </div>
</template>

The JavaScript portion of the component:

const confirm = require('./../mixins/confirm.js');

export default {
    computed: {
        submitText() {
            return this.busy ? 'Submitting&hellip;' : 'Add wrestler';
        },
        wrestlersSortedByName() {
            return this.wrestlers.sort((a, b) => {
                return a.name.localeCompare(b.name);
            });
        }
    },
    data() {
        return {
            busy: false,
            loading: true,
            name: '',
            wrestlers: []
        };
    },
    methods: {
        addWrestler() {
            this.busy = true;

            axios.post(this.action, { name: this.name })
                .then((response) => {
                    this.wrestlers.push(response.data.data);
                })
                .catch((response) => {
                    alert('Error adding wrestler to video.');
                })
                .then(() => {
                    this.busy = false;
                    this.name = '';
                    this.$refs.name.focus();
                });
        },
        loadWrestlers() {
            axios.get(this.action).then((response) => {
                this.wrestlers = response.data.data;
                this.loading = false;
            });
        },
        removeWrestler(wrestler, index) {
            const message = `Are you sure you wish to remove ${wrestler.name} from this video?`;

            this.confirm(message, () => {
                axios.delete(`${this.action}/${wrestler.id}`)
                    .then((response) => {
                        this.wrestlers.splice(index, 1);
                    })
                    .catch((response) => {
                        alert('Error removing wrestler.');
                    });
            });
        }
    },
    mixins: [
        confirm
    ],
    mounted() {
        this.loadWrestlers();
    },
    props: {
        action: {
            required: true,
            type: String
        }
    }
}

And finally, the contents of ./mixins/confirm.js:

module.exports = {
    methods: {
        confirm(message, callback) {
            if (window.confirm(message)) {
                callback();
            }
        }
    }
};

Style notes: I’m aware of the @ shorthand for v-on:, but prefer the longer form as it looks more HTML-y to me.

\$\endgroup\$
  • \$\begingroup\$ @SamOnela Not sure I follow? this.action is a URL, i.e. http://example.com/videos/1/wrestlers, so is just a string. \$\endgroup\$ – Martin Bean Nov 28 '17 at 16:40
  • \$\begingroup\$ Oh sorry - I was confusing the data and props properties \$\endgroup\$ – Sᴀᴍ Onᴇᴌᴀ Nov 28 '17 at 16:45
  • \$\begingroup\$ @SamOnela Ah, yeah. It’s a bit awkward as the API returns a JSON response nested in a data key! \$\endgroup\$ – Martin Bean Nov 28 '17 at 17:09
2
\$\begingroup\$

It looks like the code makes good use of basic VueJS concepts and practices- using computed properties, binding values, conditional display with v-if, etc.

I also like the application of arrow functions, promise handling, etc.

I see there is a key attribute bound to the list items (i.e. v-bind:key="wrestler.id"). Correct me if I am wrong but that doesn't appear to be an attribute of <li>. What is that used for? Edit: Oh yes, thanks for reminding me about the key attribute for list editing

Then in the addWrestler() method, there are two .then() callbacks. Is there a reason not to move the three lines in the second callback into the first?

axios.get(this.action, { name: this.name })
    .then((response) => {
        this.wrestlers.push(response.data.data);
        this.busy = false;
        this.name = '';
        this.$refs.name.focus();
    })
    .catch(...)

If you really wanted to shorten the code for the promise callbacks, you could abstract those into methods.

\$\endgroup\$
  • \$\begingroup\$ Thanks. The v-bind:key is something that seems to be recommended by Vue to “identify” nodes in a list: vuejs.org/v2/guide/list.html#key. The second then() callback in the example pointed out was to DRY the code out a bit: I wanted to do those things whether the request succeeded or failed, so chained another then() callback to avoid copying it into both the first then() and catch() callbacks. \$\endgroup\$ – Martin Bean Nov 29 '17 at 10:37
  • \$\begingroup\$ Thanks for reminding me about the key attribute for list rendering. And good call with the DRYness! \$\endgroup\$ – Sᴀᴍ Onᴇᴌᴀ Nov 29 '17 at 16:38

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.