sortabletable fixes, make ledger table sortable

app/extensions/brave/locales/en-US/preferences.properties

@@ -82,6 +82,7 @@ rank=Rank
 views=Views
 timeSpent=Time Spent
 include=Include
+percentage=%
 bravery=Bravery
 hintsTitle=Helpful hints
 hint0=The Bravery panel allows you turn HTTPS Everywhere on or off. HTTPS Everywhere automatically rewrites your HTTP traffic to HTTPS for supported sites to keep you more secure.

js/about/history.js

@@ -34,10 +34,15 @@ class HistoryDay extends ImmutableComponent {
     return <div>
       <div className='sectionTitle historyDayName'>{this.props.date}</div>
       <SortableTable headings={['time', 'title', 'domain']}
+        defaultHeading='time'
+        defaultHeadingSortOrder='desc'
         rows={this.props.entries.map((entry) => [
-          entry.get('lastAccessedTime')
-            ? new Date(entry.get('lastAccessedTime')).toLocaleTimeString()
-            : '',
+          {
+            html: entry.get('lastAccessedTime')
+              ? new Date(entry.get('lastAccessedTime')).toLocaleTimeString()
+              : '',
+            value: entry.get('lastAccessedTime')
+          },
           entry.get('customTitle') || entry.get('title')
             ? entry.get('customTitle') || entry.get('title')
             : entry.get('location'),

js/about/preferences.js

@@ -21,7 +21,6 @@ const getSetting = require('../settings').getSetting
 const SortableTable = require('../components/sortableTable')
 const Button = require('../components/button')
 const searchProviders = require('../data/searchProviders')
-const pad = require('underscore.string/pad')
 
 const adblock = appConfig.resourceNames.ADBLOCK
 const cookieblock = appConfig.resourceNames.COOKIEBLOCK
@@ -163,16 +162,16 @@ class SiteSettingCheckbox extends ImmutableComponent {
   }
 }
 
-class LedgerTableRow extends ImmutableComponent {
+class LedgerTable extends ImmutableComponent {
   get synopsis () {
-    return this.props.synopsis
+    return this.props.ledgerData.get('synopsis')
   }
 
-  get formattedTime () {
-    var d = this.synopsis.get('daysSpent')
-    var h = this.synopsis.get('hoursSpent')
-    var m = this.synopsis.get('minutesSpent')
-    var s = this.synopsis.get('secondsSpent')
+  getFormattedTime (synopsis) {
+    var d = synopsis.get('daysSpent')
+    var h = synopsis.get('hoursSpent')
+    var m = synopsis.get('minutesSpent')
+    var s = synopsis.get('secondsSpent')
     if (d << 0 > 364) {
       return '>1y'
     }
@@ -183,14 +182,12 @@ class LedgerTableRow extends ImmutableComponent {
     return (d + h + m + s + '')
   }
 
-  padLeft (v) { return pad(v, 12, '0') }
-
-  get hostPattern () {
-    return `https?://${this.synopsis.get('site')}`
+  getHostPattern (synopsis) {
+    return `https?://${synopsis.get('site')}`
   }
 
-  get enabled () {
-    const hostSettings = this.props.siteSettings.get(this.hostPattern)
+  enabledForSite (synopsis) {
+    const hostSettings = this.props.siteSettings.get(this.getHostPattern(synopsis))
     if (hostSettings) {
       const result = hostSettings.get('ledgerPayments')
       if (typeof result === 'boolean') {
@@ -200,54 +197,54 @@ class LedgerTableRow extends ImmutableComponent {
     return true
   }
 
-  render () {
-    const faviconURL = this.synopsis.get('faviconURL') || appConfig.defaultFavicon
-    const rank = this.synopsis.get('rank')
-    const views = this.synopsis.get('views')
-    const duration = this.synopsis.get('duration')
-    const publisherURL = this.synopsis.get('publisherURL')
-    // TODO: This should redistribute percentages accordingly when a site is
-    // enabled/disabled for payments.
-    const percentage = this.synopsis.get('percentage')
-    const site = this.synopsis.get('site')
+  getRow (synopsis) {
+    if (!synopsis || !synopsis.get) {
+      return []
+    }
+    const faviconURL = synopsis.get('faviconURL') || appConfig.defaultFavicon
+    const rank = synopsis.get('rank')
+    const views = synopsis.get('views')
+    const duration = synopsis.get('duration')
+    const publisherURL = synopsis.get('publisherURL')
+    const percentage = synopsis.get('percentage')
+    const site = synopsis.get('site')
     const defaultSiteSetting = true
 
-    return <tr className={this.enabled ? '' : 'paymentsDisabled'}>
-      <td className='alignRight' data-sort={this.padLeft(rank)}>{rank}</td>
-      <td><a href={publisherURL} target='_blank'><img src={faviconURL} alt={site} /><span>{site}</span></a></td>
-      <td><SiteSettingCheckbox hostPattern={this.hostPattern} defaultValue={defaultSiteSetting} prefKey='ledgerPayments' siteSettings={this.props.siteSettings} checked={this.enabled} /></td>
-      <td className='alignRight' data-sort={this.padLeft(views)}>{views}</td>
-      <td className='alignRight' data-sort={this.padLeft(duration)}>{this.formattedTime}</td>
-      <td className='alignRight' data-sort={this.padLeft(percentage)}>{percentage}</td>
-    </tr>
+    return [
+      rank,
+      {
+        html: <a href={publisherURL} target='_blank'><img src={faviconURL} alt={site} /><span>{site}</span></a>,
+        value: site
+      },
+      {
+        html: <SiteSettingCheckbox hostPattern={this.getHostPattern(synopsis)} defaultValue={defaultSiteSetting} prefKey='ledgerPayments' siteSettings={this.props.siteSettings} checked={this.enabledForSite(synopsis)} />,
+        value: this.enabledForSite(synopsis) ? 1 : 0
+      },
+      views,
+      {
+        html: this.getFormattedTime(synopsis),
+        value: duration
+      },
+      percentage
+    ]
   }
-}
 
-class LedgerTable extends ImmutableComponent {
   render () {
-    const synopsis = this.props.ledgerData.get('synopsis')
-    if (!synopsis || !synopsis.size) {
+    if (!this.synopsis || !this.synopsis.size) {
       return null
     }
     return <div id='ledgerTable'>
-      <table className='sort'>
-        <thead>
-          <tr>
-            <th className='sort-header' data-l10n-id='rank' />
-            <th className='sort-header' data-l10n-id='publisher' />
-            <th className='sort-header' data-l10n-id='include' />
-            <th className='sort-header' data-l10n-id='views' />
-            <th className='sort-header' data-l10n-id='timeSpent' />
-            <th className='sort-header'>&#37;</th>
-          </tr>
-        </thead>
-        <tbody>
-        {
-          synopsis.map((row) => <LedgerTableRow synopsis={row}
-            siteSettings={this.props.siteSettings} />)
+      <SortableTable
+        headings={['rank', 'publisher', 'include', 'views', 'timeSpent', 'percentage']}
+        defaultHeading='rank'
+        overrideDefaultStyle
+        columnClassNames={['alignRight', '', '', 'alignRight', 'alignRight', 'alignRight']}
+        rowClassNames={
+          this.synopsis.map((item) =>
+            this.enabledForSite(item) ? '' : 'paymentsDisabled').toJS()
         }
-        </tbody>
-      </table>
+        onContextMenu={aboutActions.contextMenu}
+        rows={this.synopsis.map((synopsis) => this.getRow(synopsis)).toJS()} />
     </div>
   }
 }
@@ -600,10 +597,20 @@ class SearchTab extends ImmutableComponent {
         display: 'inline-block',
         verticalAlign: 'middle'
       }
-      array.push([<SearchSelectEntry name={entry.name} settings={this.props.settings} />,
-        <SearchEntry name={entry.name} iconStyle={iconStyle}
-          onChangeSetting={this.props.onChangeSetting} />,
-        <SearchShortcutEntry shortcut={entry.shortcut} />])
+      array.push([
+        {
+          html: <SearchSelectEntry name={entry.name} settings={this.props.settings} />,
+          value: entry.name
+        },
+        {
+          html: <SearchEntry name={entry.name} iconStyle={iconStyle} onChangeSetting={this.props.onChangeSetting} />,
+          value: entry.name
+        },
+        {
+          html: <SearchShortcutEntry shortcut={entry.shortcut} />,
+          value: entry.shortcut
+        }
+      ])
     })
     return array
   }
@@ -616,6 +623,7 @@ class SearchTab extends ImmutableComponent {
     return <div>
       <div className='sectionTitle' data-l10n-id='searchSettings' />
       <SortableTable headings={['default', 'searchEngine', 'engineGoKey']} rows={this.searchProviders}
+        defaultHeading='searchEngine'
         addHoverClass onClick={this.hoverCallback.bind(this)} />
       <div className='sectionTitle' data-l10n-id='locationBarSettings' />
       <SettingsList>

js/components/sortableTable.js

@@ -5,14 +5,19 @@
 const React = require('react')
 const ImmutableComponent = require('./immutableComponent')
 const tableSort = require('tablesort')
+const cx = require('../lib/classSet')
 
-/**
- * Represents a sortable table with supp
- */
+tableSort.extend('number', (item) => {
+  return typeof item === 'number'
+}, (a, b) => {
+  a = isNaN(a) ? 0 : a
+  b = isNaN(b) ? 0 : b
+  return b - a
+})
 
 class SortableTable extends ImmutableComponent {
   componentDidMount (event) {
-    return tableSort(document.getElementsByClassName('sortableTable')[0])
+    return tableSort(this.table)
   }
   get hasClickHandler () {
     return typeof this.props.onClick === 'function'
@@ -21,23 +26,25 @@ class SortableTable extends ImmutableComponent {
     return this.props.columnClassNames &&
       this.props.columnClassNames.length === this.props.headings.length
   }
+  get hasRowClassNames () {
+    return this.props.rowClassNames &&
+      this.props.rowClassNames.length === this.props.rows.length
+  }
   get hasDoubleClickHandler () {
     return typeof this.props.onDoubleClick === 'function'
   }
   get hasContextMenu () {
     return typeof this.props.onContextMenu === 'function' &&
       typeof this.props.contextMenuName === 'string'
   }
-  getHandlerInput (rows, index) {
-    if (this.props.rowObjects) {
-      return typeof this.props.rowObjects[index].toJS === 'function'
-        ? this.props.rowObjects[index].toJS()
-        : this.props.rowObjects[index]
-    }
-    return rows[index]
-  }
-  getRowAttributes (handlerInput, index) {
+  getRowAttributes (row, index) {
     const rowAttributes = {}
+    const handlerInput = this.props.rowObjects
+      ? (typeof this.props.rowObjects[index].toJS === 'function'
+        ? this.props.rowObjects[index].toJS()
+        : this.props.rowObjects[index])
+      : row
+
     if (this.props.addHoverClass) {
       rowAttributes.className = 'rowHover'
     }
@@ -53,53 +60,51 @@ class SortableTable extends ImmutableComponent {
     return rowAttributes
   }
   render () {
-    let headings = []
-    let rows = []
-    let columnClassNames = []
-
     if (!this.props.headings || !this.props.rows) {
       return false
     }
 
-    if (this.hasColumnClassNames) {
-      this.props.columnClassNames.forEach((className) => columnClassNames.push(className))
-    }
-
-    for (let i = 0; i < this.props.rows.length; i++) {
-      rows[i] = []
-      for (let j = 0; j < this.props.headings.length; j++) {
-        headings[j] = headings[j] || <th className='sort-header' data-l10n-id={this.props.headings[j]} />
-        rows[i][j] = typeof columnClassNames[j] === 'string'
-          ? <td className={columnClassNames[j]} data-sort={this.props.rows[i][j]}>{this.props.rows[i][j] === true ? '✕' : this.props.rows[i][j]}</td>
-          : <td data-sort={this.props.rows[i][j]}>{this.props.rows[i][j] === true ? '✕' : this.props.rows[i][j]}</td>
-      }
-
-      const handlerInput = this.getHandlerInput(rows, i)
-      const rowAttributes = this.getRowAttributes(handlerInput, i)
-
-      rows[i] = rowAttributes.onContextMenu
-      ? <tr {...rowAttributes} data-context-menu-disable>{rows[i]}</tr>
-      : rows[i] = <tr {...rowAttributes}>{rows[i]}</tr>
-    }
-    return <table className='sortableTable sort'>
+    return <table className={cx({
+      sort: true,
+      sortableTable: !this.props.overrideDefaultStyle
+    })}
+      ref={(node) => { this.table = node }}>
       <thead>
         <tr>
-          {headings}
+          {this.props.headings.map((heading, j) => {
+            const firstEntry = this.props.rows[0][j]
+            let dataType = typeof firstEntry
+            if (dataType === 'object' && firstEntry.value) {
+              dataType = typeof firstEntry.value
+            }
+            return <th className={cx({
+              'sort-header': true,
+              'sort-default': heading === this.props.defaultHeading})}
+              data-l10n-id={heading}
+              data-sort-method={dataType === 'number' ? 'number' : undefined}
+              data-sort-order={this.props.defaultHeadingSortOrder} />
+          })}
         </tr>
       </thead>
       <tbody>
-        {rows}
+        {
+          this.props.rows.map((row, i) => {
+            const entry = row.map((item, j) => {
+              const value = typeof item === 'object' ? item.value : item
+              const html = typeof item === 'object' ? item.html : item
+              return <td className={this.hasColumnClassNames ? this.props.columnClassNames[j] : undefined} data-sort={value}>
+                {value === true ? '✕' : html}
+              </td>
+            })
+            const rowAttributes = this.getRowAttributes(row, i)
+            return <tr {...rowAttributes}
+              data-context-menu-disable={rowAttributes.onContextMenu ? true : undefined}
+              className={this.hasRowClassNames ? this.props.rowClassNames[i] : undefined}>{entry}</tr>
+          })
+        }
       </tbody>
     </table>
   }
 }
 
-SortableTable.defaultProps = {
-  headings: React.PropTypes.array.isRequired,
-  rows: React.PropTypes.array.isRequired,
-  columnClassNames: React.PropTypes.array,
-  addHoverClass: React.PropTypes.bool,
-  contextMenuName: React.PropTypes.string
-}
-
 module.exports = SortableTable

less/sortableTable.less

@@ -4,6 +4,18 @@
 
 @import "variables.less";
 
+table.sort {
+  th {
+    cursor: pointer;
+    text-decoration: underline;
+    color: @darkGray;
+
+    &:hover {
+      color: #000;
+    }
+  }
+}
+
 table.sortableTable {
   width: 100%;
   border: solid 1px @lightGray;
@@ -17,7 +29,6 @@ table.sortableTable {
 
     th {
       background: @lightGray;
-      color: @darkGray;
       text-align: left;
       font-weight: 300;
       padding: 8px;

package.json

@@ -106,7 +106,6 @@
     "tldjs": "1.6.2",
     "tracking-protection": "1.1.x",
     "underscore": "1.8.3",
-    "underscore.string": "^3.3.4",
     "url-loader": "^0.5.7"
   },
   "devDependencies": {