History | Log In     View a printable version of the current page.  
Issue Details (XML | Word | Printable)

Key: LPP-4007
Type: Task Task
Status: Closed Closed
Resolution: Won't Fix
Priority: -- --
Assignee: Unassigned
Reporter: Benjamin Shine
Votes: 0
Watchers: 3
Operations

If you were logged in you would be able to see more operations.
OpenLaszlo

Rewrite drawview to override construct

Created: 21/May/07 10:14 AM   Updated: 22/Jun/07 12:57 PM
Component/s: Extensions - drawview
Affects Version/s: 4.0.2
Fix Version/s: None

Time Tracking:
Not Specified

Severity: Minor
Runtime: N/A
Fix in hand: False


 Description  « Hide
from email by ptw: drawview is doing its construction in an onconstruct handler, so it is not really fully constructed until _after_ onconstruct. Is this a common pattern? Probably people are leery of overriding the construct method (which should achieve the same effect), but I don't think that is right. `onconstruct` is supposed to signal that the instance is fully constructed, which won't be true if your class is going to do more construction in onconstruct.

I really don't like sprinkling `if (this.isinited)` around all your event handlers. It should not be necessary. It's overhead on every event that you only need during construction.

Should we have a guideline that says you should not use onconstruct to do construction? That you have to do that by overriding the construct method? (It's really not that scary.)

Does someone want to make the experiment of rewriting drawview to override construct? (You have to invoke the super method first, before you do any of your own construction:

  super.construct.apply(this, arguments);

then you should be able to remove all the `if (this.isinited)` in handlers.


 All   Comments   Work Log   Change History      Sort Order: Ascending order - Click to sort in descending order
Benjamin Shine - 21/May/07 03:32 PM
Fixing this should make this code snippet work:
solating further, the problem comes down to calling drawview.clear() before the drawview is completely built. This code snippet demonstrates the problem; running in dhtml gives a debugger error,
ERROR: http://localhost:8080/legals/test/drawing/drawing.lzx?lzt=object&lzt=object&debug=true&lzr=dhtml&_canvas_debug=true:188: this.context has no properties

    <drawview width="200" height="200">
        <handler name="onwidth">
            <![CDATA[
            this.drawStructure();
            ]]>
        </handler>
        <method name="drawStructure">
            this.clear();
        </method>
    </drawview>

P T Withington - 23/May/07 01:37 PM
[16:34] maxcarlson: LPP-4007 Rewrite drawview to override construct
[16:34] maxcarlson: http://jira.openlaszlo.org/jira/browse/LPP-4007
[16:34] p7w: You wanna try it, or me?
[16:35] maxcarlson: I think this is taken care of.
[16:35] p7w: You are the drawview wizard.
[16:35] maxcarlson: I'll take it and verify...[16:35] p7w: It is taken care of with a lot of runtime 'if's that should not be needed
[16:35] p7w: which is the point of 4007.
[16:36] maxcarlson: ptw, I'll ping you offline wrt 4007
[16:36] p7w: i.e., drawview might be sped up if every method did not have a 'if ready' around it.
[16:36] p7w: k

Jim Grandy - 01/Jun/07 09:29 PM
You can't actually move the code into the construct method. The 'this.__LZcanvas.setAttribute' calls, if made in construct(), make the height of the canvas incorrect.

André Bargull - 10/Jun/07 06:36 AM
The "onconstruct"-code can be moved into the "construct"-method, but only after LPP-3905 has been fixed!

Max Carlson - 15/Jun/07 05:21 PM
Alas, I tried doing this many different ways (even after fixing the width/height) and there's not enough of the DOM initialized at construct time. See LPP- 4150 for more. Back to onconstruct! It's not so bad... And Tucker, don't worry, it's not a common pattern either.