alanverbner / iohk-training Goto Github PK

Currently you re-generate the invoices data on every server restart, which is fine for this example app but it would be more elegant to only generate the data if it does not exist yet 😉

If you your app would rerender your component in various situations your $(window).scroll event listeners would not be cleaned up correctly!

In general the code looks good but in some cases it would be nice to cleanup the formatting and avoid splitting up code flow into too many functions unnecessarily.

One example are imports:

import {
  Invoices
} from '../api/invoices'

can and should be a simple one-liner:

import { Invoices } from '../api/invoices';

Here you split the code up into many small functions which is nice for big apis and when there is real complexity going on. But this component is so simple that this is less readable in the end!

function areAllLoaded( data ) {
  return data.total === data.loaded
}

function isIndicatorVisible( window, target ) {

  const threshold = window.scrollTop() + window.height() - target.height();
  return target.offset().top < threshold;
}

function tryLoadMoreData( instance ) {
  var target = instance.$( "#load-more" );
  if ( !target.length ) return;

  if ( isIndicatorVisible( $( window ), target ) && !areAllLoaded( instance.data ) ) {
    instance.data.onNeedMoreItems()
  }
}

Template.InfiniteScroll.onRendered( function onBodyCreated() {
  $( window ).scroll( () => {
    tryLoadMoreData( this )
  } )
} )

Template.InfiniteScroll.helpers( {

  allLoaded() {
    return areAllLoaded( this )
  },

  moreToLoad() {
    return !areAllLoaded( this )
  }
} )

you could pack this code into ~20-30 lines of code instead, as simple as possible 😉

Currently there is just the iron router error page showing up that there is no route defined for the URL path "/". Would be nice to add a redirect for that 😉

Currently you return the total count of all invoices but the client actually needs to know how many invoices there are for the current set of filter and sorting options! This means you have to use the same query for the total count too.