From d88e7efd04e2613e8f29df5daad97144278849ad Mon Sep 17 00:00:00 2001 From: Kevin Ansfield Date: Wed, 7 Nov 2018 15:06:28 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=8E=A8=20Changed=20tags=20list,=20filter?= =?UTF-8?q?=20dropdown,=20and=20select=20input=20to=20sort=20alphabeticall?= =?UTF-8?q?y=20(#1066)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit no issue - switch tags sorting from ID based to alphabetical - pre-requisite to nested tags sorting --- .../admin/app/components/gh-psm-tags-input.js | 11 +++++--- ghost/admin/app/controllers/posts.js | 4 ++- ghost/admin/app/controllers/settings/tags.js | 17 +++--------- .../admin/app/templates/components/gh-tag.hbs | 12 ++++----- ghost/admin/tests/acceptance/content-test.js | 17 ++++++++++++ .../tests/acceptance/settings/tags-test.js | 26 +++++++++++++++++-- .../components/gh-psm-tags-input-test.js | 6 ++--- 7 files changed, 65 insertions(+), 28 deletions(-) diff --git a/ghost/admin/app/components/gh-psm-tags-input.js b/ghost/admin/app/components/gh-psm-tags-input.js index 41cb5b90ab..74b75d608a 100644 --- a/ghost/admin/app/components/gh-psm-tags-input.js +++ b/ghost/admin/app/components/gh-psm-tags-input.js @@ -1,6 +1,7 @@ import Component from '@ember/component'; import {computed} from '@ember/object'; import {inject as service} from '@ember/service'; +import {sort} from '@ember/object/computed'; export default Component.extend({ @@ -11,10 +12,14 @@ export default Component.extend({ tagName: '', // internal attrs - availableTags: null, + _availableTags: null, + + availableTags: sort('_availableTags.[]', function (tagA, tagB) { + return tagA.name.localeCompare(tagB.name); + }), availableTagNames: computed('availableTags.@each.name', function () { - return this.get('availableTags').map(tag => tag.get('name').toLowerCase()); + return this.availableTags.map(tag => tag.name.toLowerCase()); }), init() { @@ -23,7 +28,7 @@ export default Component.extend({ // to a live-query that will be immediately populated with what's in the // store and be updated when the above query returns this.store.query('tag', {limit: 'all'}); - this.set('availableTags', this.store.peekAll('tag')); + this.set('_availableTags', this.store.peekAll('tag')); }, actions: { diff --git a/ghost/admin/app/controllers/posts.js b/ghost/admin/app/controllers/posts.js index 8e0a1187e7..81da668e1f 100644 --- a/ghost/admin/app/controllers/posts.js +++ b/ghost/admin/app/controllers/posts.js @@ -79,7 +79,9 @@ export default Controller.extend({ }), availableTags: computed('_availableTags.[]', function () { - let tags = this.get('_availableTags').filter(tag => tag.get('id') !== null); + let tags = this.get('_availableTags') + .filter(tag => tag.get('id') !== null) + .sort((tagA, tagB) => tagA.name.localeCompare(tagB.name)); let options = tags.toArray(); options.unshiftObject({name: 'All tags', slug: null}); diff --git a/ghost/admin/app/controllers/settings/tags.js b/ghost/admin/app/controllers/settings/tags.js index 9d7d809967..044fdeed44 100644 --- a/ghost/admin/app/controllers/settings/tags.js +++ b/ghost/admin/app/controllers/settings/tags.js @@ -14,21 +14,12 @@ export default Controller.extend({ tagContentFocused: equal('keyboardFocus', 'tagContent'), filteredTags: computed('tags.@each.isNew', function () { - return this.get('tags').filterBy('isNew', false); + return this.tags.filterBy('isNew', false); }), - // TODO: replace with ordering by page count once supported by the API - sortedTags: sort('filteredTags', function (a, b) { - let idA = +a.get('id'); - let idB = +b.get('id'); - - if (idA > idB) { - return 1; - } else if (idA < idB) { - return -1; - } - - return 0; + // tags are sorted by name + sortedTags: sort('filteredTags', function (tagA, tagB) { + return tagA.name.localeCompare(tagB.name); }), actions: { diff --git a/ghost/admin/app/templates/components/gh-tag.hbs b/ghost/admin/app/templates/components/gh-tag.hbs index 3d88782f07..04ecd800bd 100644 --- a/ghost/admin/app/templates/components/gh-tag.hbs +++ b/ghost/admin/app/templates/components/gh-tag.hbs @@ -1,14 +1,14 @@ -
+
{{#link-to 'settings.tags.tag' tag class="tag-edit-button"}} - {{tag.name}} - /{{tag.slug}} + {{tag.name}} + /{{tag.slug}} {{#if tag.isInternal}} - internal + internal {{/if}} -

{{tag.description}}

- +

{{tag.description}}

+ {{#link-to "posts" (query-params type=null author=null tag=tag.slug order=null)}} {{tag.count.posts}} {{/link-to}} diff --git a/ghost/admin/tests/acceptance/content-test.js b/ghost/admin/tests/acceptance/content-test.js index 1e16865872..d09eea9582 100644 --- a/ghost/admin/tests/acceptance/content-test.js +++ b/ghost/admin/tests/acceptance/content-test.js @@ -5,6 +5,7 @@ import { authenticateSession, invalidateSession } from 'ghost-admin/tests/helpers/ember-simple-auth'; +import {clickTrigger} from 'ember-power-select/test-support/helpers'; import {expect} from 'chai'; describe('Acceptance: Content', function () { @@ -124,6 +125,22 @@ describe('Acceptance: Content', function () { expect(currentURL(), 'url after double-click').to.equal(`/editor/${authorPost.id}`); }); + + it('sorts tags filter alphabetically', async function () { + server.create('tag', {name: 'B - Second', slug: 'second'}); + server.create('tag', {name: 'Z - Last', slug: 'last'}); + server.create('tag', {name: 'A - First', slug: 'first'}); + + await visit('/'); + await clickTrigger('[data-test-tag-select]'); + + let options = find('.ember-power-select-option'); + + expect(options[0].textContent.trim()).to.equal('All tags'); + expect(options[1].textContent.trim()).to.equal('A - First'); + expect(options[2].textContent.trim()).to.equal('B - Second'); + expect(options[3].textContent.trim()).to.equal('Z - Last'); + }); }); describe('as author', function () { diff --git a/ghost/admin/tests/acceptance/settings/tags-test.js b/ghost/admin/tests/acceptance/settings/tags-test.js index 2d7ea8d0a8..41ef0d038c 100644 --- a/ghost/admin/tests/acceptance/settings/tags-test.js +++ b/ghost/admin/tests/acceptance/settings/tags-test.js @@ -180,8 +180,12 @@ describe('Acceptance: Settings - Tags', function () { await fillIn('.tag-settings-pane input[name="name"]', 'New Name'); await triggerEvent('.tag-settings-pane input[name="name"]', 'blur'); + // extra timeout needed for Travis - sometimes it doesn't update + // quick enough and an extra wait() call doesn't help + await timeout(100); + // check we update with the data returned from the server - expect(find('.settings-tags .settings-tag:last .tag-title').text(), 'tag list updates on save') + expect(find('.settings-tag')[0].querySelector('.tag-title').textContent.trim(), 'tag list updates on save') .to.equal('New Name'); expect(find('.tag-settings-pane input[name="name"]').val(), 'settings form updates on save') .to.equal('New Name'); @@ -217,7 +221,8 @@ describe('Acceptance: Settings - Tags', function () { // it adds the tag to the list and selects expect(find('.settings-tags .settings-tag').length, 'tag list count after creation') .to.equal(3); - expect(find('.settings-tags .settings-tag:last .tag-title').text(), 'new tag list item title') + // new tag will be second in the list due to alphabetical sorting + expect(find('.settings-tags .settings-tag')[1].querySelector('.tag-title').textContent.trim(), 'new tag list item title') .to.equal('New Tag'); expect(find('a[href="/ghost/settings/tags/new-tag"]').hasClass('active'), 'highlights new tag') .to.be.true; @@ -312,5 +317,22 @@ describe('Acceptance: Settings - Tags', function () { expect(currentPath()).to.equal('error404'); expect(currentURL()).to.equal('/settings/tags/unknown'); }); + + it('sorts tags correctly', async function () { + server.create('tag', {name: 'B - Second', slug: 'second'}); + server.create('tag', {name: 'Z - Last', slug: 'last'}); + server.create('tag', {name: 'A - First', slug: 'first'}); + + await visit('settings/tags'); + + // second wait is needed for the vertical-collection to settle + await wait(); + + let tags = find('[data-test-tag]'); + + expect(tags[0].querySelector('[data-test-name]').textContent.trim()).to.equal('A - First'); + expect(tags[1].querySelector('[data-test-name]').textContent.trim()).to.equal('B - Second'); + expect(tags[2].querySelector('[data-test-name]').textContent.trim()).to.equal('Z - Last'); + }); }); }); diff --git a/ghost/admin/tests/integration/components/gh-psm-tags-input-test.js b/ghost/admin/tests/integration/components/gh-psm-tags-input-test.js index 09055545d4..1670b7680c 100644 --- a/ghost/admin/tests/integration/components/gh-psm-tags-input-test.js +++ b/ghost/admin/tests/integration/components/gh-psm-tags-input-test.js @@ -69,7 +69,7 @@ describe.skip('Integration: Component: gh-psm-tags-input', function () { expect(selected[1].textContent).to.have.string('Tag Three'); }); - it('exposes all tags as options', async function () { + it('exposes all tags as options sorted alphabetically', async function () { run(() => { this.set('post', this.get('store').findRecord('post', 1)); }); @@ -81,8 +81,8 @@ describe.skip('Integration: Component: gh-psm-tags-input', function () { let options = findAll('.ember-power-select-option'); expect(options.length).to.equal(4); expect(options[0].textContent).to.have.string('Tag One'); - expect(options[1].textContent).to.have.string('Tag Two'); - expect(options[2].textContent).to.have.string('Tag Three'); + expect(options[1].textContent).to.have.string('Tag Three'); + expect(options[2].textContent).to.have.string('Tag Two'); expect(options[3].textContent).to.have.string('#Internal Tag'); });