In the long term, replace jQuery and plugins with Vue

[philipbelesky]

Currently we use jQuery for basic tasks, and a bunch of its plugins for specialist tasks (datatables, form validation, drag and drop).

• [x] New drag/drop interfaces will be done using Vue, given its greater flexibility and maintainability.
• [x] Ideally our tables would be rendered using Vue also, given datatables is overkill for our needs, requires costly template loops, and is difficult to modularise the templates for. Replacing these completely would be a big effort, but a good starting point would be for the public interface.
• [ ] To the extent we use bootstrap plugins these can be supplanted with Vue Strap
• [ ] Form validation could be easily replaced with a Vue library (and at the same time expanded to cover all forms; not just ballots)
• [ ] At that point we don't have any need to include jQuery

[philipbelesky]

Keeping track:

• [ ] AdjAlloc
• [ ] AdjFeedback
• [ ] Availability
• [ ] Breakqual
• [ ] Draw
• [ ] Breakqual
• [X] Motions
• [x] Participants
• [X] Results
• [X] Standings
• [ ] Venues

[philipbelesky]

Completed/merged for all public-facing tables (and their admin equivalents when shared). Quick notes:

• If a template is just a heading and a table then in most cases it's template.html file has been deleted; instead subclass HeadlessTemplateView and provide a page_title and page_emoji.
• Views creating Vue tables subclass VueTableMixin and provide the template with the table's data by kwargs["tableData"] = json.dumps(motions_data)

• In the above example motions_data is an array of all the table rows. Each row is an array of all the cells. Each cell takes the form of:

  {
    'head': {
          'key': 'Aff Vetoes'
     },
    'cell': {
          'text': motion.aff_vetoes
    }
  }

• The cell dict can optionally have a sort, icon, emoji, tooltip, and link values.
• The head dict can optionally have a tooltip, icon, visible-sm, visible-md, and visible-lg values
• In theory it would be nice to not be setting the head of each cell and instead just provide a one-off array of all the header values, but in practice this is kind of a pain and combining them ends up producing less boilerplate code.

• VueTableMixin provides methods for constructing standard sets of rows, such as adjudicators_cells(), team_cells(), or venue_cells(). If more than one table is creating the same series of rows then there should be a function for it within the app or in VueTableMixin (if its general purpose). These are generally used when looping through a data set to build an array of rows:

  row_array.extend(self.team_cells(team, tournament))

• I haven't done extensive testing but the pages seem to be loading faster and using fewer queries.
• The above structure probably still has plenty of opportunities for further abstraction; given this is a major refactor of a lot of views I'm taking a walk-before-running kinda approach
• There are some minor front-end regressions: table headers wont remain fixed in place, and only one sorting order can apply at a time. These are on my todo list.

[czlee]

Awesome! With abstraction, as an easy start: Given that the table data should be added to the context, should we just make it so that the table is constructed in a (new) method get_table_data(), and then in most cases get_context_data() wouldn't need to be overridden? This mirrors how Django implements things like FormMixin, where the form is provided by get_form() and the base implementation of get_context_data() runs kwargs["form"] = self.get_form().

[czlee]

Also it's cool how you've merged a bunch of cells, so that e.g. emoji aren't in a cell to themselves anymore :trophy:

[philipbelesky]

Yea, I'm trying to prevent the tables from blowing out horizontally as a kind of stop-gap until I can figure out a proper responsive solution.

Having a get_table_data() makes total sense, should have thought of that. That function should also probably set the sort_key kwarg if available. Actually just thinking of it, tableData should be an array of tables, so that base_vue_table.html can support say 1-3 tables without the need for base_double_vue_table.html. Did you want to do the refactor or should I?

I also need to figure out Vue's component inheritance/hierarchy so that particular cells can use custom components, say a Feedback Graph or Drag and Drop area but that probably won't break existing python code — worst case scenario its a new custom field for the cell.

[czlee]

Either way, I'm happy to do it over the next few days, but if you want to do it first that's also cool. If you want it to be able to support an arbitrary number of tables, one way is to have the base class call get_tables_data() (mind the plural), whose default implementation is just return [self.get_table_data()], then most classes will just implement get_table_data().

I've also been thinking about an elegant way to get rid of the need to include the 'head' entry in every cell (without blowing out boilerplate code). I agree that it's difficult. Possibly the header row should just be a method in its own right (get_header_data()), but then there's the question of how to get it to work with multiple tables.

[philipbelesky]

I have most of today off so might go through and do it now.

In terms of the header data, my problem with splitting it up is that in many cases the header values aren't known until the cells are processed — for example when checking for preferences, or which ranking metrics apply. If the header method was split off, a lot of those checks would then end up being repeated. So it would be nicer conceptually, but involve more code. That is unless the functions remained unified, but returned a tuple that would extend header and data arrays. Except then you're still regenerating headers each time cell data is processed, which doesn't seem like much of a win.

[czlee]

What if we generated tables by column rather than by row? So for example, rather than

table = []
speakers = Speaker.objects.filter(team__tournament=tournament)
for speaker in speakers:
    row = self.speaker_cells(speaker, tournament)
    row.append(self.team_cells(speaker.team, tournament))
    table.append(row)
return json.dumps(table)

we had

speakers = Speaker.objects.filter(team__tournament=tournament)
table = [[] for i in len(speakers)]
self.add_speaker_columns(table, speakers, tournament)
self.add_team_columns(table, [s.team for s in speakers], tournament)
return json.dumps(table)

If we really wanted to make it tidy, we could write a simple class TableData that kept track of annoying bookkeeping, had a method json() that took care of the JSONifying, then:

speakers = Speaker.objects.filter(team__tournament=tournament)
table = TableData()      # get rid of that annoying "for i in len(speakers)"
table.add_speaker_columns(speakers, tournament)      # note that this now can't access self
table.add_team_columns(table, [s.team for s in speakers], tournament)
return table.json()

then we could even change around how TableData stores info internally (e.g. for efficiency, or to change the JSON‒Vue interface, or maybe to get even fancier with customization) without having to touch existing code :wink: (at least ideally… I find in practice this only works about half the time).

In fact, if all tables are constructed then returned as JSON, then get_table_data() could just return the TableData instance:

def get_table_data(self):
    speakers = Speaker.objects.filter(team__tournament=tournament)
    table = TableData()      # get rid of that annoying "for i in len(speakers)"
    table.add_speaker_columns(speakers, tournament)      # note that this now can't access self
    table.add_team_columns(table, [s.team for s in speakers], tournament)
    return table

def get_context_data(self, **kwargs):
    kwargs["tableData"] = self.get_table_data().json()
    return super().get_context_data(**kwargs)

The downside is that you then have to construct relevant inputs like [s.team for s in speakers] in one go before passing the entire team list to the column generator. Some people might think this is more Pythonic, and I'm sufficiently used to it that it seems quite straightforward (and even tidier) to me, but I'll admit it probably ties up memory unnecessarily.

It's also not clear whether it would make other code, which actually has to run the generation, better or worse:

# in BaseTeamStandingsView
def get_table_data(self):
    standings = # code to generate standings
    table = TableData()
    for ranking_info in standings.ranking_info: # ranking_info is a dict
        table.add_standing_column(standings, ranking_info) # replaces format_iterators(), see below
    table.add_team_columns([s.team for s in standings], tournament)

    headers = [{'key': round.abbreviation} for round in rounds]
    data = []
    for standing in standings:
        cells = []
        for team_score in standing.round_results:
            cell = {'text': ''}
            if team_score.win:
                cell['icon'] = "glyphicon-arrow-up text-success"
                cell['tooltip'] = "Won against "
            else:
                cell['icon'] = "glyphicon-arrow-up text-danger"
                cell['tooltip'] = "Lost to "

            cell['text'] += "vs " + team_score.opposition.emoji + "<br>" + self.format_cell_number(team_score.score)
            cell['tooltip'] += team_score.opposition.short_name + " and received " + self.format_cell_number(team_score.score) + " total speaks"
            cells.append(cell)
        data.append(cells)
    table.add_columns(headers, data)

    # then add metrics

    return table

# in TableData
def add_ranking_column(self, standings, info): # showing just ranking for demo, would write to cover metrics too
    header = {'key': info['abbr'], 'tooltip': info['name']}
    if hasattr(info, 'glyphicon'):
        header['icon'] = info['glyphicon']
    data = []
    for standing in standings:
        rank, equal = standing.rankings[info['key']]
        if rank is None:
            cell = {'text': 'N/A', 'sort': 99999}
        else:
            cell = {
                'text': str(rank) + ('=' if equal else ''),
                'sort': rank
            }
        data.append(cell)
    self.add_column(header, data)

(compare: https://github.com/czlee/tabbycat/blob/30a2723b4e0d510dfeaa2d6f52554da38db93689/tabbycat/standings/views.py#L284-L312)

Any thoughts? Just throwing ideas around atm, I can think about this more rigorously some time (though I seem to have got carried away with this idea already).

[philipbelesky]

Hmm, generating via columns does allow for separating the headers elegantly. I also would probably prefer to run the loops when passing values to the function rather than having a bunch of block level loops. Memory usage (hopefully) isn't much of a concern.

The new Table Constructor class could also then reverse things into a row-based structure (with seperate headers) when passing to the frontend — if just to prevent reworking the frontend code for now; possibly also for efficiency reasons. It might also be worth splitting it out the class from utils.py into its tables.py or similar; with specific apps perhaps sticking their own cell-generating functions as superclasses in similar files (there's a lot of big large blocks of row generation around). A large portion of Tabbycat is generating tables, so they should probably get their own file 😛

Did you want to create a branch and have a play at creating the new abstractions and maybe implementing them for one or two view functions? Once that implementation is all stable I can go through and convert the other views.

[czlee]

Sure, I can do that. Will have a first crack at it tonight, we'll see how it goes.

[czlee]

Can you think of any situation where a single table might care about two different tournaments in each of two different columns? (i.e., can we ditch the tournament argument to each method, in favor of one in the constructor?)

[philipbelesky]

In theory there could be some use case the compares adj performance or something across tournaments, but I don't think its worth making this interface more annoying to use in order to cater for that hypothetical.

[czlee]

Done! I converted a fair number of tables, but not all of them.

How hard would it be to add the ability to specify the default sort order (as well as the default key)? It'd be nice to be able to sort the "current team standings" page in descending order by number of wins by default.

[philipbelesky]

Wooo. Adding sorting order should be pretty easy, I'll add it to the todo list when I adapt the front end to the column based approach.

On 16 June 2016 at 08:28:47, Chuan-Zheng Lee (notifications@github.com) wrote:

Done! I converted a fair number of tables, but not all of them.

How hard would it be to add the ability to specify the default sort order (as well as the default key)? It'd be nice to be able to sort the "current team standings" page in descending order by number of wins by default.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/czlee/tabbycat/issues/297#issuecomment-226338986, or mute the thread https://github.com/notifications/unsubscribe/AAeRWeCXPL0V6DHB-3jxWGRyoxjCVyLWks5qMHyfgaJpZM4HmCwV .

[philipbelesky]

Closing as outside of #379 (and the validation plugin) the reward on removing jQuery as a dependency isn't high; and it would probably mean cutting off (easy) compatibility with future versions of Bootstrap.