Safer use of "anim" as precursor to "Strip out non-essential core code into plugins"

2 views
Skip to first unread message

Martin Budden

unread,
May 15, 2006, 2:18:20 PM5/15/06
to TiddlyWikiDev
Jeremy,

I noticed in ticket #15 you're planning to allow a smaller core by
extracting out non-essentials such as animation.

There are four places where "anim" is assumed to be non-null:

1) in config.macros.slider.onClickSlider

if(config.options.chkAnimate)
should be
if(anim && config.options.chkAnimate)

2) in Story.prototype.displayTiddler

if(config.options.chkAnimate && (animate == undefined || animate ==
true))
should be
if(anim && config.options.chkAnimate && (animate == undefined ||
animate == true))

3) in Story.prototype.closeTiddler

if(config.options.chkAnimate && animate)
should be
if(anim && config.options.chkAnimate && animate)

4) in Popup.show

if(config.options.chkAnimate)
should be
if(anim && config.options.chkAnimate)

Making these changes will mean that any plug-ins that set
config.options.chkAnimate will not be broken by the removal of
animation (assuming var anim = new Animator(); is replaced by var anim
= null; at startup).

Martin

Udo Borkowski

unread,
May 15, 2006, 4:33:26 PM5/15/06
to Tiddly...@googlegroups.com
As we talk about ticket #15: I think we should do some little refactoring to get even more out of this core/non-core split.


Have a look at this example:

Story.prototype.displayTiddler = function(srcElement,title,template,animate,slowly)
{
 ...
   if(srcElement)
        {
        if(config.options.chkAnimate && (animate == undefined || animate == true))
            anim.startAnimating(new Cascade(title,srcElement,theTiddler,slowly),new Scroller(theTiddler,slowly));
        else
            window.scrollTo(0,ensureVisible(theTiddler));
        }
}

I suggest we factor out all these "if (config.options.chkAnimate...)" statements into new functions and call it through the new "visEffect" variable.

Example: the given code is factored out to the function open in the class StandardVisEffect:
function StandardVisEffect() {
    this.open = function(text,startElement,targetElement,slowly, animate) {
        if(config.options.chkAnimate && (animate === undefined || animate == true))
            anim.startAnimating(new Cascade(text,srcElement,
targetElement,slowly),new Scroller(targetElement,slowly));
        else
            window.scrollTo(0,ensureVisible(
targetElement
));
        }
    };
    ...
}
...
var visEffect = new StandardVisEffect();
...
Story.prototype.displayTiddler = function(srcElement,title,template,animate,slowly)
{
 ...
   if(srcElement)
        visEffect.open(title, srcElement, theTiddler, slowly, animate);
}

We will end up with four functions that are called through visEffect ("open", "close", "scrollVisible", "slide").

For the the "non animated" version of the TiddlyWiki we create a class that also implements these functions, but the function will just contain the "non animated" code. E.g.

function NoVisEffect() {
    this.open = function(text,startElement,targetElement,slowly, animate) {
        window.scrollTo(0,ensureVisible(targetElement));
    };
    ...
}
(BTW: the "anim" variable will not exist in that configuration)

This way we simplify the core code, get rid of four "if" statements in the core and make it easier exchange the visual effects/animation.


Also I suggest to include the animation specific config fields into the "non-core" part:

merge(config,{
    animFast: 0.12, // Speed for animations (lower == slower)
    animSlow: 0.01, // Speed for EasterEgg animations
    cascadeFast: 20, // Speed for cascade animations (higher == slower)
    cascadeSlow: 60, // Speed for EasterEgg cascade animations
    cascadeDepth: 5 // Depth of cascade animation
    });

merge(config.options ,{
    chkAnimate: true,
    });


One open issue is the "OptionsPanel" value: it contains the text "<<option chkAnimate>> EnableAnimations\n" that should only included when animation is supported. For the AdvancedOptions the common approach is to append the new options to the text (e.g. when adding options through plugins). But since the text "\nSee AdvancedOptions" should probably be the last line this is not applicable and a more sophistacted solution is required.


Udo

Jeremy Ruston

unread,
May 16, 2006, 10:16:15 AM5/16/06
to Tiddly...@googlegroups.com
Excellent, I'll create a ticket for the plugin stripping, and
reference this thread.

Cheers

Jeremy.


--
Jeremy Ruston
mailto:jer...@osmosoft.com
http://www.tiddlywiki.com

Martin Budden

unread,
May 17, 2006, 6:10:44 PM5/17/06
to TiddlyWikiDev
My suggestion was for a precursor to the full refactoring - which
you've made redundant by providing the full refactoring. Good stuff.

Martin

Reply all
Reply to author
Forward
0 new messages