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. > > > > > > >> + 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"? > Yes maybe $('xwikicontent') is more appropriate since it is implemented by all skins AFAIR. Right now we are not really clear on what's API and what's not, although there is already a lot of code relying on xwikicontent (CSS, JS, etc.) so it's kind of an API de facto. Jerome. > > 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 > _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
