Merge lp:~jcsackett/juju-gui/remove-slider-code into lp:juju-gui/experimental

Proposed by j.c.sackett
Status: Merged
Merged at revision: 521
Proposed branch: lp:~jcsackett/juju-gui/remove-slider-code
Merge into: lp:juju-gui/experimental
Diff against target: 588 lines (+2/-485)
9 files modified
app/modules-debug.js (+0/-4)
app/subapps/browser/views/sidebar.js (+2/-33)
app/subapps/browser/views/view.js (+0/-1)
app/widgets/charm-slider.js (+0/-332)
bin/merge-files (+0/-3)
lib/views/browser/charm-slider.less (+0/-29)
lib/views/stylesheet.less (+0/-1)
test/index.html (+0/-1)
test/test_charm_slider.js (+0/-81)
To merge this branch: bzr merge lp:~jcsackett/juju-gui/remove-slider-code
Reviewer Review Type Date Requested Status
Juju GUI Hackers Pending
Review via email: mp+157885@code.launchpad.net

Description of the change

Removes charm slider

Recent redesigns killed the slider widget; this branch removes the code for
the widget, as well as the instances where it was used.
It also removes scrollview-ie from the manual build step, as the only
scrollview dependency in juju-gui was the slider.

https://codereview.appspot.com/8558045/

To post a comment you must log in.
Revision history for this message
j.c.sackett (jcsackett) wrote :

Reviewers: mp+157885_code.launchpad.net,

Message:
Please take a look.

Description:
Removes charm slider

Recent redesigns killed the slider widget; this branch removes the code
for
the widget, as well as the instances where it was used.
It also removes scrollview-ie from the manual build step, as the only
scrollview dependency in juju-gui was the slider.

https://code.launchpad.net/~jcsackett/juju-gui/remove-slider-code/+merge/157885

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/8558045/

Affected files:
   A [revision details]
   M app/modules-debug.js
   M app/subapps/browser/views/sidebar.js
   M app/subapps/browser/views/view.js
   D app/widgets/charm-slider.js
   M bin/merge-files
   D lib/views/browser/charm-slider.less
   M lib/views/stylesheet.less
   M test/index.html
   D test/test_charm_slider.js

Revision history for this message
Richard Harding (rharding) wrote :

LGTM thanks for the update and sorry to see it go.

https://codereview.appspot.com/8558045/

Revision history for this message
Jeff Pihach (hatch) wrote :
Revision history for this message
j.c.sackett (jcsackett) wrote :

*** Submitted:

Removes charm slider

Recent redesigns killed the slider widget; this branch removes the code
for
the widget, as well as the instances where it was used.
It also removes scrollview-ie from the manual build step, as the only
scrollview dependency in juju-gui was the slider.

R=rharding, jeff.pihach
CC=
https://codereview.appspot.com/8558045

https://codereview.appspot.com/8558045/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'app/modules-debug.js'
2--- app/modules-debug.js 2013-04-04 14:45:42 +0000
3+++ app/modules-debug.js 2013-04-09 14:39:23 +0000
4@@ -75,10 +75,6 @@
5 fullpath: '/juju-ui/widgets/charm-container.js'
6 },
7
8- 'browser-charm-slider': {
9- fullpath: '/juju-ui/widgets/charm-slider.js'
10- },
11-
12 'browser-overlay-indicator': {
13 fullpath: '/juju-ui/widgets/overlay-indicator.js'
14 },
15
16=== modified file 'app/subapps/browser/views/sidebar.js'
17--- app/subapps/browser/views/sidebar.js 2013-04-09 13:44:44 +0000
18+++ app/subapps/browser/views/sidebar.js 2013-04-09 14:39:23 +0000
19@@ -35,35 +35,6 @@
20 },
21
22 /**
23- * Given a set of Charms generate a CharmSlider widget with that data.
24- *
25- * @method _generateSliderWidget
26- * @param {Object} sliderCharms BrowserCharmList of Charms from the API.
27- *
28- */
29- _generateSliderWidget: function(sliderCharms) {
30- var sliderWidgets = [];
31-
32- sliderCharms.each(function(charm) {
33- sliderWidgets.push(
34- new Y.juju.widgets.browser.CharmToken(charm.getAttrs()));
35- });
36-
37- if (sliderWidgets.length) {
38- var slider = new Y.juju.widgets.browser.CharmSlider({
39- items: Y.Array.map(sliderWidgets, function(widget) {
40- var node = Y.Node.create('<div>');
41- widget.render(node);
42- return node.getHTML();
43- })
44- });
45- return slider;
46- } else {
47- return false;
48- }
49- },
50-
51- /**
52 * Event handler for selecting a charm from a list on the page. Forces a
53 * render of the charm details view for the user.
54 *
55@@ -109,6 +80,8 @@
56 // display.
57 this.get('store').sidebarEditorial({
58 'success': function(data) {
59+ // XXX: j.c.sackett Apr 9, 2013: Remove all references to "slider"
60+ // once Charmworld API is updated
61 var sliderCharms = this.get('store').resultsToCharmlist(
62 data.result.slider);
63 var sliderContainer = container.one('.bws-left .slider');
64@@ -164,9 +137,6 @@
65 *
66 */
67 destructor: function() {
68- if (this.slider) {
69- this.slider.destroy();
70- }
71 if (this.charmContainers) {
72 Y.Array.each(this.charmContainers, function(container) {
73 container.destroy();
74@@ -197,7 +167,6 @@
75 }, '0.1.0', {
76 requires: [
77 'browser-charm-container',
78- 'browser-charm-slider',
79 'browser-charm-token',
80 'browser-search-widget',
81 'juju-charm-store',
82
83=== modified file 'app/subapps/browser/views/view.js'
84--- app/subapps/browser/views/view.js 2013-04-08 15:35:51 +0000
85+++ app/subapps/browser/views/view.js 2013-04-09 14:39:23 +0000
86@@ -309,7 +309,6 @@
87
88 }, '0.1.0', {
89 requires: [
90- 'browser-charm-slider',
91 'browser-charm-token',
92 'browser-search-widget',
93 'juju-charm-store',
94
95=== removed file 'app/widgets/charm-slider.js'
96--- app/widgets/charm-slider.js 2013-04-07 19:07:16 +0000
97+++ app/widgets/charm-slider.js 1970-01-01 00:00:00 +0000
98@@ -1,332 +0,0 @@
99-'use strict';
100-
101-
102-/**
103- * Provides the Charm Slider widget.
104- *
105- * @namespace juju
106- * @module widgets
107- * @submodule browser.CharmSlider
108- *
109- */
110-YUI.add('browser-charm-slider', function(Y) {
111- var sub = Y.Lang.sub,
112- ns = Y.namespace('juju.widgets.browser');
113-
114- /**
115- * The CharmSlider provides a rotating display of one member of a generic set
116- * of items, with controls to go directly to a given item.
117- *
118- * @class CharmSlider
119- * @extends {Y.ScrollView}
120- *
121- */
122- ns.CharmSlider = new Y.Base.create('browser-charm-slider', Y.ScrollView, [], {
123-
124- /**
125- * Template for the CharmSlider
126- *
127- * @property charmSliderTemplate
128- * @type {String}
129- *
130- */
131- charmSliderTemplate: '<ul width="{width}px" />',
132-
133- /**
134- * Template for a given item in the slider
135- *
136- * @property itemTemplate
137- * @type {String}
138- *
139- */
140- itemTemplate: '<li width="{width}px" data-index="{index}" />',
141-
142- /**
143- * Template used for the navigation controls.
144- *
145- * @property prevNavTemplate
146- * @type {String}
147- */
148- navTemplate: '<ul class="navigation"></div>',
149-
150- /**
151- * Template used for items in the navigation.
152- *
153- * @property navItemTemplate
154- * @type {String}
155- */
156- navItemTemplate: '<li data-index="{index}">O</li>',
157-
158- /**
159- * Advances the slider to the next item, or a designated index.
160- *
161- * @method _advanceSlide
162- * @param {string} Index to move to; if not supplied, advances to next
163- * slide.
164- * @private
165- */
166- _advanceSlide: function(index) {
167- var pages = this.pages;
168- if (index) {
169- this._stopTimer();
170- pages.scrollToIndex(index);
171- this._startTimer();
172- } else {
173- index = pages.get('index');
174- if (index < pages.get('total') - 1) {
175- pages.next();
176- } else {
177- pages.scrollToIndex(0);
178- }
179- }
180- },
181-
182- /**
183- * Creates the structure and DOM nodes for the slider.
184- *
185- * @method _generateDOM
186- * @private
187- * @return {Node} The slider's DOM nodes.
188- *
189- */
190- _generateDOM: function() {
191- var width = this.get('width'),
192- slider = Y.Node.create(
193- sub(this.charmSliderTemplate, {width: width}));
194-
195- Y.Array.map(this.get('items'), function(item, index) {
196- var tmpNode = Y.Node.create(
197- sub(this.itemTemplate, {width: width, index: index}));
198- tmpNode.setHTML(item);
199- slider.append(tmpNode);
200- }, this);
201- return slider;
202- },
203-
204- /**
205- * Generates and appends the navigation controls for the slider
206- *
207- * @method _generateSliderControls
208- * @private
209- */
210- _generateSliderControls: function() {
211- var nav = Y.Node.create(this.navTemplate);
212- Y.Array.each(this.get('items'), function(item, index) {
213- nav.append(Y.Node.create(sub(
214- this.navItemTemplate, {index: index})));
215- }, this);
216- this.get('boundingBox').append(nav);
217- },
218-
219- /**
220- * Mouseenter/mouseleave event handler
221- *
222- * @method _pauseAutoAdvance
223- * @private
224- * @param {object} mouseout or mouseover event object.
225- */
226- _pauseAutoAdvance: function(e) {
227- if (e.type === 'mouseenter') {
228- this.set('paused', true);
229- } else {
230- this.set('paused', false);
231- }
232- },
233-
234- /**
235- * Checks to see if autoadvance is set then sets up the timeouts
236- *
237- * @method _startTimer
238- * @private
239- */
240- _startTimer: function() {
241-
242- if (this.get('autoAdvance') === true) {
243- var timer = Y.later(this.get('advanceDelay'), this, function() {
244- if (this.get('paused') !== true) {
245- this._advanceSlide();
246- }
247- }, null, true);
248- this.set('timer', timer);
249- }
250- },
251-
252- /**
253- * Stops the timer for autoadvance
254- *
255- * @method _stopTimer
256- * @private
257- */
258- _stopTimer: function() {
259- var timer = this.get('timer');
260- if (timer) {
261- timer.cancel();
262- }
263- },
264-
265- /**
266- * Binds the navigate event listeners
267- *
268- * @method bindUI
269- * @private
270- */
271- bindUI: function() {
272-
273- //Call the parent bindUI method
274- ns.CharmSlider.superclass.bindUI.apply(this);
275-
276- var events = this.get('_events'),
277- boundingBox = this.get('boundingBox'),
278- nav = boundingBox.one('.navigation');
279- events.push(this.after('render', this._startTimer, this));
280- events.push(boundingBox.on('mouseenter', this._pauseAutoAdvance, this));
281- events.push(boundingBox.on('mouseleave', this._pauseAutoAdvance, this));
282- events.push(nav.delegate('click', function(e) {
283- var index = e.currentTarget.getAttribute('data-index');
284- index = parseInt(index, 10);
285- this._advanceSlide(index);
286- }, 'li', this));
287- },
288-
289- /**
290- * Detaches events attached during instantiation
291- *
292- * @method destructor
293- * @private
294- */
295- destructor: function() {
296- console.log('Clearing slider events');
297- // Stop any in-progress timer
298- this.get('timer').cancel();
299- Y.Array.each(this.get('_events'), function(event) {
300- event.detach();
301- });
302- },
303-
304- /**
305- * Initializer
306- *
307- * @method initializer
308- * @param {Object} The config object.
309- *
310- */
311- initializer: function(cfg) {
312- this.plug(Y.Plugin.ScrollViewPaginator, {
313- selector: 'li'
314- });
315-
316- },
317-
318- /**
319- * Render the nodes and HTML for the slider.
320- *
321- * @method renderUI
322- * @private
323- */
324- renderUI: function() {
325- this.get('contentBox').setHTML(this._generateDOM());
326- this._generateSliderControls();
327- }
328- }, {
329- ATTRS: {
330-
331- /**
332- * @attribute width
333- * @default 200
334- * @type {Int}
335- *
336- */
337- width: {
338- value: 500
339- },
340-
341- /**
342- * @attribute autoAdvance
343- * @default true
344- * @type {Boolean}
345- *
346- */
347- autoAdvance: {
348- value: true
349- },
350-
351- /**
352- * @attribute advanceDelay
353- * @default 3000
354- * @type {Int}
355- *
356- */
357- advanceDelay: {
358- value: 3000
359- },
360-
361- /**
362- * @attribute paused
363- * @default false
364- * @type {Boolean}
365- *
366- */
367- paused: {
368- value: false
369- },
370-
371- /**
372- * @attribute items
373- * @default []
374- * @type {Array}
375- *
376- */
377- items: {
378- value: [],
379- /**
380- * Verify items aren't larger than max value.
381- *
382- * @method validator
383- * @param {Array} The items being validated.
384- */
385- validator: function(val) {
386- return (val.length <= this.get('max'));
387- }
388- },
389-
390- /**
391- * @attribute _events
392- * @default []
393- * @type {Array}
394- *
395- */
396- _events: {
397- value: []
398- },
399-
400- /**
401- * @attribute max
402- * @default 5
403- * @type {Int}
404- *
405- */
406- max: {
407- value: 5
408- },
409-
410- /**
411- * @attribute timer
412- * @default null
413- * @type {Object}
414- *
415- */
416- timer: {
417- value: null
418- }
419- }
420- });
421-
422-}, '0.1.0', {
423- requires: [
424- 'array-extras',
425- 'base',
426- 'event-mouseenter',
427- 'scrollview',
428- 'scrollview-paginator'
429- ]
430-});
431
432=== modified file 'bin/merge-files'
433--- bin/merge-files 2013-03-27 14:54:01 +0000
434+++ bin/merge-files 2013-04-09 14:39:23 +0000
435@@ -64,9 +64,6 @@
436 reqs.push('gallery-timer');
437 reqs.push('loader-base');
438 reqs.push('tabview');
439- // IE conditionals are not resolved automatically by the merge files script
440- // causing our CI tests to fail
441- reqs.push('scrollview-base-ie');
442
443 // Get all of the YUI files and their dependencies
444 var filesToLoad = merge.getYUIFiles(reqs);
445
446=== removed file 'lib/views/browser/charm-slider.less'
447--- lib/views/browser/charm-slider.less 2013-03-08 19:42:38 +0000
448+++ lib/views/browser/charm-slider.less 1970-01-01 00:00:00 +0000
449@@ -1,29 +0,0 @@
450-.yui3-browser-charm-slider {
451-
452- .yui3-browser-charm-slider-content {
453- white-space: nowrap;
454-
455- ul {
456- margin: 0;
457- -moz-padding-start: 0;
458- padding-start: 0;
459- -webkit-padding-start: 0;
460- }
461-
462- li {
463- display: inline-block;
464- text-align: center;
465- vertical-align: middle;
466- }
467- }
468-
469- .navigate {
470- cursor: pointer;
471- list-style-type: none;
472-
473- li {
474- display: block-inline;
475- width: auto;
476- }
477- }
478-}
479
480=== modified file 'lib/views/stylesheet.less'
481--- lib/views/stylesheet.less 2013-04-05 03:07:47 +0000
482+++ lib/views/stylesheet.less 2013-04-09 14:39:23 +0000
483@@ -1647,7 +1647,6 @@
484 @import "browser/charm-container.less";
485 @import "browser/charm-full.less";
486 @import "browser/charm-token.less";
487-@import "browser/charm-slider.less";
488 @import "browser/overlay-indicator.less";
489 @import "browser/tabview.less";
490
491
492=== modified file 'test/index.html'
493--- test/index.html 2013-04-08 05:21:08 +0000
494+++ test/index.html 2013-04-09 14:39:23 +0000
495@@ -41,7 +41,6 @@
496 <script src="test_charm_container.js"></script>
497 <script src="test_charm_panel.js"></script>
498 <script src="test_charm_token.js"></script>
499- <script src="test_charm_slider.js"></script>
500 <script src="test_browser_search_widget.js"></script>
501 <script src="test_charm_store.js"></script>
502 <script src="test_charm_view.js"></script>
503
504=== removed file 'test/test_charm_slider.js'
505--- test/test_charm_slider.js 2013-03-29 17:10:44 +0000
506+++ test/test_charm_slider.js 1970-01-01 00:00:00 +0000
507@@ -1,81 +0,0 @@
508-'use strict';
509-
510-describe('charm slider', function() {
511- var container, Y;
512-
513- before(function(done) {
514- Y = YUI(GlobalConfig).use(
515- ['browser-charm-slider', 'browser-charm-token', 'event-simulate',
516- 'node-event-simulate', 'node', 'scrollview-base-ie'], function(Y) {
517- done();
518- });
519- });
520-
521- beforeEach(function() {
522- container = Y.Node.create('<div id="container"></div>');
523- Y.one('body').prepend(container);
524- });
525-
526- afterEach(function() {
527- container.remove(true);
528- });
529-
530- it('initializes', function() {
531- var cs = new Y.juju.widgets.browser.CharmSlider();
532- assert.isObject(cs);
533- });
534-
535- it('creates the right DOM', function() {
536- var cs = new Y.juju.widgets.browser.CharmSlider(),
537- items = ['foo', 'bar', 'baz'];
538- cs.set('items', items);
539- var sliderDOM = cs._generateDOM();
540- assert.equal(3, sliderDOM.all('li').size());
541- var html = sliderDOM.get('outerHTML');
542- Y.Array.each(items, function(item) {
543- assert.notEqual(-1, html.indexOf(item));
544- });
545- });
546-
547- it('renders', function() {
548- var cs = new Y.juju.widgets.browser.CharmSlider({
549- items: ['<div id="foo"/>']
550- });
551- cs.render(container);
552- assert.isObject(Y.one('#foo'));
553- });
554-
555- it('it generates buttons for each', function() {
556- var cs = new Y.juju.widgets.browser.CharmSlider(),
557- items = ['<div />', '<div />'];
558- cs.set('items', items);
559- cs.render(container);
560- var nav = Y.one('.navigation');
561- assert.equal(items.length, nav.all('li').size());
562- });
563-
564- it('pauses on hover', function() {
565- var cs = new Y.juju.widgets.browser.CharmSlider({items: ['<div/>']});
566- cs.render(container);
567- Y.one('.yui3-browser-charm-slider').simulate('mouseover');
568- assert.isTrue(cs.get('paused'), 'Slider is not paused.');
569- Y.one('.yui3-browser-charm-slider').simulate('mouseout');
570- assert.isFalse(cs.get('paused'), 'Slider is not paused.');
571- });
572-
573- it('goes to the right slide on nav click', function() {
574- var cs = new Y.juju.widgets.browser.CharmSlider({
575- items: ['<div/>', '<div/>'],
576- autoAdvance: false
577- });
578- cs.render(container);
579- assert.equal(
580- 0, cs.pages.get('index'),
581- 'Slider did not start on first slide.');
582- var li = Y.one('.navigation').all('li').pop();
583- li.simulate('click');
584- assert.equal(
585- 1, cs.pages.get('index'),
586- 'Slider did not advance to second slide.');
587- });
588-});

Subscribers

People subscribed via source and target branches