Skip to content
This repository has been archived by the owner on Nov 28, 2022. It is now read-only.

Commit

Permalink
🚀 supercharge tags performance
Browse files Browse the repository at this point in the history
closes TryGhost/Ghost#8540
- use `{{vertical-collection}}` in the tags dropdown filter list, opening the dropdown is now virtually instant as it's not attempting to immediately render components for every tag in the list
- remove pagination/infinite scroll from tags screen
- load all tags when accessing the tags screen
  - will pause to show spinner if no tags have previously been loaded
  - if tags exist in the ember data store, show the list immediately and load/update list in the background
- use `{{vertical-collection}}` to render enough tags to fill the scrollable area with a small buffer and use occlusion and element re-use to swap tags in whilst scrolling (suuuuper fast no matter number of tags loaded)
- scroll tags into view when they are selected (keyboard nav now makes a lot more sense)
- tested with 875 tags and 2x/5x CPU throttling with no major slowdowns 🎉
  • Loading branch information
kevinansfield committed Jul 7, 2017
1 parent 016d3f8 commit e7b378e
Show file tree
Hide file tree
Showing 13 changed files with 231 additions and 68 deletions.
12 changes: 1 addition & 11 deletions app/components/gh-tag.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,5 @@
import Component from 'ember-component';
import injectService from 'ember-service/inject';
import {invokeAction} from 'ember-invoke-action';

export default Component.extend({
feature: injectService(),

willDestroyElement() {
this._super(...arguments);

if (this.get('tag.isDeleted') && this.get('onDelete')) {
invokeAction(this, 'onDelete');
}
}
tagName: ''
});
6 changes: 6 additions & 0 deletions app/components/power-select-vertical-collection-options.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import OptionsComponent from 'ember-power-select/components/power-select/options';
import layout from '../templates/components/power-select-vertical-collection-options';

export default OptionsComponent.extend({
layout
});
27 changes: 26 additions & 1 deletion app/controllers/settings/tags.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import Controller from 'ember-controller';
import injectController from 'ember-controller/inject';
import run from 'ember-runloop';
import {alias, equal, sort} from 'ember-computed';

export default Controller.extend({
Expand All @@ -12,7 +13,7 @@ export default Controller.extend({
tagContentFocused: equal('keyboardFocus', 'tagContent'),

// TODO: replace with ordering by page count once supported by the API
tags: sort('model', function (a, b) {
sortedTags: sort('model', function (a, b) {
let idA = +a.get('id');
let idB = +b.get('id');

Expand All @@ -25,6 +26,30 @@ export default Controller.extend({
return 0;
}),

scrollTagIntoView(tag) {
run.scheduleOnce('afterRender', this, function () {
let id = `#gh-tag-${tag.get('id')}`;
let element = document.querySelector(id);

if (element) {
let scroll = document.querySelector('.tag-list');
let {scrollTop} = scroll;
let scrollHeight = scroll.offsetHeight;
let element = document.querySelector(id);
let elementTop = element.offsetTop;
let elementHeight = element.offsetHeight;

if (elementTop < scrollTop) {
element.scrollIntoView(true);
}

if (elementTop + elementHeight > scrollTop + scrollHeight) {
element.scrollIntoView(false);
}
}
});
},

actions: {
leftMobile() {
let firstTag = this.get('tags.firstObject');
Expand Down
35 changes: 22 additions & 13 deletions app/routes/settings/tags.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,17 @@
import $ from 'jquery';
import AuthenticatedRoute from 'ghost-admin/routes/authenticated';
import CurrentUserSettings from 'ghost-admin/mixins/current-user-settings';
import PaginationMixin from 'ghost-admin/mixins/pagination';
import ShortcutsRoute from 'ghost-admin/mixins/shortcuts-route';

export default AuthenticatedRoute.extend(CurrentUserSettings, PaginationMixin, ShortcutsRoute, {
export default AuthenticatedRoute.extend(CurrentUserSettings, ShortcutsRoute, {
titleToken: 'Settings - Tags',

paginationModel: 'tag',
paginationSettings: {
include: 'count.posts',
limit: 15
perPage: 30,
perPageParam: 'limit',
totalPagesParam: 'meta.pagination.pages',

queryParams: {
include: 'count.posts'
},

shortcuts: {
Expand All @@ -22,30 +23,38 @@ export default AuthenticatedRoute.extend(CurrentUserSettings, PaginationMixin, S
c: 'newTag'
},

// authors aren't allowed to manage tags
beforeModel() {
this._super(...arguments);

return this.get('session.user')
.then(this.transitionAuthor());
},

model(params, transition) {
return this.loadFirstPage(transition).then(() => {
return this.store.filter('tag', (tag) => {
return !tag.get('isNew');
});
// set model to a live array so all tags are shown and created/deleted tags
// are automatically added/removed. Also load all tags in the background,
// pausing to show the loading spinner if no tags have been loaded yet
model() {
let promise = this.store.query('tag', {limit: 'all', include: 'count.posts'});
let filter = this.store.filter('tag', (tag) => {
return !tag.get('isNew');
});

if (this.store.peekAll('tag').get('length') === 0) {
return promise.then(() => filter);
} else {
return filter;
}
},

deactivate() {
this._super(...arguments);
this.send('resetShortcutsScope');
this.send('resetPagination');
},

stepThroughTags(step) {
let currentTag = this.modelFor('settings.tags.tag');
let tags = this.get('controller.tags');
let tags = this.get('controller.sortedTags');
let length = tags.get('length');

if (currentTag && length) {
Expand Down
5 changes: 5 additions & 0 deletions app/routes/settings/tags/tag.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@ export default AuthenticatedRoute.extend({
return {tag_slug: model.get('slug')};
},

setupController(controller, model) {
this._super(...arguments);
this.controllerFor('settings.tags').scrollTagIntoView(model);
},

// reset the model so that mobile screens react to an empty selectedTag
deactivate() {
this._super(...arguments);
Expand Down
9 changes: 8 additions & 1 deletion app/styles/layouts/content.css
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,8 @@
}

.gh-contentfilter-menu-dropdown {
min-width: 150px;
width: 180px;
max-height: 220px;
margin-top: 10px;
padding: 5px 0 7px 0;
border: none !important;
Expand All @@ -93,6 +94,12 @@
margin: 0 10px !important;
}

.gh-contentfilter-menu-dropdown .ember-power-select-option {
overflow: hidden;
text-overflow: ellipsis;
white-space: nowrap;
}


/* Content List
/* ---------------------------------------------------------- */
Expand Down
2 changes: 1 addition & 1 deletion app/templates/components/gh-tag.hbs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<div class="settings-tag">
<div class="settings-tag" id="gh-tag-{{tag.id}}">
{{#link-to 'settings.tags.tag' tag class="tag-edit-button"}}
<span class="tag-title">{{tag.name}}</span>
<span class="label label-default">/{{tag.slug}}</span>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
{{#if select.loading}}
{{#if loadingMessage}}
<li class="ember-power-select-option ember-power-select-option--loading-message" role="option">{{loadingMessage}}</li>
{{/if}}
{{/if}}

{{#vertical-collection options minHeight=30 staticHeight=true bufferSize=5 as |opt index|}}
<li class="ember-power-select-option"
aria-selected="{{ember-power-select-is-selected opt select.selected}}"
aria-disabled={{ember-power-select-true-string-if-present opt.disabled}}
aria-current="{{eq opt select.highlighted}}"
data-option-index="{{groupIndex}}{{index}}"
role="option">
{{yield opt select}}
</li>
{{/vertical-collection}}
1 change: 1 addition & 0 deletions app/templates/posts.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
dropdownClass="gh-contentfilter-menu-dropdown"
searchPlaceholder="Search tags"
matchTriggerWidth=false
optionsComponent="power-select-vertical-collection-options"
data-test-tag-select=true
as |tag|
}}
Expand Down
21 changes: 11 additions & 10 deletions app/templates/settings/tags.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,19 @@
</header>

{{#gh-tags-management-container tags=tags selectedTag=selectedTag enteredMobile="enteredMobile" leftMobile="leftMobile" as |container|}}
{{#gh-infinite-scroll
fetch="loadNextPage"
isLoading=isLoading
classNames="tag-list"
as |checkScroll|
}}
<div class="tag-list">
<section class="tag-list-content settings-tags {{if tagListFocused 'keyboard-focused'}}">
{{#each tags as |tag|}}
{{gh-tag tag=tag onDelete=(action checkScroll)}}
{{/each}}
{{#vertical-collection sortedTags
minHeight=67
staticHeight=true
bufferSize=5
containerSelector=".tag-list"
as |tag|
}}
{{gh-tag tag=tag}}
{{/vertical-collection}}
</section>
{{/gh-infinite-scroll}}
</div>
<section class="settings-menu-container tag-settings {{if tagContentFocused 'keyboard-focused'}} {{if container.displaySettingsPane 'tag-settings-in'}}">
{{outlet}}
</section>
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
"node": ">= 4"
},
"devDependencies": {
"@html-next/vertical-collection": "^1.0.0-beta.4",
"autoprefixer": "6.7.7",
"blueimp-md5": "2.7.0",
"bower": "1.8.0",
Expand Down
42 changes: 14 additions & 28 deletions tests/acceptance/settings/tags-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import $ from 'jquery';
import destroyApp from '../../helpers/destroy-app';
import run from 'ember-runloop';
import startApp from '../../helpers/start-app';
import wait from 'ember-test-helpers/wait';
import {Response} from 'ember-cli-mirage';
import {afterEach, beforeEach, describe, it} from 'mocha';
import {authenticateSession, invalidateSession} from 'ghost-admin/tests/helpers/ember-simple-auth';
Expand Down Expand Up @@ -79,6 +80,9 @@ describe('Acceptance: Settings - Tags', function () {

await visit('/settings/tags');

// second wait is needed for the vertical-collection to settle
await wait();

// it redirects to first tag
expect(currentURL(), 'currentURL').to.equal(`/settings/tags/${tag1.slug}`);

Expand Down Expand Up @@ -125,6 +129,8 @@ describe('Acceptance: Settings - Tags', function () {
keyup(38);
});

await wait();

// it navigates to previous tag
expect(currentURL(), 'url after keyboard up arrow').to.equal(`/settings/tags/${tag1.slug}`);

Expand All @@ -138,6 +144,8 @@ describe('Acceptance: Settings - Tags', function () {
keyup(40);
});

await wait();

// it navigates to previous tag
expect(currentURL(), 'url after keyboard down arrow').to.equal(`/settings/tags/${tag2.slug}`);

Expand Down Expand Up @@ -203,6 +211,9 @@ describe('Acceptance: Settings - Tags', function () {

await visit('/settings/tags/tag-1');

// second wait is needed for the vertical-collection to settle
await wait();

expect(currentURL(), 'URL after direct load').to.equal('/settings/tags/tag-1');

// it loads all other tags
Expand All @@ -218,39 +229,14 @@ describe('Acceptance: Settings - Tags', function () {
.to.equal('Tag 1');
});

it('has infinite scroll pagination of tags list', async function () {
server.createList('tag', 32);

await visit('settings/tags/tag-0');

// it loads first page
expect(find('.settings-tags .settings-tag').length, 'tag list count on first load')
.to.equal(15);

await find('.tag-list').scrollTop(find('.tag-list-content').height());
await triggerEvent('.tag-list', 'scroll');

// it loads the second page
expect(find('.settings-tags .settings-tag').length, 'tag list count on second load')
.to.equal(30);

await find('.tag-list').scrollTop(find('.tag-list-content').height());

// NOTE: FF has issues with scrolling further in acceptance tests
// but works fine outside of tests
//
// await triggerEvent('.tag-list', 'scroll');
//
// // it loads the final page
// expect(find('.settings-tags .settings-tag').length, 'tag list count on third load')
// .to.equal(32);
});

it('shows the internal tag label', async function () {
server.create('tag', {name: '#internal-tag', slug: 'hash-internal-tag', visibility: 'internal'});

await visit('settings/tags/');

// second wait is needed for the vertical-collection to settle
await wait();

expect(currentURL()).to.equal('/settings/tags/hash-internal-tag');

expect(find('.settings-tags .settings-tag').length, 'tag list count')
Expand Down
Loading

0 comments on commit e7b378e

Please sign in to comment.