Skip to content
This repository has been archived by the owner on Apr 17, 2023. It is now read-only.

Commit

Permalink
js: added table sortable mixin; namespaces using it
Browse files Browse the repository at this point in the history
So far the column sorting was possible but user wasn't able
to share the state of it to someone through copying and pasting url.

Now every sorting action the user makes on the namespaces table,
for example, will create a new url state that can be visited and
navigated back and forth.

Fixes #1373
  • Loading branch information
vitoravelino committed Aug 17, 2017
1 parent 06af252 commit 76909e9
Show file tree
Hide file tree
Showing 10 changed files with 214 additions and 41 deletions.
45 changes: 18 additions & 27 deletions app/assets/javascripts/modules/namespaces/components/table.js
Original file line number Diff line number Diff line change
@@ -1,17 +1,26 @@
import Vue from 'vue';
import getProperty from 'lodash/get';

import Comparator from '~/utils/comparator';

import TablePagination from '~/shared/components/table-pagination';
import TableSortableMixin from '~/shared/mixins/table-sortable';

import NamespaceTableRow from './table-row';

const { set } = Vue;

export default {
template: '#js-namespaces-table-tmpl',

props: ['namespaces', 'sortable'],
props: {
namespaces: {
type: Array,
},
prefix: {
type: String,
default: 'ns_',
},
},

mixins: [TableSortableMixin],

components: {
NamespaceTableRow,
Expand All @@ -20,8 +29,6 @@ export default {

data() {
return {
sortAsc: true,
sortBy: 'attributes.clean_name',
limit: 3,
currentPage: 1,
};
Expand All @@ -33,16 +40,16 @@ export default {
},

filteredNamespaces() {
const order = this.sortAsc ? 1 : -1;
const order = this.sorting.asc ? 1 : -1;
const sortedNamespaces = [...this.namespaces];
const sample = sortedNamespaces[0];
const value = getProperty(sample, this.sortBy);
const value = getProperty(sample, this.sorting.by);
const comparator = Comparator.of(value);

// sorting
sortedNamespaces.sort((a, b) => {
const aValue = getProperty(a, this.sortBy);
const bValue = getProperty(b, this.sortBy);
const aValue = getProperty(a, this.sorting.by);
const bValue = getProperty(b, this.sorting.by);

return order * comparator(aValue, bValue);
});
Expand All @@ -54,21 +61,5 @@ export default {
},
},

methods: {
sort(attribute) {
if (!this.sortable) {
return;
}

// if sort column has changed, go always asc
// inverse current order otherwise
if (this.sortBy === attribute) {
set(this, 'sortAsc', !this.sortAsc);
} else {
set(this, 'sortAsc', true);
}

set(this, 'sortBy', attribute);
},
},

};
78 changes: 78 additions & 0 deletions app/assets/javascripts/shared/mixins/table-sortable.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
import Vue from 'vue';

import queryString from 'query-string';

const { set } = Vue;

export default {
props: {
sortable: {
type: Boolean,
default: true,
},
sortBy: {
type: String,
required: true,
},
// Seems stupid to set it as String but I wanted
// to have the type equal to the query string.
// I can handle is the same way as the query string.
sortAsc: {
type: String,
default: 'true',
},
},

data() {
return {
sorting: {
by: this.sortBy,
asc: this.sortAsc,
},
};
},

beforeMount() {
if (!this.sortable) {
return;
}

const queryObject = queryString.parse(window.location.search);
const sortByQuery = queryObject[this.prefix + 'sort_by'] || this.sorting.by;
const sortAscQuery = (queryObject[this.prefix + 'sort_asc'] || this.sorting.asc) === 'true';

set(this.sorting, 'by', sortByQuery);
set(this.sorting, 'asc', sortAscQuery);
},

methods: {
sort(attribute) {
if (!this.sortable) {
return;
}

// if sort column has changed, go always asc
// inverse current order otherwise
if (this.sorting.by === attribute) {
set(this.sorting, 'asc', !this.sorting.asc);
} else {
set(this.sorting, 'asc', true);
}

set(this.sorting, 'by', attribute);

this.updateUrlState();
},

updateUrlState() {
const queryObject = queryString.parse(window.location.search);

queryObject[this.prefix + 'sort_asc'] = this.sorting.asc;
queryObject[this.prefix + 'sort_by'] = this.sorting.by;

const queryParams = queryString.stringify(queryObject);
const url = [location.protocol, '//', location.host, location.pathname].join('');
history.pushState('', '', `${url}?${queryParams}`);
},
},
};
13 changes: 12 additions & 1 deletion app/assets/javascripts/vue-shared.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ Vue.http.interceptors.push((_request, next) => {
});
});


Vue.http.interceptors.push((request, next) => {
if ($.rails) {
// eslint-disable-next-line no-param-reassign
Expand All @@ -21,4 +20,16 @@ Vue.http.interceptors.push((request, next) => {
next();
});

// we are not a SPA and when user clicks on back/forward
// we want the page to be fully reloaded to take advantage of
// the url query params state
window.onpopstate = function (e) {
// phantomjs seems to trigger an oppopstate event
// when visiting pages, e.state is always null and
// in our component we set an empty string
if (e.state !== null) {
window.location.reload();
}
};

Vue.config.productionTip = process.env.NODE_ENV !== 'production';
4 changes: 2 additions & 2 deletions app/views/namespaces/components/_panel.html.slim
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
.panel.panel-default
.namespaces-panel.panel.panel-default
.panel-heading
.row
.col-sm-6
Expand All @@ -11,4 +11,4 @@

.table-responsive
<namespaces-not-loaded v-if="state.notLoaded"></namespaces-not-loaded>
<namespaces-table v-if="!state.isLoading && !state.notLoaded" :namespaces="namespaces" :sortable="tableSortable"></namespaces-table>
<namespaces-table v-if="!state.isLoading && !state.notLoaded" :namespaces="namespaces" :sortable="tableSortable" sort-by="attributes.clean_name"></namespaces-table>
20 changes: 10 additions & 10 deletions app/views/namespaces/components/_table.html.slim
Original file line number Diff line number Diff line change
Expand Up @@ -11,40 +11,40 @@ div
th @click=="sort('attributes.clean_name')"
i.fa.fa-fw.fa-sort{
:class=="{
'fa-sort-amount-asc': sortBy === 'attributes.clean_name' && sortAsc,
'fa-sort-amount-desc': sortBy === 'attributes.clean_name' && !sortAsc,
'fa-sort-amount-asc': sorting.by === 'attributes.clean_name' && sorting.asc,
'fa-sort-amount-desc': sorting.by === 'attributes.clean_name' && !sorting.asc,
}"
}
| Name
th @click=="sort('relationships.repositories.meta.count')"
i.fa.fa-fw.fa-sort{
:class=="{
'fa-sort-amount-asc': sortBy === 'relationships.repositories.meta.count' && sortAsc,
'fa-sort-amount-desc': sortBy === 'relationships.repositories.meta.count' && !sortAsc,
'fa-sort-amount-asc': sorting.by === 'relationships.repositories.meta.count' && sorting.asc,
'fa-sort-amount-desc': sorting.by === 'relationships.repositories.meta.count' && !sorting.asc,
}"
}
| Repositories
th @click=="sort('relationships.webhooks.meta.count')"
i.fa.fa-fw.fa-sort{
:class=="{
'fa-sort-amount-asc': sortBy === 'relationships.webhooks.meta.count' && sortAsc,
'fa-sort-amount-desc': sortBy === 'relationships.webhooks.meta.count' && !sortAsc,
'fa-sort-amount-asc': sorting.by === 'relationships.webhooks.meta.count' && sorting.asc,
'fa-sort-amount-desc': sorting.by === 'relationships.webhooks.meta.count' && !sorting.asc,
}"
}
| Webhooks
th @click=="sort('attributes.created_at')"
i.fa.fa-fw.fa-sort{
:class=="{
'fa-sort-amount-asc': sortBy === 'attributes.created_at' && sortAsc,
'fa-sort-amount-desc': sortBy === 'attributes.created_at' && !sortAsc,
'fa-sort-amount-asc': sorting.by === 'attributes.created_at' && sorting.asc,
'fa-sort-amount-desc': sorting.by === 'attributes.created_at' && !sorting.asc,
}"
}
| Created at
th @click=="sort('attributes.visibility')"
i.fa.fa-fw.fa-sort{
:class=="{
'fa-sort-amount-asc': sortBy === 'attributes.visibility' && sortAsc,
'fa-sort-amount-desc': sortBy === 'attributes.visibility' && !sortAsc,
'fa-sort-amount-asc': sorting.by === 'attributes.visibility' && sorting.asc,
'fa-sort-amount-desc': sorting.by === 'attributes.visibility' && !sorting.asc,
}"
}
| Visibility
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<namespaces-panel :state="state" :namespaces="specialNamespaces">
<namespaces-panel :namespaces="specialNamespaces">
h5 slot="name"
a[data-placement="right"
data-toggle="popover"
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
"jquery-ujs": "^1.2.1",
"lodash": "^4.17.4",
"moment": "^2.18.1",
"query-string": "^5.0.0",
"stats-webpack-plugin": "^0.4.3",
"typeahead.js": "^0.11.1",
"vue": "^2.3.3",
Expand Down
42 changes: 42 additions & 0 deletions spec/features/namespaces_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,48 @@
namespace = Namespace.find(id)
expect(namespace.visibility_public?).to be true
end

scenario "Namespace table sorting is reachable through url", js: true do
# sort asc
visit namespaces_path(ns_sort_asc: true)

expect(page).to have_css(".fa-sort-amount-asc")

# sort desc
visit namespaces_path(ns_sort_asc: false)

expect(page).to have_css(".fa-sort-amount-desc")

# sort asc & created_at
visit namespaces_path(ns_sort_asc: true, ns_sort_by: "attributes.created_at")

expect(page).to have_css("th:nth-child(4) .fa-sort-amount-asc")

# sort desc & created_at
visit namespaces_path(ns_sort_asc: false, ns_sort_by: "attributes.created_at")

expect(page).to have_css("th:nth-child(4) .fa-sort-amount-desc")
end

scenario "URL is updated when namespaces column is sorted", js: true do
visit namespaces_path

expect(page).to have_css(".namespaces-panel:last-of-type th:nth-child(4)")

# sort asc & created_at
find(".namespaces-panel:last-of-type th:nth-child(4)").click

expect(page).to have_css(".namespaces-panel th:nth-child(4) .fa-sort-amount-asc")
path = namespaces_path(ns_sort_asc: true, ns_sort_by: "attributes.created_at")
expect(page).to have_current_path(path)

# sort desc & created_at
find(".namespaces-panel:last-of-type th:nth-child(4)").click

expect(page).to have_css(".namespaces-panel th:nth-child(4) .fa-sort-amount-desc")
path = namespaces_path(ns_sort_asc: false, ns_sort_by: "attributes.created_at")
expect(page).to have_current_path(path)
end
end

describe "#update" do
Expand Down
38 changes: 38 additions & 0 deletions spec/features/teams_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,44 @@
expect(page).to have_current_path(namespace_path(namespace))
end

scenario "Namespace table sorting is reachable through url", js: true do
# sort asc
visit team_path(team, ns_sort_asc: true)

expect(page).to have_css(".namespaces-panel .fa-sort-amount-asc")

# sort desc
visit team_path(team, ns_sort_asc: false)

expect(page).to have_css(".namespaces-panel .fa-sort-amount-desc")

# sort asc & created_at
visit team_path(team, ns_sort_asc: true, ns_sort_by: "attributes.created_at")

expect(page).to have_css(".namespaces-panel th:nth-child(4) .fa-sort-amount-asc")

# sort desc & created_at
visit team_path(team, ns_sort_asc: false, ns_sort_by: "attributes.created_at")

expect(page).to have_css(".namespaces-panel th:nth-child(4) .fa-sort-amount-desc")
end

scenario "URL is updated when namespaces column is sorted", js: true do
# sort asc & created_at
find(".namespaces-panel th:nth-child(4)").click

expect(page).to have_css(".namespaces-panel th:nth-child(4) .fa-sort-amount-asc")
path = team_path(team, ns_sort_asc: true, ns_sort_by: "attributes.created_at")
expect(page).to have_current_path(path)

# sort desc & created_at
find(".namespaces-panel th:nth-child(4)").click

expect(page).to have_css(".namespaces-panel th:nth-child(4) .fa-sort-amount-desc")
path = team_path(team, ns_sort_asc: false, ns_sort_by: "attributes.created_at")
expect(page).to have_current_path(path)
end

scenario "An user can be added as a team member", js: true do
find("#add_team_user_btn").click
find("#team_user_role").select "Contributor"
Expand Down
12 changes: 12 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1247,6 +1247,10 @@ decamelize@^1.0.0, decamelize@^1.1.1, decamelize@^1.1.2:
version "1.2.0"
resolved "https://registry.yarnpkg.com/decamelize/-/decamelize-1.2.0.tgz#f6534d15148269b20352e7bee26f501f9a191290"

decode-uri-component@^0.2.0:
version "0.2.0"
resolved "https://registry.yarnpkg.com/decode-uri-component/-/decode-uri-component-0.2.0.tgz#eb3913333458775cb84cd1a1fae062106bb87545"

deep-extend@~0.4.0:
version "0.4.1"
resolved "https://registry.yarnpkg.com/deep-extend/-/deep-extend-0.4.1.tgz#efe4113d08085f4e6f9687759810f807469e2253"
Expand Down Expand Up @@ -3430,6 +3434,14 @@ query-string@^4.1.0:
object-assign "^4.1.0"
strict-uri-encode "^1.0.0"

query-string@^5.0.0:
version "5.0.0"
resolved "https://registry.yarnpkg.com/query-string/-/query-string-5.0.0.tgz#fbdf7004b4d2aff792f9871981b7a2794f555947"
dependencies:
decode-uri-component "^0.2.0"
object-assign "^4.1.0"
strict-uri-encode "^1.0.0"

querystring-es3@^0.2.0:
version "0.2.1"
resolved "https://registry.yarnpkg.com/querystring-es3/-/querystring-es3-0.2.1.tgz#9ec61f79049875707d69414596fd907a4d711e73"
Expand Down

0 comments on commit 76909e9

Please sign in to comment.