Re: [xwiki-devs] [xwiki-notifications] r34727 - in platform/web/trunk/standard/src/main/webapp/resources/uicomponents/widgets: . gallery
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
Added:
platform/web/trunk/standard/src/main/webapp/resources/uicomponents/widgets/gallery/
platform/web/trunk/standard/src/main/webapp/resources/uicomponents/widgets/gallery/gallery.css
platform/web/trunk/standard/src/main/webapp/resources/uicomponents/widgets/gallery/gallery.js
platform/web/trunk/standard/src/main/webapp/resources/uicomponents/widgets/gallery/loading.gif
platform/web/trunk/standard/src/main/webapp/resources/uicomponents/widgets/gallery/maximize.gif
platform/web/trunk/standard/src/main/webapp/resources/uicomponents/widgets/gallery/minimize.gif Log: XWIKI-6010: Add a gallery macro to display images using a slide-show view * Add resources to uicomponents/widgets/gallery
Added: platform/web/trunk/standard/src/main/webapp/resources/uicomponents/widgets/gallery/gallery.css =================================================================== --- platform/web/trunk/standard/src/main/webapp/resources/uicomponents/widgets/gallery/gallery.css (rev 0) +++ platform/web/trunk/standard/src/main/webapp/resources/uicomponents/widgets/gallery/gallery.css 2011-02-16 11:12:34 UTC (rev 34727) @@ -0,0 +1,146 @@ +#macro(ieOpacity $selector $value) +/* start IE opacity setting */ +$selector { + /* We need to quote the property value because the equal sign is not valid inside a CSS property value. */ + -ms-filter: "progid:DXImageTransform.Microsoft.Alpha(Opacity=$value)"; +} + +#foreach($item in $selector.split('\s*,\s*')) +*+html $item, ## +#end { + /* This is for IE7, which doesn't accept the quoted value but fortunately understands the escape syntax. */ + filter: alpha(opacity\=$value); + /* Unicode escape works too. */ + /*filter: alpha(opacity\003d $value);*/ +} + +#foreach($item in $selector.split('\s*,\s*')) +* html $item, ## +#end { + /* This is for IE6, which doesn't understand the escaped equal sign (not even the unicode escape) but fortunately accepts the quoted value. */ + filter: "progid:DXImageTransform.Microsoft.Alpha(Opacity=$value)"; +} +/* end IE opacity setting */ +#end + +#set($imageMaxWidth = 492) +#set($imageMaxHeight = 369) + +.xGallery { + background-color: black; + padding: 10px; + position: relative; + width: ${mathtool.add($imageMaxWidth, 128)}px; +} + +.xGallery.maximized { + left: 0; + position: absolute; + top: 0; + z-index: 1001; +} + +.xGallery .currentImageWrapper { + height: ${imageMaxHeight}px; + line-height: ${imageMaxHeight}px; + text-align: center; + width: 100%; +} + +.xGallery .currentImage { + max-height: ${imageMaxHeight}px; + max-width: ${imageMaxWidth}px; + vertical-align: middle; +} + +.xGallery .navigation { + left: 0; + margin-top: -64px; + position: absolute; + top: 50%; + width: 100%; +} + +.xGallery .previous, .xGallery .next { + color: #A0A0A0; + cursor: pointer; + font-family: courier,monospace; + font-size: 32px; + font-weight: 100; + height: 124px; + line-height: 124px; + text-align: center; + width: 32px; +} + +.xGallery .previous:hover, .xGallery .next:hover { + color: white; +} + +.xGallery .previous { + float: left; +} + +.xGallery .next { + float: right; +} + +.xGallery .index { + bottom: 10px; + color: #C0C0C0; + font-family: sans-serif; + font-size: smaller; + left: 10px; + position: absolute; +} + +.xGallery .loading { + background-image: url('loading.gif') !important; +} + +.xGallery .focusCatcher { + background-color: black; + border: 0 none; + color: black; + height: 1px; + left: 0; + overflow: hidden; + position: absolute; + top: 0; + width: 1px; + z-index: -1; +} + +.xGallery .maximize, .xGallery .minimize { + cursor: pointer; + height: 16px; + opacity: .5; + position: absolute; + right: 10px; + top: 10px; + width: 16px; +} + +.xGallery .maximize:hover, .xGallery .minimize:hover { + opacity: 1; +} + +#ieOpacity('.xGallery .maximize, .xGallery .minimize' 50) + +#ieOpacity('.xGallery .maximize:hover, .xGallery .minimize:hover' 100) + +.xGallery .maximize { + background: transparent url('maximize.gif') no-repeat scroll center; +} + +.xGallery .minimize { + background: transparent url('minimize.gif') no-repeat scroll center; +} + +html.maximized, html.maximized body { + height: 100% !important; + margin: 0 !important; + overflow: hidden !important; + padding: 0 !important; + width: 100% !important; +}
Property changes on: platform/web/trunk/standard/src/main/webapp/resources/uicomponents/widgets/gallery/gallery.css ___________________________________________________________________ Added: svn:keywords + Author Id Revision HeadURL Added: svn:eol-style + native
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 + 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') ?
+ for(var i = 0; i < imageElements.length; i++) { + var imageElement = imageElements[i]; + images.push({url: imageElement.getAttribute('src'), title: imageElement.title}); + imageElement.removeAttribute('src'); + } + return images; + }, + _onPreviousImage : function() { + this.show(this.index > 0 ? this.index - 1 : this.images.length - 1); + }, + _onNextImage : function() { + this.show(this.index < this.images.length - 1 ? this.index + 1 : 0); + }, + _onLoadImage : function() { + Element.removeClassName(this.currentImage.parentNode, 'loading'); + this.currentImage.style.visibility = 'visible'; + }, + _onErrorImage: function() { + }, + _onAbortImage: function() { + }, + _onKeyDown : function(event) { + var stop = true; + switch(event.keyCode) { + case Event.KEY_LEFT: + this._onPreviousImage(); + break; + case Event.KEY_RIGHT: + this._onNextImage(); + break; + case Event.KEY_HOME: + this.show(0); + break; + case Event.KEY_END: + this.show(this.images.length - 1); + break; + case Event.KEY_ESC: + if (this.container.hasClassName('maximized')) { + this._onToggleMaximize(); + } + break; + case 70: /* F */ + this._onToggleMaximize(); + break; + defualt: + stop = false; + break; + } + if (stop) { + Event.stop(event); + } + }, + _onToggleMaximize : function() { + this.maximizeToggle.toggleClassName('maximize'); + this.maximizeToggle.toggleClassName('minimize'); + this.maximizeToggle.title = this.maximizeToggle.hasClassName('maximize') ? 'Maximize' : 'Minimize'; + this.container.toggleClassName('maximized'); + $(document.documentElement).toggleClassName('maximized'); + if (this.container.hasClassName('maximized')) { + this._updateSize(); + } else { + this._resetSize(); + } + }, + _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' })
+ 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. 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
On 02/16/2011 12:37 PM, Jerome Velociter wrote:
+ this.indexDisplay.firstChild.nodeValue = (index + 1) + ' / ' + this.images.length;
Same for firstChild.nodeValue, I would prefer .down().update()
Not quite, indexDisplay.firstChild is a text node, so down() won't select it. this.indexDisplay.update((index + 1) + ' / ' + this.images.length); -- Sergiu Dumitriu http://purl.org/net/sergiu/
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
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
On 02/16/2011 03:06 PM, Jerome Velociter wrote: [snip]
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.
I guess my first reason is that I know the native API better than the Prototype one so it's natural that I tend to use the former. When I'm pretty sure that a native function/property works fine cross-browser (because I've used it many time in the past) I don't spend time looking for an alternative in the Prototype documentation. That's why I use parentNode rather than up() and firstChild rather than down() even if they are longer. Then I know that more code means more bugs, so even if Prototype is tested extensively it can have bugs which means that when something is not working as expected in my code (using Prototype) I have to check their code also before blaming the browser. Regarding performance, I don't think Prototype can ever beat the native API since it is relying on it. I'm pretty sure that browsers that implement querySelectorAll use it internally for getElementsByTagName. Thanks, Marius
Jerome.
[snip]
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.
Personally I prefer to always use Prototype whenever possible, as indeed mixing Prototype with native methods created a lot of problems. It's true that sometimes it looks like unneeded overhead, but it's hard to predict how variables will be used later. IMO, the performance gain is not worth the effort of analyzing whether or not an object is extended or not. And it helps having a uniform coding style.
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"?
Not officially. -- Sergiu Dumitriu http://purl.org/net/sergiu/
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
On 02/16/2011 04:27 PM, Jerome Velociter wrote: [snip]
>> +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.
The problem is that the gallery widget needs to work when the office presentation is viewed stand-alone, i.e. using the office.vm Velocity template (e.g. when you click on the eye icon next to an office attachment, if you have the office server running). I could add a 'xwikicontent' div there or use this id on the body element but it looks like a hack to me. Thanks, Marius
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.
participants (3)
-
Jerome Velociter -
Marius Dumitru Florea -
Sergiu Dumitriu