2
\$\begingroup\$

I'm trying to learn Vuejs and started this project: https://github.com/GhitaB/datatables-admin (Live demo: https://ghitab.github.io/datatables-admin/).

Copied the current version (the question will refer to it): https://jsfiddle.net/ns6umqv2/

My goal is to have a nice admin panel that will be used by editors to generate tables containing lists of items. An item basically will be a link (URL) + some metadata (the other columns defined by editor).

The content will be saved somewhere as json (not needed for the moment). In view mode that json will be rendered as Datatable.

In this simplified version you can:

  • define columns, change their order, delete
  • define rows, change their order, delete
  • edit any cell (navigate between editable cell using Tab / Shift + Tab)
  • delete all columns, delete all rows
  • render the current table as Datatable (and re-render anytime) by pressing "Preview table" button
  • sort the rendered table, filter by any word (the URL is included as searchable text)

Please take a look, I fell like it can be improved - but I can't see exactly where and how.

Also, feel free to contribute on GitHub if you like the idea.

js code:

Vue.component('editable', {
  template: `
    <div contenteditable="true" @blur="emitChange">
      {{ content }}
    </div>
  `,
  props: ['content'],
  methods: {
    emitChange(ev) {
      this.$emit('update', ev.target.textContent)
    }
  }
});

Vue.component('table-preview', {
  template: `
    <div class="table-preview-container">
      <button class='render-table' v-on:click="render_table">Preview table</button>
      <table class="table-render-preview"></table>
    </div>
  `,
  props: ['content'],
  methods: {
    render_table() {
      var columns = this.$parent.columns;
      var rows = this.$parent.rows;
      var el = event.srcElement;
      var parent = el.offsetParent;
      var table_placeholder = document.querySelector('.table-render-preview');


      function make_table_html(columns, rows) {
        function render_link(url) {
          if(url !== undefined) {
            return "<a href='" + url +"' target='_blank' title=" + url + ">Link<span style='display:none !important'>" + url + "</span></a>";
          } else {
            return "N/A";
          }
        }

        var result = "<table border=1><thead><tr>";

        for(var i = 0; i < columns.length; i++) {
          result += "<th>" + columns[i] + "</th>";
        }

        result += "</thead><tbody>"
        for(var i = 0; i < rows.length; i++) {
          result += "<tr>";
          for(var j = 0; j < rows[i].length; j++) {
            if(columns[j] == "URL") {
              result += "<td>" +  render_link(rows[i][j]) + "</td>";
            } else {
              result += "<td>" + rows[i][j] + "</td>";
            }
          }
          result += "</tr>";
        }
        result += "</tbody></table>";

        return result;
      }

      if ($.fn.DataTable.isDataTable(".table-render-preview")) {
        $('.table-render-preview').DataTable().clear().destroy();
      }

      var new_el = document.createElement("table");
      new_el.className = "table-render-preview";
      new_el.innerHTML = make_table_html(columns, rows);
      table_placeholder.parentNode.replaceChild(new_el, table_placeholder);

      $('.table-render-preview').dataTable({
        "destroy": true,
        aaSorting: []
      });
    }
  }
});

new Vue({
  el: '#datatables-admin',
  data: {
    LOREM: "Click me to edit",
    NONE: "",
    columns: ['Click me to edit', 'Demo column 2', 'Demo column 3', 'URL'],
    rows: [
      ['col1 data1', 'col2 data1', 'col3 data1', 'https://www.google.com'],
      ['col1 data2', 'col2 data2', 'col3 data2', 'https://www.yahoo.com']
    ],
  },
  methods: {
    refresh: function() {
      this.$forceUpdate();
    },

    update_col: function(content, col_index) {
      this.columns[col_index] = content;
      this.refresh();
    },

    update_row: function(content, row_index, col_index) {
      this.rows[row_index][col_index] = content;
      this.refresh();
    },

    add_col: function(col_index) {
      // Add a new column at given index
      this.columns.splice(col_index, 0, this.LOREM);
      for(var i = 0; i < this.rows.length; i++) {
        var row = this.rows[i];
        row.splice(col_index, 0, this.NONE);
      }
      this.refresh();
    },

    delete_col: function(col_index, skip_confirm = false) {
      if(!skip_confirm) {
        var result = confirm("Are you sure you want to delete this column?");
        if(!result) {
          return;
        }
      }

      // Remove column
      this.columns.splice(col_index, 1);

      // Remove related items in rows
      for(var i = 0; i < this.rows.length; i++) {
        var row = this.rows[i];
        row.splice(col_index, 1);
      }

      this.refresh();
    },

    add_row: function(row_index) {
      // Add a new row at given index
      this.rows.splice(row_index, 0, new Array(this.columns.length));

      for(var i = 0; i < this.columns.length; i++) {
        this.rows[row_index][i] = this.NONE;
      }

      this.refresh();
    },

    delete_row: function(row_index, skip_confirm = false) {
      if(!skip_confirm) {
        var result = confirm("Are you sure you want to delete this row?");
        if(!result) {
          return;
        }
      }

      this.rows.splice(row_index, 1);

      this.refresh();
    },

    delete_all_rows: function() {
      var result = confirm("Are you sure you want to delete all rows?");
      if(!result) {
        return;
      }

      var nr_rows = this.rows.length;
      for(var i = 0; i < nr_rows; i++) {
        this.delete_row(0, skip_confirm = true);
      }

      this.refresh();
    },

    delete_all_cols: function() {
      var result = confirm("Are you sure you want to delete all columns?");
      if(!result) {
        return;
      }

      var nr_cols = this.columns.length;
      for(var i = 0; i < nr_cols; i++) {
        this.delete_col(0, skip_confirm = true);
      }

      this.refresh();
    },

    move_col_to_left: function(col_index) {
      if(col_index == 0) {
        return;
      }
      var temp = this.columns[col_index - 1];
      this.columns[col_index - 1] = this.columns[col_index];
      this.columns[col_index] = temp;

      for(var i = 0; i < this.rows.length; i++) {
        temp = this.rows[i][col_index - 1];
        this.rows[i][col_index - 1] = this.rows[i][col_index];
        this.rows[i][col_index] = temp;
      }

      this.refresh();
    },

    move_col_to_right: function(col_index) {
      if(col_index == this.columns.length - 1) {
        return;
      }
      var temp = this.columns[col_index + 1];
      this.columns[col_index + 1] = this.columns[col_index];
      this.columns[col_index] = temp;

      for(var i = 0; i < this.rows.length; i++) {
        temp = this.rows[i][col_index + 1];
        this.rows[i][col_index + 1] = this.rows[i][col_index];
        this.rows[i][col_index] = temp;
      }

      this.refresh();
    },

    move_row_up: function(row_index) {
      if(row_index == 0) {
        return;
      }
      var temp = this.rows[row_index - 1];
      this.rows[row_index - 1] = this.rows[row_index];
      this.rows[row_index] = temp;

      this.refresh();
    },

    move_row_down: function(row_index) {
      if(row_index == this.rows.length - 1) {
        return;
      }
      var temp = this.rows[row_index + 1];
      this.rows[row_index + 1] = this.rows[row_index];
      this.rows[row_index] = temp;

      this.refresh();
    }
  }
});

html:

<!doctype html>
<html>
<head>
  <meta charset="utf-8" />
  <meta http-equiv="X-UA-Compatible" content="IE=edge,chrome=1" />
  <title>Datatables admin</title>
  <meta name="description" content="Datatables administration panel" />
  <meta name="viewport" content="width=device-width, initial-scale=1" />

  <link rel="stylesheet" href="styles.css" />
  <link rel="stylesheet" href="https://use.fontawesome.com/releases/v5.3.1/css/all.css"
        integrity="sha384-mzrmE5qonljUremFsqc01SB46JvROS7bZs3IO2EmfFsd15uHvIt+Y8vEf7N7fWAU" crossorigin="anonymous" />
  <link rel="stylesheet" href="https://cdn.datatables.net/1.10.18/css/jquery.dataTables.min.css" />
</head>

<body class="dta">
  <div id="datatables-admin">
    <h1>Datatables admin</h1>

    <table id="editor">
      <thead>
        <tr>
          <th v-for="(column, index_col) in columns" :key="index_col">
            <i class="fas fa-long-arrow-alt-left fa-2x dta-btn move-col-left" title="Move column to left" v-on:click="move_col_to_left(index_col)"></i>
            <i class="fas fa-long-arrow-alt-right fa-2x dta-btn move-col-right" title="Move column to right" v-on:click="move_col_to_right(index_col)"></i>
            <i class="fas fa-plus fa-2x dta-btn add-col" title="Add a column after this one" v-on:click="add_col(index_col + 1)"></i>
            <i class="fas fa-times fa-2x dta-btn delete-col" title="Delete this column" v-on:click="delete_col(index_col)"></i>
            <br />
            <editable :content="columns[index_col]" v-on:update="update_col($event, index_col)"></editable>
          </th>
          <th>
            <i class="fas fa-plus fa-2x dta-btn add-col" title="Add a column" v-on:click="add_col(0)"></i>
            <i class="fas fa-plus fa-2x dta-btn add-row" title="Add a row" v-on:click="add_row(0)"></i>
            <i class="fas fa-times fa-2x dta-btn delete-all-cols" title="Delete all columns" v-on:click="delete_all_cols"></i>
            <i class="fas fa-times fa-2x dta-btn delete-all-rows" title="Delete all rows" v-on:click="delete_all_rows"></i>
          </th>
        </tr>
      </thead>
      <tbody>
        <tr v-for="(row, index_row) in rows" :key="index_row">
          <td v-for="(column, index_col) in columns" :key="index_col">
            <editable :content="rows[index_row][index_col]" v-on:update="update_row($event, index_row, index_col)"></editable>
          <td>
            <i class="fas fa-long-arrow-alt-up fa-2x dta-btn move-row-up" v-on:click="move_row_up(index_row)" title="Move row up"></i>
            <i class="fas fa-long-arrow-alt-down fa-2x dta-btn move-row-down" v-on:click="move_row_down(index_row)" title="Move row down"></i>
            <i class="fas fa-plus fa-2x dta-btn add-row" title="Add a row under this one" v-on:click="add_row(index_row + 1)"></i>
            <i class="fas fa-times fa-2x dta-btn delete-row" title="Delete this row" v-on:click="delete_row(index_row)"></i>
          </td>
        </tr>
      </tbody>
    </table>

    <table-preview></table-preview>
  </div>

  <script
    src="https://code.jquery.com/jquery-3.3.1.min.js"
    integrity="sha256-FgpCb/KJQlLNfOu91ta32o/NMZxltwRo8QtmkMRdAu8="
    crossorigin="anonymous"></script>

  <script src="https://cdn.datatables.net/1.10.18/js/jquery.dataTables.min.js"></script>

  <script src="https://cdn.jsdelivr.net/npm/vue/dist/vue.js"></script><!-- dev -->
  <!-- <script src="https://cdn.jsdelivr.net/npm/vue"></script> -->
  <script src="datatables-admin.js"></script>
</body>
</html>

Styles:

/* https://coolors.co/96adc8-d7ffab-fcff6c-d89d6a-6d454c
   #96adc8 #d7ffab #fcff6c #d89d6a #6d454c
*/
@import url('https://fonts.googleapis.com/css?family=Open+Sans|Tangerine');

body.dta {
  font-family: 'Open Sans', sans-serif;
  background: #6d454c;
  color: #6d454c;
}

body.dta div#datatables-admin {
  background: #EEEEEE;
  padding: 20px;
  margin: 20px;
}

body.dta div#datatables-admin h1 {
  font-family: 'Tangerine', cursive;
  color: #d89d6a;
  text-align: center;
  font-size: 86px;
  font-weight: 500;
  margin: 20px;
}

body.dta div#datatables-admin table#editor {
  background: #CCCCCC;
  padding: 10px;
  border-spacing: 5px;
  color: #111111;
  margin: auto;
}

body.dta div#datatables-admin table#editor tr,
body.dta div#datatables-admin table#editor td,
body.dta div#datatables-admin table#editor th {
  background: #FFFFFF;
  padding: 5px;
  margin: 5px;
  text-align: center;
}

body.dta div#datatables-admin table#editor th {
  user-select: none;
  -moz-user-select: none;
  -khtml-user-select: none;
  -webkit-user-select: none;
  -o-user-select: none;
}

body.dta div#datatables-admin .dta-btn {
  color: #96adc8;

  user-select: none;
  -moz-user-select: none;
  -khtml-user-select: none;
  -webkit-user-select: none;
  -o-user-select: none;
}

body.dta div#datatables-admin .dta-btn:hover {
  color: #d89d6a;
  cursor: pointer;
}

body.dta div#datatables-admin div.table-preview-container {
  text-align: center;
  margin-top: 40px;
}

body.dta div#datatables-admin table.table-render-preview {
  margin: auto;
}

body.dta div#datatables-admin button.render-table {
  margin-bottom: 40px;
  padding: 10px;
}
\$\endgroup\$
2
  • 1
    \$\begingroup\$ Welcome to Code Review! Unfortunately we can't review this because you explained a bug scenario. Please read What topics can I ask about here? - note that it states "If you are looking for feedback on a specific working piece of code...then you are in the right place!" Also, when posting your question, there should have been text on the side that read "Your question must contain code that is already working correctly..." After you fix the code please update it and we will review it. \$\endgroup\$ Commented Oct 2, 2018 at 16:37
  • 1
    \$\begingroup\$ Oh, thank you very much. Updated the question - I consider it a stable version now - first one. \$\endgroup\$
    – GhitaB
    Commented Oct 3, 2018 at 2:42

1 Answer 1

1
\$\begingroup\$

#General feedback Whenever I see jQuery and VueJS used together, I question whether they both need to be used. For example, many of the jQuery selection can be converted to using $refs or perhaps something like slots. But because you are using the jQuery datatables plugin jQuery can't really be eliminated in an easy fashion...

I did a search online for "vuejs datatables" and found various packages, like this vuejsexample.com package, VueDatatable on github and an NPM package that uses the same dependencies as yours. You might consider switching from the jQuery plugin to one of those Vue packages...

#Specific feedback ##Error: Undefined variable: event

methods: {
    render_table() {
      var columns = this.$parent.columns;
      var rows = this.$parent.rows;
      var el = event.srcElement;
             //^ReferenceError: event is undefined

I was able to add event as the first argument and see the rendered table.

##Flaw with arguments on method call In delete_all_rows() there is a call in the for loop:

delete_all_rows: function() {
  var result = confirm("Are you sure you want to delete all rows?");
  if(!result) {
    return;
  }
  var nr_rows = this.rows.length;
  for(var i = 0; i < nr_rows; i++) {
    this.delete_row(0, skip_confirm = true);

And similarly in the for loop in delete_all_cols():

this.delete_col(0, skip_confirm = true);

I presume you are intending to set that second argument value to true, which works but at the same time this is declaring a global variable (since no local variable skip_confirm exists in those methods) before passing that value along. Perhaps you merely copied those arguments straight from the method signature. Remove the skip_confirm = to avoid declaring global variables.

this.delete_col(0, true);

#Suggestions ##Table methods You could consider adding rows with HTMLTableElement.insertRow() and adding cells with HTMLTableRowElement.insertCell() instead of using string literals - or else use a template...

##EcmaScript-2015 features Some features of EcmaScript-2015, A.K.A. ES-6 are used:

  • The template properties for the components are defined using template literals,
  • The default values for arugments on methods - like delete_col: function(col_index, skip_confirm = false) {

Because those features are used, you might as well use other ES-6 features like:

###VueJS features As mentioned above, use $refs instead of querying for elements. For example, the Table preview component table could be updated like below:

template: `
<div ref="tablePreviewContainer" class="table-preview-container">
  <button class='render-table' v-on:click="render_table">Preview table</button>
  <table ref="tableRenderPreview" class="table-render-preview"></table>
</div>
`,

Then in the methods, instead of querying the entire DOM for that element:

var table_placeholder = document.querySelector('.table-render-preview');

use $refs to access them:

const table_placeholder = this.$refs.tableRenderPreview;

Notice that const was used instead of var because that value never gets re-assigned.

For more tips on cleaner JS, check out this article. I know it is a few years old and bashes jQuery but has good points.

\$\endgroup\$

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.