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"?
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