Having problems with MochiKit.Visual.blindDown

[Michael Bond-7]

Ran into MochiKit while trying out PlotKit, looks like it has a lot of
useful tools but am just getting started with it.

In trying to use the blind/slide effects available in trunk svn, I
noticed that the blindDown/slideDown don't seem to 'slide', but rather
just snap open, though blindUp/slideUp seem to work as expected. I've
noticed the same on the demo page http://mochikit.com/examples/effects/effects_blind.html.
The odd thing is that it all seems to work correctly on the demo page
at http://mochikit.com/examples/effects/index.html so not quite sure
what is wrong.

One useful addition would be the ability to slide sideways. One use of
this would be to 'slide' from one graph to another when switching tabs
on mochibot demo at http://www.mochibot.com/my/dashboard.html. One
good example of this is http://www.panic.com/coda/.

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "MochiKit" group.
To post to this group, send email to mochikit@...
To unsubscribe from this group, send email to mochikit-unsubscribe@...
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---

[Michael Bond-7]

I've played with this a bit more and have found that the
scaleMode.originalHeight and scaleMode.originalWidth aren't getting
set properly if the original values are 'auto' and the element is
hidden prior to the call to blindDown (and I suspect slideDown, etc
suffer the same problem). I was able to get around this by adding a
beforeSetupInternal to blindDown which shows the element prior to
getting the computed width and height. I also found that the
makeClipping call had to be moved as well or the computed height
didn't come out right. Below is the change to get it working:

MochiKit.Visual.blindDown = function (element, /* optional */ options)
{
    /***

    Blind an element down: restore its vertical size.

    ***/
    var d = MochiKit.DOM;
    var s = MochiKit.Style;
    element = d.getElement(element);
    var elemClip;
    options = MochiKit.Base.update({
        scaleContent: false,
        scaleX: false,
        scaleFrom: 0,
        restoreAfterFinish: true,
        beforeSetupInternal: function (effect) {
            elemClip = d.makeClipping(effect.element);
            s.showElement(effect.element);
            effect.options.scaleMode = {originalHeight:
s.getElementHeight(element),
                originalWidth: s.getElementWidth(element)};
        },
        afterSetupInternal: function (effect) {
            s.setStyle(effect.element, {height: '0px'});
        },
        afterFinishInternal: function (effect) {
            d.undoClipping(effect.element, elemClip);
        }
    }, options);
    return new MochiKit.Visual.Scale(element, 100, options);
};

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "MochiKit" group.
To post to this group, send email to mochikit@...
To unsubscribe from this group, send email to mochikit-unsubscribe@...
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---

[Michael Bond-7]

Just tested slideDown, it exhibits the same behavior. I've put in
essentially the same fix but it seems to have a horizontal positioning
problem if the element starts out hidden until the element is
completely revealed.


/** @id MochiKit.Visual.slideDown */
MochiKit.Visual.slideDown = function (element, /* optional */ options)
{
    /***

    Slide an element down.
    It needs to have the content of the element wrapped in a container
    element with fixed height.

    ***/
    var d = MochiKit.DOM;
    var b = MochiKit.Base;
    var s = MochiKit.Style;
    element = d.getElement(element);
    if (!element.firstChild) {
        throw "MochiKit.Visual.slideDown must be used on a element
with a child";
    }
    d.removeEmptyTextNodes(element);
    var oldInnerBottom = s.getStyle(element.firstChild, 'bottom') ||
0;
    var elemClip;
    options = b.update({
        scaleContent: false,
        scaleX: false,
        scaleFrom: 0,
        restoreAfterFinish: true,
        beforeSetupInternal: function (effect) {
            elemClip = d.makeClipping(effect.element);
            s.showElement(effect.element);
            scaleMode = {originalHeight: s.getElementHeight(element),
                originalWidth: s.getElementWidth(element)},
        },
        afterSetupInternal: function (effect) {
            d.makePositioned(effect.element);
            d.makePositioned(effect.element.firstChild);
            if (/Opera/.test(navigator.userAgent)) {
                s.setStyle(effect.element, {top: ''});
            }
            s.setStyle(effect.element, {height: '0px'});
        },
        afterUpdateInternal: function (effect) {
            s.setStyle(effect.element.firstChild,
               {bottom: (effect.dims[0] -
s.getElementHeight(effect.element)) + 'px'});
        },
        afterFinishInternal: function (effect) {
            d.undoClipping(effect.element, elemClip);
            // IE will crash if child is undoPositioned first
            if (/MSIE/.test(navigator.userAgent)) {
                d.undoPositioned(effect.element);
                d.undoPositioned(effect.element.firstChild);
            } else {
                d.undoPositioned(effect.element.firstChild);
                d.undoPositioned(effect.element);
            }
            s.setStyle(effect.element.firstChild, {bottom:
oldInnerBottom});
        }
    }, options);

    return new MochiKit.Visual.Scale(element, 100, options);
};

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "MochiKit" group.
To post to this group, send email to mochikit@...
To unsubscribe from this group, send email to mochikit-unsubscribe@...
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---

[Michael Bond-7]

Here's an updated blindDown, had a syntax error that showed up in
Firefox but not in Safari/WebKit:


/** @id MochiKit.Visual.slideDown */
MochiKit.Visual.slideDown = function (element, /* optional */ options)
{
    /***

    Slide an element down.
    It needs to have the content of the element wrapped in a container
    element with fixed height.

    ***/
    var d = MochiKit.DOM;
    var b = MochiKit.Base;
    var s = MochiKit.Style;
    element = d.getElement(element);
    if (!element.firstChild) {
        throw "MochiKit.Visual.slideDown must be used on a element
with a child";
    }
    d.removeEmptyTextNodes(element);
    var oldInnerBottom = s.getStyle(element.firstChild, 'bottom') ||
0;
    var elemClip;
    options = b.update({
        scaleContent: false,
        scaleX: false,
        scaleFrom: 0,
        restoreAfterFinish: true,
        beforeSetupInternal: function (effect) {
            elemClip = d.makeClipping(effect.element);
            s.showElement(effect.element);
            scaleMode = {originalHeight: s.getElementHeight(element),
                originalWidth: s.getElementWidth(element)};
        },
        afterSetupInternal: function (effect) {
            d.makePositioned(effect.element);
            d.makePositioned(effect.element.firstChild);
            if (/Opera/.test(navigator.userAgent)) {
                s.setStyle(effect.element, {top: ''});
            }
            s.setStyle(effect.element, {height: '0px'});
        },
        afterUpdateInternal: function (effect) {
            s.setStyle(effect.element.firstChild,
               {bottom: (effect.dims[0] -
s.getElementHeight(effect.element)) + 'px'});
        },
        afterFinishInternal: function (effect) {
            d.undoClipping(effect.element, elemClip);
            // IE will crash if child is undoPositioned first
            if (/MSIE/.test(navigator.userAgent)) {
                d.undoPositioned(effect.element);
                d.undoPositioned(effect.element.firstChild);
            } else {
                d.undoPositioned(effect.element.firstChild);
                d.undoPositioned(effect.element);
            }
            s.setStyle(effect.element.firstChild, {bottom:
oldInnerBottom});
        }
    }, options);

    return new MochiKit.Visual.Scale(element, 100, options);
};

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "MochiKit" group.
To post to this group, send email to mochikit@...
To unsubscribe from this group, send email to mochikit-unsubscribe@...
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---

[Per Cederberg-2]

Thanks for all your work on this! We very much appreciate patches and
bug reports, although people are sometimes too busy to respond to
email. :-(

After investigating this, I think the main problem here was introduced
in revision 1383. That change introduced the new functions
MochiKit.Style.getElementHeight and MochiKit.Style.getElementWidth and
changed MochiKit.Visual to use them in several places. There are
apparently issues with these functions and they are also lacking test
cases.

I've been discussing the design of these with Christoph Zwerschke on
the list a bit. Perhaps you could read up a bit on this so that I
don't have to repeat myself here?

Anyway. I think we should rollback most of rev 1383 to avoid these
problems. We'll then get back the issues with paddings, but that seems
less problematic. The best way forward would be to improve
getElementDimensions() in a fully backwards-compatible way to
correctly handle this.

Anybody up for the task?

Cheers,

/Per

On Tue, Jun 10, 2008 at 6:50 PM, Michael Bond <mikejbond@...> wrote:

>
> Here's an updated blindDown, had a syntax error that showed up in
> Firefox but not in Safari/WebKit:
>
>
> /** @id MochiKit.Visual.slideDown */
> MochiKit.Visual.slideDown = function (element, /* optional */ options)
> {
>    /***
>
>    Slide an element down.
>    It needs to have the content of the element wrapped in a container
>    element with fixed height.
>
>    ***/
>    var d = MochiKit.DOM;
>    var b = MochiKit.Base;
>    var s = MochiKit.Style;
>    element = d.getElement(element);
>    if (!element.firstChild) {
>        throw "MochiKit.Visual.slideDown must be used on a element
> with a child";
>    }
>    d.removeEmptyTextNodes(element);
>    var oldInnerBottom = s.getStyle(element.firstChild, 'bottom') ||
> 0;
>    var elemClip;
>    options = b.update({
>        scaleContent: false,
>        scaleX: false,
>        scaleFrom: 0,
>        restoreAfterFinish: true,
>        beforeSetupInternal: function (effect) {
>            elemClip = d.makeClipping(effect.element);
>            s.showElement(effect.element);
>            scaleMode = {originalHeight: s.getElementHeight(element),
>                originalWidth: s.getElementWidth(element)};
>        },
>        afterSetupInternal: function (effect) {
>            d.makePositioned(effect.element);
>            d.makePositioned(effect.element.firstChild);
>            if (/Opera/.test(navigator.userAgent)) {
>                s.setStyle(effect.element, {top: ''});
>            }
>            s.setStyle(effect.element, {height: '0px'});
>        },
>        afterUpdateInternal: function (effect) {
>            s.setStyle(effect.element.firstChild,
>               {bottom: (effect.dims[0] -
> s.getElementHeight(effect.element)) + 'px'});
>        },
>        afterFinishInternal: function (effect) {
>            d.undoClipping(effect.element, elemClip);
>            // IE will crash if child is undoPositioned first
>            if (/MSIE/.test(navigator.userAgent)) {
>                d.undoPositioned(effect.element);
>                d.undoPositioned(effect.element.firstChild);
>            } else {
>                d.undoPositioned(effect.element.firstChild);
>                d.undoPositioned(effect.element);
>            }
>            s.setStyle(effect.element.firstChild, {bottom:
> oldInnerBottom});
>        }
>    }, options);
>
>    return new MochiKit.Visual.Scale(element, 100, options);
> };
>
> >
>

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "MochiKit" group.
To post to this group, send email to mochikit@...
To unsubscribe from this group, send email to mochikit-unsubscribe@...
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---

[Christoph Zwerschke]

Per Cederberg schrieb:
 > After investigating this, I think the main problem here was introduced
 > in revision 1383. That change introduced the new functions
 > MochiKit.Style.getElementHeight and MochiKit.Style.getElementWidth and
 > changed MochiKit.Visual to use them in several places. There are
 > apparently issues with these functions and they are also lacking test
 > cases.

Maybe we can solve this by calculating getElementHeight in a different
way, as jQuery does it, namely by using the existing getDimensions
values and then subtracting padding and border (see also
http://trac.mochikit.com/ticket/84). But I agree we also need more tests
  for these functions. I will try to look at this again this weekend.

-- Christoph

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "MochiKit" group.
To post to this group, send email to mochikit@...
To unsubscribe from this group, send email to mochikit-unsubscribe@...
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---

[Per Cederberg-2]

Ok, it would be great if you looked at this!

I did something similar once, but it is a bit of a pain to read 4
computed values (which is slow) instead of just using offsetHeight.
But since it is just used during initialization, speed won't be a
problem here. But we should still keep it separate from
getElementDimensions for performance reasons I think.

Cheers,

/Per

On Wed, Jun 11, 2008 at 10:50 AM, Christoph Zwerschke <cito@...> wrote:

>
> Per Cederberg schrieb:
>  > After investigating this, I think the main problem here was introduced
>  > in revision 1383. That change introduced the new functions
>  > MochiKit.Style.getElementHeight and MochiKit.Style.getElementWidth and
>  > changed MochiKit.Visual to use them in several places. There are
>  > apparently issues with these functions and they are also lacking test
>  > cases.
>
> Maybe we can solve this by calculating getElementHeight in a different
> way, as jQuery does it, namely by using the existing getDimensions
> values and then subtracting padding and border (see also
> http://trac.mochikit.com/ticket/84). But I agree we also need more tests
>  for these functions. I will try to look at this again this weekend.
>
> -- Christoph
>
> >
>

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "MochiKit" group.
To post to this group, send email to mochikit@...
To unsubscribe from this group, send email to mochikit-unsubscribe@...
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---

[Christoph Zwerschke]

Per Cederberg schrieb:
 > I did something similar once, but it is a bit of a pain to read 4
 > computed values (which is slow) instead of just using offsetHeight.
 > But since it is just used during initialization, speed won't be a
 > problem here. But we should still keep it separate from
 > getElementDimensions for performance reasons I think.

Yes. Maybe we should just add an additional function
getFullElementDimensions that returns the same as getElementDimensions,
and additionally also border, margin and padding plus the calculated
"real" width and height. Of course this will be slow, but it will not be
used very often. But the initialization of the visual effects is a use
case. And setElementDimensions should recognize the additional values
and operate correctly if they are present, otherwise behave like before.

Instead of an additional function, we can also consider a boolean
parameter to getElementDimensions for returning the additional values.

-- Christoph

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "MochiKit" group.
To post to this group, send email to mochikit@...
To unsubscribe from this group, send email to mochikit-unsubscribe@...
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---

[Per Cederberg-2]

I like the idea of adding a boolean parameter to
getElementDimensions(). But we needn't return the actual values found
I think. Like this:

getElementDimensions(element[,innerSize=false])

So calling getElementDimensions(elem, true) would return the inner
dimensions instead of the outer (as the current version). That way, no
change would be needed to setElementDimensions at all.

Cheers,

/Per

On Wed, Jun 11, 2008 at 1:32 PM, Christoph Zwerschke <cito@...> wrote:

>
> Per Cederberg schrieb:
>  > I did something similar once, but it is a bit of a pain to read 4
>  > computed values (which is slow) instead of just using offsetHeight.
>  > But since it is just used during initialization, speed won't be a
>  > problem here. But we should still keep it separate from
>  > getElementDimensions for performance reasons I think.
>
> Yes. Maybe we should just add an additional function
> getFullElementDimensions that returns the same as getElementDimensions,
> and additionally also border, margin and padding plus the calculated
> "real" width and height. Of course this will be slow, but it will not be
> used very often. But the initialization of the visual effects is a use
> case. And setElementDimensions should recognize the additional values
> and operate correctly if they are present, otherwise behave like before.
>
> Instead of an additional function, we can also consider a boolean
> parameter to getElementDimensions for returning the additional values.
>
> -- Christoph
>
> >
>

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "MochiKit" group.
To post to this group, send email to mochikit@...
To unsubscribe from this group, send email to mochikit-unsubscribe@...
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---

[Christoph Zwerschke]

Per Cederberg schrieb:
> getElementDimensions(element[,innerSize=false])

Yes, that's what I meant.

Or, to make clearer what is returned, we could use withPadding=true
instead of innerSize=false. We could then also provide additional
parameters withPadding and withMargin, or just a single parameter
extend=2 which can be 0, 1, 2, 3 for inner, padding, border, margin.

We could also provide additional functions for getting the padding,
border and margin as suggested in #84, and these could be use by
getElementDimensions internally.

-- Christoph


--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "MochiKit" group.
To post to this group, send email to mochikit@...
To unsubscribe from this group, send email to mochikit-unsubscribe@...
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---

[Christoph Zwerschke]

Christoph Zwerschke schrieb:
> Or, to make clearer what is returned, we could use withPadding=true
> instead of innerSize=false.

Sorry, I meant withBorder=true. getElementDimensions returns the size
including padding and border.

-- Christoph

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "MochiKit" group.
To post to this group, send email to mochikit@...
To unsubscribe from this group, send email to mochikit-unsubscribe@...
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---

[Per Cederberg-2]

Yes, well. I was thinking simpler is better. So I though a single flag
would be sufficient to either remove both padding and borders, or to
keep the values unchanged (as today). If someone wants more detail,
there is always the helper functions for retrieving padding and border
widths.

I was kind of thinking to include those first in version 1.5, but now
that they seem important in order to fix this bug I think we might as
well include them in 1.4. Need to create some good test cases for all
of this though. And docs. Feel free to fix these issues if you have
time during the weekend. I'll let you know if I commit anything to svn
related to this.

Cheers,

/Per

On Wed, Jun 11, 2008 at 6:33 PM, Christoph Zwerschke <cito@...> wrote:

>
> Christoph Zwerschke schrieb:
>> Or, to make clearer what is returned, we could use withPadding=true
>> instead of innerSize=false.
>
> Sorry, I meant withBorder=true. getElementDimensions returns the size
> including padding and border.
>
> -- Christoph
>
> >
>

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "MochiKit" group.
To post to this group, send email to mochikit@...
To unsubscribe from this group, send email to mochikit-unsubscribe@...
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---

[Christoph Zwerschke]

Per Cederberg schrieb:
 > Yes, well. I was thinking simpler is better. So I though a single flag
 > would be sufficient to either remove both padding and borders, or to
 > keep the values unchanged (as today). If someone wants more detail,
 > there is always the helper functions for retrieving padding and border
 > widths.

Ok, I agree a single parameter with default set for the current behavior
is best. Let me know which you prefer of innerWidth=False or
withBorder=True or extend=2 (where extend can be 0, 1, 2, 3 for inner,
with padding, border, margin).

 > I was kind of thinking to include those first in version 1.5, but now
 > that they seem important in order to fix this bug I think we might as
 > well include them in 1.4. Need to create some good test cases for all
 > of this though. And docs. Feel free to fix these issues if you have
 > time during the weekend. I'll let you know if I commit anything to svn
 > related to this.

Ok. You are referring to http://trac.mochikit.com/ticket/84 here, right?
Btw, I think the name getPaddingBox suggested there for getting only the
padding is misleading; I would think that getPaddingBox should return
getElementDimensinos(extend=1) as discussed above.

-- Chris

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "MochiKit" group.
To post to this group, send email to mochikit@...
To unsubscribe from this group, send email to mochikit-unsubscribe@...
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---

[Christoph Zwerschke]

Michael Bond schrieb:
 > Here's an updated blindDown, had a syntax error that showed up in
 > Firefox but not in Safari/WebKit:

Mike, I just refactored the computation of element sizes in r1388. Can
you have a look at whether this also solves the problems with blindDown
or whether your patch is still necessary?

-- Christoph

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "MochiKit" group.
To post to this group, send email to mochikit@...
To unsubscribe from this group, send email to mochikit-unsubscribe@...
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---

[Christoph Zwerschke]

Per Cederberg schrieb:
> Yes, well. I was thinking simpler is better. So I though a single flag
> would be sufficient to either remove both padding and borders, or to
> keep the values unchanged (as today). If someone wants more detail,
> there is always the helper functions for retrieving padding and border
> widths.

Eventually found some time and implemented it like that in r1388.

-- Christoph

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "MochiKit" group.
To post to this group, send email to mochikit@...
To unsubscribe from this group, send email to mochikit-unsubscribe@...
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---

[Michael Bond-7]

Sorry I didn't get back sooner, I've been busy with many other things.
I'm not primarily a web developer so but I still tend to do a fair
amount of it for my company. At this point the various blind and slide
functions are working fine on the three platforms I've been testing on
(Firefox 3, Safari 3.1 and IE 7). I've updated the various slide left/
right functions to work on all three as well, the last patch had
problems on IE 7. I've emailed the patch to Per Cederberg as I don't
have access to upload patches to trac direction, the original ticket #
is 312, http://trac.mochikit.com/ticket/312. It's still fairly ugly
but I've updated it to use the getElementDimensions and such as
discussed.

I still have one unresolved issue with Firefox 3 "jumping" when the
animation starts, haven't really had the time to look into that. I've
got an example of it in action at http://clicksense.lucidmedia.com:8080/clicksense/demo.
This is a staging url so not sure how long it's going to be active. If
anyone can figure out why the sliding jumps when the animation starts
I would love to know if I don't get to it myself. It may well have to
do with the inner dimensions from getElementDimensions but not sure
and at the moment am tied up in some other things.

On Jun 21, 2:59 pm, Christoph Zwerschke <c...@...> wrote:
> Michael Bond schrieb:
>  > Here's an updated blindDown, had a syntax error that showed up in
>  > Firefox but not in Safari/WebKit:
>
> Mike, I just refactored the computation of element sizes in r1388. Can
> you have a look at whether this also solves the problems with blindDown
> or whether your patch is still necessary?
>
> -- Christoph

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "MochiKit" group.
To post to this group, send email to mochikit@...
To unsubscribe from this group, send email to mochikit-unsubscribe@...
For more options, visit this group at http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---