Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Datatable helper #846

Merged
merged 38 commits into from
Jul 19, 2018
Merged

Datatable helper #846

merged 38 commits into from
Jul 19, 2018

Conversation

Stanton
Copy link
Member

@Stanton Stanton commented Jul 3, 2018

Creates a new html.datatable() helper to easily create fully marked up datatables based on the normal JSON data object that would also work for DTs ajax methods.

The helper will use the horizontal scrolling overflow method by default, it can also be set to collapse by setting the 'overflow': 'collapse' option.

Column attributes can be set through the attrs option.

Basic usage

{{ 
    html.datatable({ 
        'data': data
    }) 
}}

Data example

{
    'columns': [
        { 'title': 'Title' },
        { 'title': 'Owner' },
        { 'title': 'Modified' },
        {
            'title': 'Live',
            'attrs': {
                'data-orderable': 'false',
                'class': 'u-text-align-center u-shrink-to-fit'
            }
        },
        {
            'title': 'Visible',
            'attrs': {
                'data-orderable': 'false',
                'class': 'u-text-align-center u-shrink-to-fit'
            }
        }
    ],
    'data': [
        {
            'Title': 'The Terminator',
            'Owner': 'James Cameron',
            'Modified': '1991-07-01 12:34',
            'Live': true,
            'Visible': false 
        },
        {
            'Title': 'Conan the Barbarian',
            'Owner': 'John Millius',
            'Modified': '1982-04-02 12:34',
            'Live': false,
            'Visible': true 
        }
    ]
}

Stanton and others added 30 commits June 13, 2018 16:53
Also adds new row-actions dropdown pattern
# Only the main Sass file needs front matter (the dashes are enough)
---

$blue: #28a8e0;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Invalid CSS after "$blue: #28a8e0": expected "{", was ";"

}

pre .deletion {
background-color: #FFC8BD;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Color #FFC8BD should be written as #ffc8bd

}

pre .addition {
background-color: #BAEEBA;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Color #BAEEBA should be written as #baeeba

}

pre .diff .change {
background-color: #BCCFF9;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Color #BCCFF9 should be written as #bccff9

}

pre .tex .formula {
background-color: #EEE;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Color #EEE should be written as #eee

color: #000;
}

pre .css .class, pre .css .id {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Each selector in a comma sequence should be on its own single line


pre .variable {
color: rgb(63,110,116);
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rule declaration should be followed by an empty line

}

pre .variable {
color: rgb(63,110,116);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commas in function arguments should be followed by one space

pre .setting,
pre .params,
pre .clojure .attribute {
color: rgb(92,38,153);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commas in function arguments should be followed by one space

color: rgb(28,0,207);
}

pre .class .title,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Selector built_in should be written in hyphenated BEM (Block Element Modifier) format

@codecov-io
Copy link

codecov-io commented Jul 12, 2018

Codecov Report

Merging #846 into develop will decrease coverage by 1.31%.
The diff coverage is 23.07%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #846      +/-   ##
===========================================
- Coverage    97.36%   96.05%   -1.32%     
===========================================
  Files           56       56              
  Lines         1973     2002      +29     
===========================================
+ Hits          1921     1923       +2     
- Misses          52       79      +27
Impacted Files Coverage Δ
js/PulsarUIComponent.js 50.94% <23.07%> (-16.59%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eb5d628...61fac93. Read the comment docs.

});


$this.on('click', '.js-select-all', function(e) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really minor (and something we should setup a linter for) we are inconsistent with function () vs function(), I prefer the former.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something to add to the linter?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to add the linter first 😄

$allCheckboxes = $this.find('.js-select');

if ($checkbox.hasClass('selected')) {
table.rows( { search: 'applied' } ).deselect()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

semicolon

$checkbox.removeClass('selected');
$allCheckboxes.removeClass('selected').prop('checked', false);
} else {
table.rows( { search: 'applied' } ).select()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

semicolon

&.is-offline {
color: #c84d40;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should these be variables?

@@ -7,6 +7,8 @@ var $ = require('jquery'),
tab = require('../../../js/libs/tab'),
PulsarUIComponent = require('../../../js/PulsarUIComponent');

$.fx.off = !$.fx.off;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we explicitly set this to true or false? Toggling the value assuming it will always be one or the other seems a bit risky.

@@ -41,7 +41,7 @@ describe('Pulsar UI Component', function() {

this.history = {
pushState: sinon.stub()
};
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should have a semicolon shouldn't it?

@@ -0,0 +1,5 @@
source 'https://rubygems.org'

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style/FrozenStringLiteralComment: Missing magic comment # frozen_string_literal: true.

Copy link
Collaborator

@jamesjacobs jamesjacobs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's all the list.js stuff added to libs?

@Stanton Stanton merged commit 4f5d54c into develop Jul 19, 2018
@Stanton Stanton deleted the datatable-helper branch July 19, 2018 10:12
@Stanton Stanton added this to the 8.0.x milestone Sep 17, 2018
Stanton added a commit that referenced this pull request Jan 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants