On Wed, Feb 16, 2011 at 1:24 PM, Marius Dumitru Florea < [email protected]> wrote:
Hi Jerome,
On 02/16/2011 01:37 PM, Jerome Velociter wrote:
On Wed, Feb 16, 2011 at 12:12 PM, mflorea <[email protected]>wrote:
Author: mflorea Date: 2011-02-16 12:12:34 +0100 (Wed, 16 Feb 2011) New Revision: 34727
[snip]
Added:
platform/web/trunk/standard/src/main/webapp/resources/uicomponents/widgets/gallery/gallery.js
=================================================================== ---
platform/web/trunk/standard/src/main/webapp/resources/uicomponents/widgets/gallery/gallery.js
(rev 0) +++
platform/web/trunk/standard/src/main/webapp/resources/uicomponents/widgets/gallery/gallery.js
2011-02-16 11:12:34 UTC (rev 34727) @@ -0,0 +1,139 @@ +var XWiki = (function (XWiki) { +// Start XWiki augmentation. +XWiki.Gallery = Class.create({ + initialize : function(container) { + this.images = this._collectImages(container); + + this.container = container.update('<input type="text" tabindex="-1" class="focusCatcher"/><div class="currentImageWrapper"><img class="currentImage" alt="Current image"/></div><div
class="navigation"><div
class="previous" title="Show previous image"><</div><div class="next" title="Show next image">></div><div style="clear:both"></div></div><div class="index">0 / 0</div><div class="maximize" title="Maximize"></div>');
Titles will not be localized here
Right. Thanks for catching this. I forgot about it.
+ this.container.addClassName('xGallery');
+ + this.focusCatcher = this.container.down('.focusCatcher'); + this.focusCatcher.observe('keydown', this._onKeyDown.bindAsEventListener(this)); + this.container.observe('click', function() { + this.focusCatcher.focus(); + }.bind(this)); + + this.container.down('.previous').observe('click', this._onPreviousImage.bind(this)); + this.container.down('.next').observe('click', this._onNextImage.bind(this)); + + this.currentImage = this.container.down('.currentImage'); + this.currentImage.observe('load', this._onLoadImage.bind(this)); + this.currentImage.observe('error', this._onErrorImage.bind(this)); + this.currentImage.observe('abort', this._onAbortImage.bind(this)); + + this.indexDisplay = this.container.down('.index'); + + this.maximizeToggle = this.container.down('.maximize'); + this.maximizeToggle.observe('click', this._onToggleMaximize.bind(this)); + + this.show(0); + }, + _collectImages : function(container) { + var images = []; + var imageElements = container.getElementsByTagName('img');
Why not use container.select('img') ?
[snip]
+ _updateSize : function() { + var dimensions = document.viewport.getDimensions(); + // Remove container padding; + var width = dimensions.width - 20; + var height = dimensions.height - 20; + this.container.style.width = width + 'px'; + this.container.style.height = height + 'px'; + this.currentImage.parentNode.style.height = height + 'px'; + this.currentImage.parentNode.style.lineHeight = height + 'px';
I think as a best practice we should prefer prototype.js accessors versus. a mix of prototype helpers and native DOM properties. It's more consistent and more bug-proof in my opinion. I tend to always assume elements have been extended by prototype, but when the code mixes prototype.js and native accessors, you have greater chances to introduce non-extended elements as variables / members of your class that another developer will maybe miss.
Here you would do
this.currentImage.up().setStyle({ lineHeight: height + 'px' })
I prefer to use native JavaScript APIs whenever possible and use Prototype only when: * it simplifies my code * it fixes a cross browser issue.
You can make a proposal but I'd be -0 for using Prototype in any situation.
I'm curious to know the rationale behind your reasoning. I see mostly advantages in preferring prototype over native APIs : * Browser issues can be tricky, I personally would not assume that I always know when to use prototype to avoid one. I admit I don't test my code in all browsers (Konqueror, Opera, etc. you name it.), but I know prototype.js is extensively tested including in exotic browsers. * Using prototype.js selectors you should get the best of breed of browsers selectors, so overall the performance is better. When you use getElementsByTagName I guess you assume it runs faster than prototype selector. Maybe it's true today (but I would think not by far) but I will pretty likely not be tomorrow (and let's hope by far) - when most browsers will have a good implementation of querySelectorAll (like Opera has for example) ; which prototype selector will certainly rely on (when it's available) in future versions (if not already, haven't checked). The advantage of using prototype.js here is the same argument of having a unified API and hiding implementation details. Native JS APIs is unfortunately always a moving target, since browsers implement specifications at their own pace, not in the same order, etc. Only a using a library's API you can abstract this and be pretty safe your code will run fine everywhere and with maximized performance. * Where do you place the bar ? Why $('element') simplifies your code over .getElementById('element') but not .select('img') over .getElementsByTagName('img') * RE my argument about not extended elements I won't make a proposal to enforce that, but again I'm curious to hear what you think. Jerome.
+ this.currentImage.style.maxHeight = height + 'px'; + // Remove width reserved for the navigation arrows. + this.currentImage.style.maxWidth = (width - 128) + 'px'; + }, + _resetSize : function() { + this.container.style.cssText = ''; + this.container.removeAttribute('style'); + this.currentImage.parentNode.style.cssText = ''; + this.currentImage.parentNode.removeAttribute('style'); + this.currentImage.style.cssText = ''; + this.currentImage.removeAttribute('style'); + }, + show : function(index) { + if (index< 0 || index>= this.images.length || index == this.index)
{
+ return; + } + this.currentImage.style.visibility = 'hidden'; + Element.addClassName(this.currentImage.parentNode, 'loading'); + this.currentImage.title = this.images[index].title; + this.currentImage.src = this.images[index].url; + this.index = index; + this.indexDisplay.firstChild.nodeValue = (index + 1) + ' / ' + this.images.length;
Same for firstChild.nodeValue, I would prefer .down().update()
+ } +}); +// End XWiki augmentation. +return XWiki; +}(XWiki || {})); + +Element.observe(document, "dom:loaded", function() { + $$('.gallery').each(function(gallery) {
Same remark as for the dashboard macro, IMO this is too greedy and $mainContentArea.select('.gallery') would perform better.
The problem is not really each such selector taken individually, but if we continue adding more and more, it will make the loading after DOM is loaded clumsy.
I agree, but I don't like the fact that gallery.js would depend on the presence of the mainContentArea element. Is this element part of the public "API"?
Thanks, Marius
Jerome.
+ new XWiki.Gallery(gallery); + }); +});
Property changes on:
platform/web/trunk/standard/src/main/webapp/resources/uicomponents/widgets/gallery/gallery.js
___________________________________________________________________ Added: svn:keywords + Author Id Revision HeadURL Added: svn:eol-style + native
Added:
platform/web/trunk/standard/src/main/webapp/resources/uicomponents/widgets/gallery/loading.gif
=================================================================== (Binary files differ)
Property changes on:
platform/web/trunk/standard/src/main/webapp/resources/uicomponents/widgets/gallery/loading.gif
___________________________________________________________________ Added: svn:mime-type + image/gif
Added:
platform/web/trunk/standard/src/main/webapp/resources/uicomponents/widgets/gallery/maximize.gif
=================================================================== (Binary files differ)
Property changes on:
platform/web/trunk/standard/src/main/webapp/resources/uicomponents/widgets/gallery/maximize.gif
___________________________________________________________________ Added: svn:mime-type + image/gif
Added:
platform/web/trunk/standard/src/main/webapp/resources/uicomponents/widgets/gallery/minimize.gif
=================================================================== (Binary files differ)
Property changes on:
platform/web/trunk/standard/src/main/webapp/resources/uicomponents/widgets/gallery/minimize.gif
___________________________________________________________________ Added: svn:mime-type + image/gif
_______________________________________________ notifications mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/notifications
_______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs