Use query params for sortable table headings

This commit is contained in:
Robin Ward 2014-04-16 12:05:54 -04:00
parent 2eab288dc9
commit b3ed8b6a32
26 changed files with 110 additions and 181 deletions

View file

@ -23,10 +23,7 @@ Discourse.BasicTopicListComponent = Ember.Component.extend({
_initFromTopicList: function(topicList) {
if (topicList !== null) {
this.setProperties({
topics: topicList.get('topics'),
sortOrder: topicList.get('sortOrder')
});
this.set('topics', topicList.get('topics'));
this.rerender();
}
},

View file

@ -12,23 +12,16 @@ Discourse.SortableHeadingComponent = Ember.Component.extend({
attributeBindings: ['colspan'],
sortable: function() {
return this.get('sortOrder') && this.get('sortBy');
}.property('sortOrder', 'sortBy'),
return this.get('order') && this.get('sortBy');
}.property('order', 'sortBy'),
iconSortClass: function() {
var sortable = this.get('sortable');
if (sortable && this.get('sortBy') === this.get('sortOrder.order')) {
return this.get('sortOrder.descending') ? 'fa fa-chevron-down' : 'fa fa-chevron-up';
if (this.get('sortable') && this.get('sortBy') === this.get('order')) {
return this.get('ascending') ? 'fa fa-chevron-up' : 'fa fa-chevron-down';
}
}.property('sortable', 'sortOrder.order', 'sortOrder.descending'),
}.property('sortable', 'order', 'ascending'),
click: function() {
var sortOrder = this.get('sortOrder'),
sortBy = this.get('sortBy');
if (sortBy && sortOrder) {
sortOrder.toggle(sortBy);
}
this.sendAction('action', this.get('sortBy'));
}
});

View file

@ -25,3 +25,4 @@ Discourse.DiscoveryController = Discourse.ObjectController.extend({
showMoreMonthlyUrl: function() { return this.showMoreUrl('monthly'); }.property('category', 'noSubcategories'),
showMoreYearlyUrl: function() { return this.showMoreUrl('yearly'); }.property('category', 'noSubcategories')
});

View file

@ -0,0 +1,6 @@
Discourse.DiscoverySortableController = Discourse.Controller.extend({
needs: ['discoveryTopics'],
queryParams: ['order', 'ascending'],
order: Em.computed.alias('controllers.discoveryTopics.order'),
ascending: Em.computed.alias('controllers.discoveryTopics.ascending')
});

View file

@ -11,7 +11,20 @@ Discourse.DiscoveryTopicsController = Discourse.DiscoveryController.extend({
bulkSelectEnabled: false,
selected: [],
order: 'default',
ascending: false,
actions: {
changeSort: function(sortBy) {
if (sortBy === this.get('order')) {
this.toggleProperty('ascending');
} else {
this.setProperties({ order: sortBy, ascending: false });
}
this.get('model').refreshSort(sortBy, this.get('ascending'));
},
// Show newly inserted topics
showInserted: function() {
var tracker = Discourse.TopicTrackingState.current();
@ -117,11 +130,6 @@ Discourse.DiscoveryTopicsController = Discourse.DiscoveryController.extend({
}.property('allLoaded', 'topics.length'),
loadMoreTopics: function() {
var topicList = this.get('model');
return topicList.loadMore().then(function(moreUrl) {
if (!Em.isEmpty(moreUrl)) {
Discourse.URL.replaceState(Discourse.getURL("/") + topicList.get('filter') + "/more");
}
});
return this.get('model').loadMore();
}
});

View file

@ -22,12 +22,7 @@ Discourse.TopicController = Discourse.ObjectController.extend(Discourse.Selected
return this.get('postStream.summary') ? "summary" : null;
}.property('postStream.summary'),
username_filters: function(key, value) {
// TODO: Ember bug? If I don't have a value parameter this does not update
if (value) {
}
return this.get('postStream.streamFilters.username_filters');
}.property("postStream.streamFilters.username_filters"),
username_filters: Discourse.computed.queryAlias('postStream.streamFilters.username_filters'),
init: function() {
this._super();

View file

@ -110,6 +110,23 @@ Discourse.computed = {
});
});
return computed.property.apply(computed, args);
}
},
/**
Creates a one way alias to a computed property, suitable for query params.
@method queryAlias
@param {String} path to the alias
@param {String} defaultValue for the variable (omitted if equal)
**/
queryAlias: function(path, defaultValue) {
return Ember.computed(function(key, value) {
if (value) {
// Annoying but this ensures the parameter is present
}
var result = this.get(path);
if (typeof result !== "undefined" && result.toString() === defaultValue) { return; }
return result;
}).property(path);
}
};

View file

@ -10,9 +10,6 @@ Discourse.URL = Em.Object.createWithMixins({
// Used for matching a topic
TOPIC_REGEXP: /\/t\/([^\/]+)\/(\d+)\/?(\d+)?/,
// Used for matching a /more URL
MORE_REGEXP: /\/more$/,
/**
Browser aware replaceState. Will only be invoked if the browser supports it.
@ -75,7 +72,6 @@ Discourse.URL = Em.Object.createWithMixins({
// TODO: Extract into rules we can inject into the URL handler
if (this.navigatedToHome(oldPath, path)) { return; }
if (this.navigatedToListMore(oldPath, path)) { return; }
if (this.navigatedToPost(oldPath, path)) { return; }
if (path.match(/^\/?users\/[^\/]+$/)) {
@ -111,24 +107,6 @@ Discourse.URL = Em.Object.createWithMixins({
return false;
},
/**
@private
If we're viewing more topics, scroll to where we were previously.
@method navigatedToListMore
@param {String} oldPath the previous path we were on
@param {String} path the path we're navigating to
**/
navigatedToListMore: function(oldPath, path) {
// If we transition from a /more path, scroll to the top
if (this.MORE_REGEXP.exec(oldPath) && (oldPath.indexOf(path) === 0)) {
window.scrollTo(0, 0);
}
return false;
},
/**
@private

View file

@ -1,30 +0,0 @@
/**
Represents the sort order of something, for example a topics list.
@class SortOrder
@extends Ember.Object
@namespace Discourse
@module Discourse
**/
Discourse.SortOrder = Ember.Object.extend({
order: 'default',
descending: true,
/**
Changes the sort to another column
@method toggle
@params {String} order the new sort order
**/
toggle: function(order) {
if (this.get('order') === order) {
this.toggleProperty('descending');
} else {
this.setProperties({
order: order,
descending: true
});
}
}
});

View file

@ -27,7 +27,6 @@ Discourse.TopList.reopenClass({
if (result[period]) {
// instanciate a new topic list with no sorting
topList.set(period, Discourse.TopicList.from(result[period]));
topList.set(period + ".sortOrder", undefined);
}
});

View file

@ -45,23 +45,12 @@ Discourse.TopicList = Discourse.Model.extend({
});
},
sortOrder: function() {
return Discourse.SortOrder.create();
}.property(),
/**
If the sort order changes, replace the topics in the list with the new
order.
@observes sortOrder
**/
_sortOrderChanged: function() {
refreshSort: function(order, ascending) {
var self = this,
sortOrder = this.get('sortOrder'),
params = this.get('params');
params.sort_order = sortOrder.get('order');
params.sort_descending = sortOrder.get('descending');
params.order = order;
params.ascending = ascending;
this.set('loaded', false);
var finder = finderFor(this.get('filter'), params);
@ -73,8 +62,7 @@ Discourse.TopicList = Discourse.Model.extend({
topics.pushObjects(newTopics);
self.setProperties({ loaded: true, more_topics_url: result.topic_list.more_topics_url });
});
}.observes('sortOrder.order', 'sortOrder.descending'),
},
loadMore: function() {
if (this.get('loadingMore')) { return Ember.RSVP.resolve(); }

View file

@ -32,25 +32,17 @@ Discourse.Route.buildRoutes(function() {
Discourse.Site.currentProp('periods').forEach(function(period) {
var top = 'top' + period.capitalize();
router.route(top, { path: '/top/' + period });
router.route(top, { path: '/top/' + period + '/more' });
router.route(top + 'Category', { path: '/category/:slug/l/top/' + period });
router.route(top + 'Category', { path: '/category/:slug/l/top/' + period + '/more' });
router.route(top + 'CategoryNone', { path: '/category/:slug/none/l/top/' + period });
router.route(top + 'CategoryNone', { path: '/category/:slug/none/l/top/' + period + '/more' });
router.route(top + 'Category', { path: '/category/:parentSlug/:slug/l/top/' + period });
router.route(top + 'Category', { path: '/category/:parentSlug/:slug/l/top/' + period + '/more' });
});
// filters
Discourse.Site.currentProp('filters').forEach(function(filter) {
router.route(filter, { path: '/' + filter });
router.route(filter, { path: '/' + filter + '/more' });
router.route(filter + 'Category', { path: '/category/:slug/l/' + filter });
router.route(filter + 'Category', { path: '/category/:slug/l/' + filter + '/more' });
router.route(filter + 'CategoryNone', { path: '/category/:slug/none/l/' + filter });
router.route(filter + 'CategoryNone', { path: '/category/:slug/none/l/' + filter + '/more' });
router.route(filter + 'Category', { path: '/category/:parentSlug/:slug/l/' + filter });
router.route(filter + 'Category', { path: '/category/:parentSlug/:slug/l/' + filter + '/more' });
});
this.route('categories');

View file

@ -7,7 +7,8 @@
@namespace Discourse
@module Discourse
**/
Discourse.DiscoveryRoute = Discourse.Route.extend(Discourse.OpenComposer, {
Discourse.DiscoveryRoute = Discourse.Route.extend(Discourse.ScrollTop, Discourse.OpenComposer, {
actions: {
loading: function() {
var controller = this.controllerFor('discovery');
@ -25,6 +26,7 @@ Discourse.DiscoveryRoute = Discourse.Route.extend(Discourse.OpenComposer, {
var controller = this.controllerFor('discovery');
Ember.run.cancel(controller.get('scheduledSpinner'));
controller.setProperties({ loading: false, loadingSpinner: false });
this._scrollTop();
},
didTransition: function() {

View file

@ -6,15 +6,28 @@
**/
function buildTopicRoute(filter) {
return Discourse.Route.extend({
queryParams: {
sort: { replace: true },
ascending: { replace: true }
},
beforeModel: function() {
this.controllerFor('navigationDefault').set('filterMode', filter);
},
model: function() {
model: function(data, transaction) {
var params = transaction.queryParams;
// attempt to stop early cause we need this to be called before .sync
Discourse.ScreenTrack.current().stop();
return Discourse.TopicList.list(filter).then(function(list) {
var findOpts = {};
if (params && params.order) { findOpts.order = params.order; }
if (params && params.ascending) { findOpts.ascending = params.ascending; }
return Discourse.TopicList.list(filter, findOpts).then(function(list) {
var tracking = Discourse.TopicTrackingState.current();
if (tracking) {
tracking.sync(list, filter);
@ -24,7 +37,13 @@ function buildTopicRoute(filter) {
});
},
setupController: function(controller, model) {
setupController: function(controller, model, trans) {
controller.setProperties({
order: Em.get(trans, 'queryParams.order'),
ascending: Em.get(trans, 'queryParams.ascending')
});
var period = filter.indexOf('/') > 0 ? filter.split('/')[1] : '',
filterText = I18n.t('filters.' + filter.replace('/', '.') + '.title', {count: 0});
@ -141,6 +160,7 @@ Discourse.addInitializer(function() {
Discourse.DiscoveryCategoryNoneRoute = buildCategoryRoute('latest', {no_subcategories: true});
Discourse.Site.currentProp('filters').forEach(function(filter) {
Discourse["Discovery" + filter.capitalize() + "Controller"] = Discourse.DiscoverySortableController.extend();
Discourse["Discovery" + filter.capitalize() + "Route"] = buildTopicRoute(filter);
Discourse["Discovery" + filter.capitalize() + "CategoryRoute"] = buildCategoryRoute(filter);
Discourse["Discovery" + filter.capitalize() + "CategoryNoneRoute"] = buildCategoryRoute(filter, {no_subcategories: true});

View file

@ -10,14 +10,8 @@ Discourse.TopicRoute = Discourse.Route.extend({
redirect: function() { Discourse.redirectIfLoginRequired(this); },
queryParams: {
filter: {
refreshModel: false,
replace: true
},
username_filters: {
refreshModel: false,
replace: true
}
filter: { replace: true },
username_filters: { replace: true }
},
actions: {
@ -114,16 +108,18 @@ Discourse.TopicRoute = Discourse.Route.extend({
return topic;
},
model: function(params) {
model: function(params, transition) {
var queryParams = transition.queryParams;
var topic = this.modelFor('topic');
if (topic && (topic.get('id') === parseInt(params.id, 10))) {
this.setupParams(topic, params);
this.setupParams(topic, queryParams);
// If we have the existing model, refresh it
return topic.get('postStream').refresh().then(function() {
return topic;
});
} else {
return this.setupParams(Discourse.Topic.create(_.omit(params, 'username_filters', 'filter')), params);
return this.setupParams(Discourse.Topic.create(_.omit(params, 'username_filters', 'filter')), queryParams);
}
},

View file

@ -41,7 +41,7 @@ Discourse.UserRoute = Discourse.Route.extend({
serialize: function(model) {
if (!model) return {};
return { username: model.get('username').toLowerCase() };
return { username: (Em.get(model, 'username') || '').toLowerCase() };
},
setupController: function(controller, user) {

View file

@ -3,24 +3,24 @@
<table id="topic-list">
<thead>
<tr>
{{#sortable-heading sortBy="default" sortOrder=sortOrder}}
{{#sortable-heading sortBy="default"}}
{{i18n topic.title}}
{{/sortable-heading}}
{{#unless controller.hideCategory}}
{{#sortable-heading sortBy="category" sortOrder=sortOrder}}
{{#sortable-heading sortBy="category"}}
{{i18n category_title}}
{{/sortable-heading}}
{{/unless}}
{{#sortable-heading sortBy="posts" number=true sortOrder=sortOrder}}
{{#sortable-heading sortBy="posts" number=true}}
{{i18n posts}}
{{/sortable-heading}}
{{#sortable-heading sortBy="likes" number=true sortOrder=sortOrder}}
{{#sortable-heading sortBy="likes" number=true}}
{{i18n likes}}
{{/sortable-heading}}
{{#sortable-heading sortBy="views" number=true sortOrder=sortOrder}}
{{#sortable-heading sortBy="views" number=true}}
{{i18n views}}
{{/sortable-heading}}
{{#sortable-heading sortBy="activity" number=true colspan="2" sortOrder=sortOrder}}
{{#sortable-heading sortBy="activity" number=true colspan="2"}}
{{i18n activity}}
{{/sortable-heading}}
</tr>

View file

@ -16,27 +16,27 @@
{{/if}}
</th>
{{/if}}
{{#sortable-heading sortBy="default" sortOrder=sortOrder}}
{{#sortable-heading sortBy="default" action="changeSort" order=order ascending=ascending}}
{{i18n topic.title}}
{{/sortable-heading}}
{{#unless controller.hideCategory}}
{{#sortable-heading sortBy="category" sortOrder=sortOrder}}
{{#sortable-heading sortBy="category" action="changeSort" order=order ascending=ascending}}
{{i18n category_title}}
{{/sortable-heading}}
{{/unless}}
{{#sortable-heading sortBy="posters" sortOrder=sortOrder}}
{{#sortable-heading sortBy="posters" action="changeSort" order=order ascending=ascending}}
{{i18n users}}
{{/sortable-heading}}
{{#sortable-heading sortBy="posts" number=true sortOrder=sortOrder}}
{{#sortable-heading sortBy="posts" number=true action="changeSort" order=order ascending=ascending}}
{{i18n posts}}
{{/sortable-heading}}
{{#sortable-heading sortBy="likes" number=true sortOrder=sortOrder}}
{{#sortable-heading sortBy="likes" number=true action="changeSort" order=order ascending=ascending}}
{{i18n likes}}
{{/sortable-heading}}
{{#sortable-heading sortBy="views" number=true sortOrder=sortOrder}}
{{#sortable-heading sortBy="views" number=true action="changeSort" order=order ascending=ascending}}
{{i18n views}}
{{/sortable-heading}}
{{#sortable-heading sortBy="activity" number=true colspan="2" sortOrder=sortOrder}}
{{#sortable-heading sortBy="activity" number=true colspan="2" action="changeSort" order=order ascending=ascending}}
{{i18n activity}}
{{/sortable-heading}}
</tr>

View file

@ -26,10 +26,7 @@ Discourse.DiscoveryTopicsView = Discourse.View.extend(Discourse.LoadMore, {
},
_readjustScrollPosition: function() {
var scrollTo = Discourse.Session.currentProp('topicListScrollPosition'),
url = document.location.href;
if (url && url.indexOf('/more') === -1) { scrollTo = 0; }
var scrollTo = Discourse.Session.currentProp('topicListScrollPosition');
if (typeof scrollTo !== "undefined") {
Em.run.schedule('afterRender', function() {

View file

@ -1,8 +1,6 @@
// These will help us migrate up to the new ember's default behavior
window.ENV = {
CP_DEFAULT_CACHEABLE: true,
VIEW_PRESERVES_CONTEXT: true,
MANDATORY_SETTER: false,
FEATURES: {'query-params-new': true}
};

View file

@ -236,8 +236,8 @@ class ListController < ApplicationController
route_params = {format: 'json'}
route_params[:category] = @category.slug_for_url if @category
route_params[:parent_category] = @category.parent_category.slug_for_url if @category && @category.parent_category
route_params[:sort_order] = opts[:sort_order] if opts[:sort_order].present?
route_params[:sort_descending] = opts[:sort_descending] if opts[:sort_descending].present?
route_params[:order] = opts[:order] if opts[:order].present?
route_params[:ascending] = opts[:ascending] if opts[:ascending].present?
route_params
end
@ -265,8 +265,8 @@ class ListController < ApplicationController
topic_ids: param_to_integer_list(:topic_ids),
exclude_category: (params[:exclude_category] || select_menu_item.try(:filter)),
category: params[:category],
sort_order: params[:sort_order],
sort_descending: params[:sort_descending],
order: params[:order],
ascending: params[:ascending],
status: params[:status]
}
options[:no_subcategories] = true if params[:no_subcategories] == 'true'

View file

@ -249,7 +249,6 @@ Discourse::Application.routes.draw do
# We've renamed popular to latest. If people access it we want a permanent redirect.
get "popular" => "list#popular_redirect"
get "popular/more" => "list#popular_redirect"
resources :categories, :except => :show
get "category/:id/show" => "categories#show"
@ -278,13 +277,9 @@ Discourse::Application.routes.draw do
Discourse.filters.each do |filter|
get "#{filter}" => "list##{filter}"
get "#{filter}/more" => "list##{filter}"
get "category/:category/l/#{filter}" => "list#category_#{filter}", as: "category_#{filter}"
get "category/:category/l/#{filter}/more" => "list#category_#{filter}"
get "category/:category/none/l/#{filter}" => "list#category_none_#{filter}", as: "category_none_#{filter}"
get "category/:category/none/l/#{filter}/more" => "list#category_none_#{filter}"
get "category/:parent_category/:category/l/#{filter}" => "list#parent_category_category_#{filter}", as: "parent_category_category_#{filter}"
get "category/:parent_category/:category/l/#{filter}/more" => "list#parent_category_category_#{filter}"
end
get "search" => "search#query"

View file

@ -16,13 +16,13 @@ class TopicQuery
topic_ids
visible
category
sort_order
order
ascending
no_subcategories
sort_descending
no_definitions
status).map(&:to_sym)
# Maps `sort_order` to a columns in `topics`
# Maps `order` to a columns in `topics`
SORTABLE_MAPPING = {
'likes' => 'like_count',
'views' => 'views',
@ -189,8 +189,8 @@ class TopicQuery
end
def apply_ordering(result, options)
sort_column = SORTABLE_MAPPING[options[:sort_order]] || 'default'
sort_dir = (options[:sort_descending] == "false") ? "ASC" : "DESC"
sort_column = SORTABLE_MAPPING[options[:order]] || 'default'
sort_dir = (options[:ascending] == "true") ? "ASC" : "DESC"
# If we are sorting in the default order desc, we should consider including pinned
# topics. Otherwise, just use bumped_at.

View file

@ -151,7 +151,7 @@ describe TopicQuery do
context 'sort_order' do
def ids_in_order(order, descending=true)
TopicQuery.new(admin, sort_order: order, sort_descending: descending ? 'true' : 'false').list_latest.topics.map(&:id)
TopicQuery.new(admin, order: order, ascending: descending ? 'false' : 'true').list_latest.topics.map(&:id)
end
it "returns the topics in correct order" do

View file

@ -148,7 +148,7 @@ test("streamFilters", function() {
postStream.toggleParticipant(participant.username);
deepEqual(postStream.get('streamFilters'), {
username_filters: ['eviltrout']
username_filters: 'eviltrout'
}, "streamFilters contains the username we filtered");
});

View file

@ -1,23 +0,0 @@
module("Discourse.SortOrder");
test('defaults', function() {
var sortOrder = Discourse.SortOrder.create();
equal(sortOrder.get('order'), 'default', 'it is `default` by default');
equal(sortOrder.get('descending'), true, 'it is descending by default');
});
test('toggle', function() {
var sortOrder = Discourse.SortOrder.create();
sortOrder.toggle('default');
equal(sortOrder.get('descending'), false, 'if we toggle the same name it swaps the asc/desc');
sortOrder.toggle('name');
equal(sortOrder.get('order'), 'name', 'it changes the order');
equal(sortOrder.get('descending'), true, 'when toggling names it switches back to descending');
sortOrder.toggle('name');
sortOrder.toggle('name');
equal(sortOrder.get('descending'), true, 'toggling twice goes back to descending');
});