[#LPP-2181] incubator component scrolledittext.lzx

OpenLaszlo

incubator component scrolledittext.lzx

Created: 13/Jun/06 12:46 PM Updated: 26/Oct/06 02:46 PM

Component/s:

Components - Incubator

Affects Version/s:

3.3.1

Fix Version/s:

3.4, 3.3.3

Time Tracking:

Not Specified

File Attachments:

scrolledittext.lzx (5 kb)

Severity:	Major
Fixed in Change#:	1,101
Runtime:	N/A
Fix in hand:	True

Description

« Hide

Test case - components/incubator/test/scrolledittext-test.lzx

Type characters until they wrap... Notice that the characters go behind the scrollbar. The textfield width does not respond to the scrollbar vis.

This component was updated with a dynamically instantiated scrollbar. This created a regression in the textfields behavior that would set the width upon the scrollbars visibility. This is because the constraint was delclared in the markup and the object does not exist. A delegate fixes the problem.

See the attached file for the suggested fix.

Severity is major because sallen cannot take it anymore!

All

Comments

Work Log

Change History

Sort Order:

Ascending order - Click to sort in descending order

[ Permlink | « Hide ]

Amy Muntz - 19/Jun/06 11:47 AM

Do the code review. Send out to laszlo-dev and contributor (listed in bug) and paste a copy into the JIRA bug comments section. Update test file, with case, if one is not provided.

[ Permlink | « Hide ]

Philip Romanik - 19/Jun/06 10:43 PM

Code Review of scrolledittext.lzx

1. A piece of debugging code crept into the code (tstDbg in this case)
        <_newinternalinputtext name="inp" id="tstDbg" x="${parent.border}"

2. The event function is mixing variables. Both v and this._vs refer to the same variable, and they should not be used in the same method. For example:

Original:

<method name="setvscrollwidth" args="v">
    if(v){
        this.setAttribute( "vscrollwidth", this._vs.width);
    }else{
        this.setAttribute( "vscrollwidth", 0);
    }
</method>

Consistent (and streamlined) version:

<method name="setvscrollwidth" args="v">
    this.setAttribute("vscrollwidth", (this._vs ? this._vs.width : 0));
</method

[ Permlink | « Hide ]

Amy Muntz - 20/Jun/06 09:50 AM

Michael - assigning to you. Please remove the debug code and use the simplified method that resulted from the email between you and Phil and attach new changeset. Then, we can get this checked in.

(Email:
Hi Michael,

Yes, you are right. Sorry about that. When I read through the code I
believed the conditional was making sure this._vs was not null.

Phil

v is for visible.

the latter method of eval still stands though...

so

<method name="setvscrollwidth" args="v">
this.setAttribute("vscrollwidth", (v ? this._vs.width : 0)); </method>

no?)

[ Permlink | « Hide ]

Philip Romanik - 20/Jun/06 07:45 PM

Two more small revisions on the file. Checked into trunk at revision 1101.

[ Permlink | « Hide ]

Philip Romanik - 07/Jul/06 11:27 AM

Code review and 2 revisions complete.