launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #06561
[Merge] lp:~rharding/launchpad/911857_extract_js into lp:launchpad
Richard Harding has proposed merging lp:~rharding/launchpad/911857_extract_js into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #911857 in Launchpad itself: "bugtask macros tableview has js code that should be extracted/tested"
https://bugs.launchpad.net/launchpad/+bug/911857
For more details, see:
https://code.launchpad.net/~rharding/launchpad/911857_extract_js/+merge/95438
= Summary =
There is a lot of JS code in the bugtask-macros-tableview.pt that makes it difficult to test and find/replace when updating and fixing things since it's a .pt file and not a .js file.
Templates should have bare bones JS code in them.
== Proposed Fix ==
This pulls out the JS into a buglisting.TableView object that handles building and binding up the view that the template was previously doing.
It tries to break up the building/binding/rendering into a more 'widget' like setup. This is somewhat how the JS would be in a YUI 3.5 View object. So it's partially setup to work in that format.
== Tests ==
== Demo and Q/A ==
The buglisting page shouldn't appear any differently. Head to launchpad/+bugs url and using the buglisting UI. The url should still update due to history events, ajax calls made out due to sort order changes, and the gear icon should enable the user to customize the view as usual.
== Lint ==
Linting changed files:
lib/lp/bugs/javascript/buglisting.js
lib/lp/bugs/templates/bugtask-macros-tableview.pt
--
https://code.launchpad.net/~rharding/launchpad/911857_extract_js/+merge/95438
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rharding/launchpad/911857_extract_js into lp:launchpad.
=== modified file 'lib/lp/bugs/javascript/buglisting.js'
--- lib/lp/bugs/javascript/buglisting.js 2012-01-10 21:25:39 +0000
+++ lib/lp/bugs/javascript/buglisting.js 2012-03-01 19:32:21 +0000
@@ -7,222 +7,432 @@
* @submodule buglisting
*/
-YUI.add('lp.bugs.buglisting', function(Y) {
-
-var module = Y.namespace('lp.bugs.buglisting');
-
-
-/**
- * Constructor.
- *
- * This is the model of the current batch, including the ordering, position,
- * and what fields are visibile.
- *
- * These values are stored in the History object, so that the browser
- * back/next buttons correctly adjust. The system defaults for field
- * visibility are fixed, so they are stored directly on the object.
- *
- * Accepts a config containing:
- * - field_visibility the requested field visibility as an associative array
- * - field_visibility_defaults the system defaults for field visibility as an
- * associative array.
- * - batch_key: A string representing the position and ordering of the
- * current batch, as returned by listing_navigator.get_batch_key
- */
-module.BugListingModel = function() {
- module.BugListingModel.superclass.constructor.apply(this, arguments);
-};
-
-
-module.BugListingModel.NAME = 'buglisting-model';
-
-
-module.BugListingModel.ATTRS = {
- field_visibility_defaults: {
- value: null
- }
-};
-
-
-Y.extend(module.BugListingModel, Y.Base, {
- /**
- * Initializer sets up the History object that stores most of the model
- * data.
- */
- initializer: function(config) {
- this.set('history', new Y.History({
- initialState: Y.merge(
- config.field_visibility, {batch_key: config.batch_key})
- }));
- },
-
- /**
- * Return the current field visibility, as an associative array.
- * Since the history contains field values that are not field-visibility,
- * use field_visibility_defaults to filter out non-field-visibility
- * values.
- */
- get_field_visibility: function() {
- var result = this.get('history').get();
- var key_source = this.get('field_visibility_defaults');
- Y.each(result, function(value, key) {
- if (!key_source.hasOwnProperty(key)){
- delete result[key];
- }
- });
- return result;
- },
-
- /**
- * Set the field visibility, updating history. Accepts an associative
- * array.
- */
- set_field_visibility: function(value) {
- this.get('history').add(value);
- },
-
- /**
- * Return the current batch key.
- */
- get_batch_key: function() {
- return this.get('history').get('batch_key');
- },
-
- /**
- * Set the current batch. The batch_key and the query mapping identifying
- * the batch must be supplied.
- */
- set_batch: function(batch_key, query) {
- var url = '?' + Y.QueryString.stringify(query);
- this.get('history').addValue('batch_key', batch_key, {url: url});
- }
-});
-
-
-/**
- * Constructor.
- * current_url is used to determine search params.
- * cache is the JSONRequestCache for the batch.
- * template is the template to use for rendering batches.
- * target is a YUI node to update when rendering batches.
- * navigation_indices is a YUI NodeList of nodes to update with the current
- * batch info.
- * io_provider is something providing the Y.io interface, typically used for
- * testing. Defaults to Y.io.
- */
-module.BugListingNavigator = function(config) {
- module.BugListingNavigator.superclass.constructor.apply(
- this, arguments);
-};
-
-module.BugListingNavigator.ATTRS = {
-};
-
-Y.extend(
- module.BugListingNavigator,
- Y.lp.app.listing_navigator.ListingNavigator, {
- _bindUI: function () {
- Y.lp.app.inlinehelp.init_help();
- },
-
- initializer: function(config) {
- this.get('model').get('history').after(
- 'change', this.history_changed, this);
- },
- /**
- * Event handler for history:change events.
- */
- history_changed: function(e) {
- if (e.newVal.hasOwnProperty('batch_key')) {
- var batch_key = e.newVal.batch_key;
- var batch = this.get('batches')[batch_key];
- this.pre_fetch_batches();
- this.render();
- this._bindUI();
- }
- else {
- // Handle Chrom(e|ium)'s initial popstate.
- this.get('model').get('history').replace(e.prevVal);
- }
- },
-
- /**
- * Return the model to use for rendering the batch. This will include
- * updates to field visibility.
- */
- get_render_model: function(current_batch) {
- return Y.merge(
- current_batch.mustache_model,
- this.get('model').get_field_visibility());
- },
-
- /**
- * Handle a previously-unseen batch by storing it in the cache and
- * stripping out field_visibility values that would otherwise shadow the
- * real values.
- */
- handle_new_batch: function(batch) {
- var key, i;
- Y.each(batch.field_visibility, function(value, key) {
- for (i = 0; i < batch.mustache_model.items.length; i++) {
- delete batch.mustache_model.items[i][key];
- }
- });
- return this.constructor.superclass.handle_new_batch.call(this, batch);
- }
-
-},{
- make_model: function(batch_key, cache) {
- return new module.BugListingModel({
- batch_key: batch_key,
- field_visibility: cache.field_visibility,
- field_visibility_defaults: cache.field_visibility_defaults
- });
- },
- get_search_params: function(config) {
- var search_params = Y.lp.app.listing_navigator.get_query(
- config.current_url);
- delete search_params.start;
- delete search_params.memo;
- delete search_params.direction;
- delete search_params.orderby;
- return search_params;
- }
-});
-
-/**
- * Factory to return a BugListingNavigator for the given page.
- */
-module.BugListingNavigator.from_page = function() {
- var target = Y.one('#client-listing');
- if (Y.Lang.isNull(target)){
- return null;
- }
- var navigation_indices = Y.all('.batch-navigation-index');
- var pre_fetch = Y.lp.app.listing_navigator.get_feature_flag(
- 'bugs.dynamic_bug_listings.pre_fetch');
- Y.lp.app.listing_navigator.linkify_navigation();
- var navigator = new module.BugListingNavigator({
- current_url: window.location,
- cache: LP.cache,
- template: LP.mustache_listings,
- target: target,
- navigation_indices: navigation_indices,
- pre_fetch: Boolean(pre_fetch)
- });
- navigator.set('backwards_navigation', Y.all('.first,.previous'));
- navigator.set('forwards_navigation', Y.all('.last,.next'));
- navigator.clickAction('.first', navigator.first_batch);
- navigator.clickAction('.next', navigator.next_batch);
- navigator.clickAction('.previous', navigator.prev_batch);
- navigator.clickAction('.last', navigator.last_batch);
- navigator.render_navigation();
- return navigator;
-};
-
-
-
-}, "0.1", {
- "requires": [
- "history", "node", 'lp.app.listing_navigator', 'lp.app.inlinehelp', 'lp.app.indicator']
+YUI.add('lp.bugs.buglisting', function (Y) {
+
+ var module = Y.namespace('lp.bugs.buglisting');
+
+
+ /**
+ * Constructor.
+ *
+ * This is the model of the current batch, including the ordering,
+ * position, and what fields are visibile.
+ *
+ * These values are stored in the History object, so that the browser
+ * back/next buttons correctly adjust. The system defaults for field
+ * visibility are fixed, so they are stored directly on the object.
+ *
+ * Accepts a config containing:
+ * - field_visibility the requested field visibility as an associative
+ * array
+ * - field_visibility_defaults the system defaults for field visibility
+ * as an associative array.
+ * - batch_key: A string representing the position and ordering of the
+ * current batch, as returned by listing_navigator.get_batch_key
+ */
+ module.BugListingModel = function () {
+ module.BugListingModel.superclass.constructor.apply(this, arguments);
+ };
+
+
+ module.BugListingModel.NAME = 'buglisting-model';
+
+
+ module.BugListingModel.ATTRS = {
+ field_visibility_defaults: {
+ value: null
+ }
+ };
+
+
+ Y.extend(module.BugListingModel, Y.Base, {
+ /**
+ * Initializer sets up the History object that stores most of the
+ * model data.
+ */
+ initializer: function(config) {
+ this.set('history', new Y.History({
+ initialState: Y.merge(
+ config.field_visibility, {
+ batch_key: config.batch_key
+ }
+ )
+ }));
+ },
+
+ /**
+ * Return the current field visibility, as an associative array.
+ * Since the history contains field values that are not
+ * field-visibility, use field_visibility_defaults to filter out
+ * non-field-visibility values.
+ */
+ get_field_visibility: function () {
+ var result = this.get('history').get();
+ var key_source = this.get('field_visibility_defaults');
+ Y.each(result, function(value, key) {
+ if (!key_source.hasOwnProperty(key)){
+ delete result[key];
+ }
+ });
+ return result;
+ },
+
+ /**
+ * Set the field visibility, updating history. Accepts an associative
+ * array.
+ */
+ set_field_visibility: function(value) {
+ this.get('history').add(value);
+ },
+
+ /**
+ * Return the current batch key.
+ */
+ get_batch_key: function() {
+ return this.get('history').get('batch_key');
+ },
+
+ /**
+ * Set the current batch. The batch_key and the query mapping
+ * identifying the batch must be supplied.
+ */
+ set_batch: function(batch_key, query) {
+ var url = '?' + Y.QueryString.stringify(query);
+ this.get('history').addValue('batch_key', batch_key, {url: url});
+ }
+ });
+
+
+ /**
+ * Constructor.
+ * current_url is used to determine search params.
+ * cache is the JSONRequestCache for the batch.
+ * template is the template to use for rendering batches.
+ * target is a YUI node to update when rendering batches.
+ * navigation_indices is a YUI NodeList of nodes to update with the
+ * current batch info.
+ * io_provider is something providing the Y.io interface, typically used
+ * for testing. Defaults to Y.io.
+ */
+ module.BugListingNavigator = function(config) {
+ module.BugListingNavigator.superclass.constructor.apply(
+ this, arguments);
+ };
+
+ module.BugListingNavigator.ATTRS = {
+ };
+
+ Y.extend(
+ module.BugListingNavigator,
+ Y.lp.app.listing_navigator.ListingNavigator, {
+ _bindUI: function () {
+ Y.lp.app.inlinehelp.init_help();
+ },
+
+ initializer: function(config) {
+ this.get('model').get('history').after(
+ 'change', this.history_changed, this);
+ },
+ /**
+ * Event handler for history:change events.
+ */
+ history_changed: function(e) {
+ if (e.newVal.hasOwnProperty('batch_key')) {
+ var batch_key = e.newVal.batch_key;
+ var batch = this.get('batches')[batch_key];
+ this.pre_fetch_batches();
+ this.render();
+ this._bindUI();
+ }
+ else {
+ // Handle Chrom(e|ium)'s initial popstate.
+ this.get('model').get('history').replace(e.prevVal);
+ }
+ },
+
+ /**
+ * Return the model to use for rendering the batch. This will include
+ * updates to field visibility.
+ */
+ get_render_model: function(current_batch) {
+ return Y.merge(
+ current_batch.mustache_model,
+ this.get('model').get_field_visibility());
+ },
+
+ /**
+ * Handle a previously-unseen batch by storing it in the cache and
+ * stripping out field_visibility values that would otherwise shadow the
+ * real values.
+ */
+ handle_new_batch: function(batch) {
+ var key, i;
+ Y.each(batch.field_visibility, function(value, key) {
+ for (i = 0; i < batch.mustache_model.items.length; i++) {
+ delete batch.mustache_model.items[i][key];
+ }
+ });
+ return this.constructor.superclass.handle_new_batch.call(this,
+ batch);
+ }
+
+ },{
+ make_model: function(batch_key, cache) {
+ return new module.BugListingModel({
+ batch_key: batch_key,
+ field_visibility: cache.field_visibility,
+ field_visibility_defaults: cache.field_visibility_defaults
+ });
+ },
+ get_search_params: function(config) {
+ var search_params = Y.lp.app.listing_navigator.get_query(
+ config.current_url);
+ delete search_params.start;
+ delete search_params.memo;
+ delete search_params.direction;
+ delete search_params.orderby;
+ return search_params;
+ }
+ });
+
+ /**
+ * Factory to return a BugListingNavigator for the given page.
+ */
+ module.BugListingNavigator.from_page = function() {
+ var target = Y.one('#client-listing');
+ if (Y.Lang.isNull(target)){
+ return null;
+ }
+ var navigation_indices = Y.all('.batch-navigation-index');
+ var pre_fetch = Y.lp.app.listing_navigator.get_feature_flag(
+ 'bugs.dynamic_bug_listings.pre_fetch');
+ Y.lp.app.listing_navigator.linkify_navigation();
+ var navigator = new module.BugListingNavigator({
+ current_url: window.location,
+ cache: LP.cache,
+ template: LP.mustache_listings,
+ target: target,
+ navigation_indices: navigation_indices,
+ pre_fetch: Boolean(pre_fetch)
+ });
+ navigator.set('backwards_navigation', Y.all('.first,.previous'));
+ navigator.set('forwards_navigation', Y.all('.last,.next'));
+ navigator.clickAction('.first', navigator.first_batch);
+ navigator.clickAction('.next', navigator.next_batch);
+ navigator.clickAction('.previous', navigator.prev_batch);
+ navigator.clickAction('.last', navigator.last_batch);
+ navigator.render_navigation();
+ return navigator;
+ };
+
+
+ /**
+ * Helper view object for managing the buglisting code on the actual table
+ * view.
+ *
+ * @class TableView
+ * @extends Y.Base
+ * @namespace lp.bugs.buglisting
+ *
+ */
+ module.TableView = Y.Base.create('buglisting-tableview', Y.Base,
+ [], {
+
+ /**
+ * Hook into the model events to aid in setting up history events.
+ *
+ * @method _bind_history
+ * @private
+ *
+ */
+ _bind_history: function () {
+ var that = this;
+ this.navigator.get('model').get('history').after(
+ 'change', function(e) {
+ // Only update the sort buttons if we've got a valid batch
+ // key.
+ if (Y.Object.hasKey(e.newVal, 'batch_key')) {
+ Y.lp.buglisting_utils.update_sort_button_visibility(
+ that.orderby,
+ e.newVal
+ );
+ }
+ }
+ );
+ },
+
+ /**
+ * Setup the order bar widget for use in the table view.
+ *
+ * @method _build_orderbar
+ * @private
+ *
+ */
+ _build_orderbar: function () {
+ var that = this;
+ that.orderby = new Y.lp.ordering.OrderByBar({
+ srcNode: Y.one('#bugs-orderby'),
+ sort_keys: this.get('sort_keys'),
+ active: this.get('active_sort_key'),
+ sort_order: this.get('sort_order'),
+ config_slot: true
+ });
+ Y.on('orderbybar:sort', function(e) {
+ that.navigator.first_batch(e);
+ });
+ },
+
+ /**
+ * We need to parse out the active key in case it indicates we should be
+ * desc sorting, etc.
+ *
+ * @method _init_sort
+ * @private
+ *
+ */
+ _init_sort: function () {
+ var active_key = this.get('active_sort_key');
+ if (active_key.charAt(0) === '-') {
+ this.set('active_sort_key',
+ active_key.substring(1, active_key.length));
+ this.set('sort_order', 'desc');
+ }
+ },
+
+ /**
+ * If after init we still don't have any keys to sort on, we go with our
+ * default sort key.
+ *
+ * @method _check_default_sort
+ * @private
+ *
+ */
+ _check_default_sort: function () {
+ var active_key = this.get('active_key');
+ var unknown_sort_key = true;
+
+ Y.each(this.get('sort_keys'), function(sort_key) {
+ if (sort_key[0] === active_key) {
+ unknown_sort_key = false;
+ }
+ });
+ if (unknown_sort_key) {
+ this.set('active_sort_key', this.get('default_sort'));
+ }
+ },
+
+ /**
+ * General YUI initializer setting up the tableview.
+ *
+ * @method intializer
+ * @param {Object} cfg
+ *
+ */
+ initializer: function (cfg) {
+ this.navigator =
+ Y.lp.bugs.buglisting.BugListingNavigator.from_page();
+
+ if (Y.Lang.isNull(this.navigator)){
+ return;
+ }
+
+ // now that we've set the values from the LP.cache, let's process it
+ this._init_sort();
+ // if we don't have sort values, we might want to set some defaults
+ this._check_default_sort();
+ this._build_orderbar();
+ this._bind_history();
+ },
+
+ /**
+ * Handle any UI binding, building for the tableview.
+ *
+ * @method render
+ *
+ */
+ render: function () {
+ var that = this;
+ var field_visibility =
+ that.navigator.get('model').get_field_visibility();
+
+ that.orderby.always_display = ['title'];
+ that.orderby.render();
+
+ // The listing util needs to be called AFTER the orderby widget is
+ // rendered or the little gear icon has no home and ends up in DOM
+ // limbo land.
+ var config_node = that.orderby.get('config_node');
+ that.list_util = new Y.lp.buglisting_utils.BugListingConfigUtil({
+ srcNode: config_node,
+ model: that.navigator.get('model')
+ });
+ that.list_util.render();
+
+ Y.on('buglisting-config-util:fields-changed', function(e) {
+ that.navigator.change_fields(
+ that.list_util.get('field_visibility'));
+ });
+
+ // The advanced search page contains sort options that have
+ // no related data fields we can display. If a user has selected
+ // such a sort order, this sort option should always be visible.
+ var check_visibility =
+ field_visibility['show_' + this.get('active_sort_key')];
+ if (check_visibility === undefined) {
+ that.orderby.always_display.push(active_sort_key);
+ }
+
+ Y.lp.buglisting_utils.update_sort_button_visibility(
+ that.orderby,
+ field_visibility
+ );
+ }
+ }, {
+ ATTRS: {
+ /**
+ * @attribute default_sort
+ * @default importance
+ * @type String
+ *
+ */
+ default_sort: {
+ value: 'importance'
+ },
+
+ /**
+ * @attribute active_sort_key
+ * @default undefined
+ * @type string
+ *
+ */
+ active_sort_key: {
+
+ },
+
+ /**
+ * @attribute sort_keys
+ * @default undefined
+ * @type Array
+ *
+ */
+ sort_keys: {
+ },
+
+ /**
+ * @attribute sort_order
+ * @default asc
+ * @type String
+ *
+ */
+ sort_order: {
+ value: 'asc'
+ }
+ }
+ });
+
+}, '0.1', {
+ 'requires': [
+ 'history', 'node', 'lp.app.listing_navigator', 'lp.app.inlinehelp',
+ 'lp.app.indicator', 'lp.ordering', 'lp.buglisting_utils'
+ ]
});
=== modified file 'lib/lp/bugs/templates/bugtask-macros-tableview.pt'
--- lib/lp/bugs/templates/bugtask-macros-tableview.pt 2012-02-07 10:43:49 +0000
+++ lib/lp/bugs/templates/bugtask-macros-tableview.pt 2012-03-01 19:32:21 +0000
@@ -681,76 +681,14 @@
show_new_listings request/features/bugs.dynamic_bug_listings.enabled;
advanced_search view/shouldShowAdvancedForm"
tal:condition="python: show_new_listings and not advanced_search">
- LPJS.use('lp.bugs.buglisting', 'lp.ordering', 'lp.buglisting_utils',
- function(Y) {
+ LPJS.use('lp.bugs.buglisting', function(Y) {
Y.on('domready', function() {
- var navigator = Y.lp.bugs.buglisting.BugListingNavigator
- .from_page();
- if (Y.Lang.isNull(navigator)){
- return;
- }
- var sort_keys = LP.cache.sort_keys;
- var active_sort_key = LP.cache.order_by;
- var sort_order = 'asc';
- if (active_sort_key.charAt(0) === '-') {
- active_sort_key = active_sort_key.substring(
- 1, active_sort_key.length);
- sort_order = 'desc';
- }
- var unknown_sort_key = true;
- Y.each(sort_keys, function(sort_key) {
- if (sort_key[0] === active_sort_key) {
- unknown_sort_key = false;
- }
- });
- if (unknown_sort_key) {
- active_sort_key = 'importance';
- }
- var orderby = new Y.lp.ordering.OrderByBar({
- srcNode: Y.one('#bugs-orderby'),
- sort_keys: sort_keys,
- active: active_sort_key,
- sort_order: sort_order,
- config_slot: true
- });
- orderby.render();
- Y.on('orderbybar:sort', function(e) {
- navigator.first_batch(e);
- });
- var model = navigator.get('model');
- model.get('history').after(
- 'change', function(e) {
- // Only update the sort buttons if we've got a valid batch
- // key.
- if (Y.Object.hasKey(e.newVal, 'batch_key')) {
- Y.lp.buglisting_utils.update_sort_button_visibility(
- orderby,
- e.newVal
- );
- }
- }
- );
- var config_node = orderby.get('config_node');
- var list_util = new Y.lp.buglisting_utils.BugListingConfigUtil({
- srcNode: config_node,
- model: model
- });
- list_util.render();
- Y.on('buglisting-config-util:fields-changed', function(e) {
- navigator.change_fields(list_util.get('field_visibility'));
- });
- var field_visibility =
- navigator.get('model').get_field_visibility();
- orderby.always_display = ['title'];
- // The advanced search page contains sort options that have
- // no related data fields we can display. If a user has selected
- // such a sort order, this sort option should always be visible.
- if (field_visibility['show_' + active_sort_key] === undefined) {
- orderby.always_display.push(active_sort_key);
- }
- Y.lp.buglisting_utils.update_sort_button_visibility(
- orderby, field_visibility);
- });
+ var view = new Y.lp.bugs.buglisting.TableView({
+ active_sort_key: LP.cache.order_by,
+ sort_keys: LP.cache.sort_keys
+ });
+ view.render();
+ })
});
</script>
</metal:listing_navigator>
