lewisje / ourlibrary

Automatically exported from code.google.com/p/ourlibrary
0 stars 0 forks source link

changeImage always attaches overlay to body element #8

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
If effects are available, the new changeImage function creates an overlay
and attaches it to the body element. This could cause issues if the image's
parent element has overflow set to hidden and the image overflows the
container dimensions. As the overlay gets attached to the body, while the
effect is transitioning, the image overflows its container.

My proposed new version of that function follows below.

if (typeof changeImage == 'function') {

        oldChangeImage = changeImage;

        changeImage = (function() {

          if (canAdjustStyle.visibility && canAdjustStyle.display && body
&& isHostMethod(body, 'cloneNode') && isHostMethod(body, 'appendChild') &&
isHostMethod(body, 'removeChild')) {

            return function(el, src, options, fnDone) {

              var elTemp;

              var docNode = getElementDocument(el);

              var parent = getElementParentElement(el) ||
getBodyElement(docNode);

              var done = function() {

                oldChangeImage(el, src);

                showElement(elTemp, false);

                body.removeChild(elTemp);

                elTemp = null;

              };

              if (options && options.effects) {

                elTemp = el.cloneNode(false);

                elTemp.id = elementUniqueId(el) + '_temporaryoverlay';

                oldChangeImage(elTemp, src);

                elTemp.style.visibility = 'hidden';

                elTemp.style.display = 'none';

                elTemp.style.position = 'absolute';

                elTemp.style.left = elTemp.style.top = '0';

                parent.appendChild(elTemp);

                elTemp.style.display = 'block';

                overlayElement(elTemp, el);

                showElement(elTemp, true, options, function() { done(); if
(fnDone) { fnDone(el); } });

              }

              else {

                oldChangeImage(el, src);

                if (fnDone) { fnDone(el); }

              }

            };

          }

          return oldChangeImage;

        })();

Additional feature testing may be needed, as I'm not sure if the fact that
body has appendChild assures that the element's parent node also has that
method available.

Original issue reported on code.google.com by gabrielg...@gmail.com on 30 Mar 2010 at 5:46

GoogleCodeExporter commented 9 years ago
> var done = function() {
>    oldChangeImage(el, src);
>    showElement(elTemp, false);
>    body.removeChild(elTemp);
     ^^^^
     oops, this should be parent, my bad.
>    elTemp = null;
> };

I'm attaching the tested snippet, since posting the code here introduces all 
those
line breaks.

Original comment by gabrielg...@gmail.com on 30 Mar 2010 at 5:58

Attachments:

GoogleCodeExporter commented 9 years ago
I need to think about this a bit and review your proposal. In the case of a 
positioned 
ancestor, it does make sense to append the overlay image to the ancestor rather 
than 
the body.

Original comment by dmark.ci...@gmail.com on 30 Mar 2010 at 4:30

GoogleCodeExporter commented 9 years ago
One thing I suggest is using the (unfortunately named) getPositionedParent 
(should be 
getPositionedAncestor to be accurate), rather than getElementParentElement.

I will make the necessary changes prior to the release

Original comment by dmark.ci...@gmail.com on 30 Mar 2010 at 5:10

GoogleCodeExporter commented 9 years ago
I've changed to getPositionedParent and everything worked fine in my example. 
Thanks :)

Original comment by gabrielg...@gmail.com on 30 Mar 2010 at 5:25

GoogleCodeExporter commented 9 years ago

Original comment by dmark.ci...@gmail.com on 13 May 2010 at 9:40