From 76909e9c931def78daa5dbf1516450d9ff9e086b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADtor=20Avelino?= Date: Thu, 17 Aug 2017 16:37:36 -0300 Subject: [PATCH] js: added table sortable mixin; namespaces using it 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 --- .../modules/namespaces/components/table.js | 45 +++++------ .../shared/mixins/table-sortable.js | 78 +++++++++++++++++++ app/assets/javascripts/vue-shared.js | 13 +++- .../namespaces/components/_panel.html.slim | 4 +- .../namespaces/components/_table.html.slim | 20 ++--- .../partials/_special_namespaces.html.slim | 2 +- package.json | 1 + spec/features/namespaces_spec.rb | 42 ++++++++++ spec/features/teams_spec.rb | 38 +++++++++ yarn.lock | 12 +++ 10 files changed, 214 insertions(+), 41 deletions(-) create mode 100644 app/assets/javascripts/shared/mixins/table-sortable.js diff --git a/app/assets/javascripts/modules/namespaces/components/table.js b/app/assets/javascripts/modules/namespaces/components/table.js index 683c88cb4..5dda6d37e 100644 --- a/app/assets/javascripts/modules/namespaces/components/table.js +++ b/app/assets/javascripts/modules/namespaces/components/table.js @@ -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, @@ -20,8 +29,6 @@ export default { data() { return { - sortAsc: true, - sortBy: 'attributes.clean_name', limit: 3, currentPage: 1, }; @@ -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); }); @@ -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); - }, - }, + }; diff --git a/app/assets/javascripts/shared/mixins/table-sortable.js b/app/assets/javascripts/shared/mixins/table-sortable.js new file mode 100644 index 000000000..bbab81a52 --- /dev/null +++ b/app/assets/javascripts/shared/mixins/table-sortable.js @@ -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}`); + }, + }, +}; diff --git a/app/assets/javascripts/vue-shared.js b/app/assets/javascripts/vue-shared.js index e81f8651f..6cbb9556b 100644 --- a/app/assets/javascripts/vue-shared.js +++ b/app/assets/javascripts/vue-shared.js @@ -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 @@ -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'; diff --git a/app/views/namespaces/components/_panel.html.slim b/app/views/namespaces/components/_panel.html.slim index ce185af15..d04eaa5c1 100644 --- a/app/views/namespaces/components/_panel.html.slim +++ b/app/views/namespaces/components/_panel.html.slim @@ -1,4 +1,4 @@ -.panel.panel-default +.namespaces-panel.panel.panel-default .panel-heading .row .col-sm-6 @@ -11,4 +11,4 @@ .table-responsive - + diff --git a/app/views/namespaces/components/_table.html.slim b/app/views/namespaces/components/_table.html.slim index 1ed1e03c7..91a275676 100644 --- a/app/views/namespaces/components/_table.html.slim +++ b/app/views/namespaces/components/_table.html.slim @@ -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 diff --git a/app/views/namespaces/partials/_special_namespaces.html.slim b/app/views/namespaces/partials/_special_namespaces.html.slim index 58969d427..0f9e22dad 100644 --- a/app/views/namespaces/partials/_special_namespaces.html.slim +++ b/app/views/namespaces/partials/_special_namespaces.html.slim @@ -1,4 +1,4 @@ - + h5 slot="name" a[data-placement="right" data-toggle="popover" diff --git a/package.json b/package.json index 3e24869bb..df5716d7a 100644 --- a/package.json +++ b/package.json @@ -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", diff --git a/spec/features/namespaces_spec.rb b/spec/features/namespaces_spec.rb index e2242973e..956b1b4bd 100644 --- a/spec/features/namespaces_spec.rb +++ b/spec/features/namespaces_spec.rb @@ -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 diff --git a/spec/features/teams_spec.rb b/spec/features/teams_spec.rb index bd638507f..bad91db9a 100644 --- a/spec/features/teams_spec.rb +++ b/spec/features/teams_spec.rb @@ -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" diff --git a/yarn.lock b/yarn.lock index 552867919..e3d117801 100644 --- a/yarn.lock +++ b/yarn.lock @@ -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" @@ -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"