I need a Venn diagram to figure out what is shared logic. Here's an approximation. I can try to factor out each of the common operations into a common method.
The only thing that worries me is that the order in which the "setrecheight" calculations gets done is slightly different in the $lzc$set_height and in updateSize. In $lzc$set_height , the "this.height" is set first, and then the resource stretching code gets run, whereas it's the opposite in updateHeight. Oh well... ================================================================ LzView $lzc$set_height: ================================================================ [A] decides whether to set hassetheight [B] set this.height [C] check if cached height has changed, updates cached height [D] if null height value passed in, does some calculation to set view size based on resource height and the 'stretches' flag. [E] if pixellock, compute integer value [F] if "_setrescheight" does some other calculation on resource. (What does "_setrescheight" actually stand for? It seems to be indicate that "stretches" is set, why aren't we just looking at the value of 'stretches'?) [G] call sprite.setHeight [H] call parent.checkHeight [I] send 'onheight' event ================================================================ LzView.updateHeight: ================================================================ [D or F?] Does some calculation on stretched resources, which ought to be equivalent to what the $lzc$set_height does, but I'm not sure IF view is autosizing (hassetheight==false) then: [B] set this.height [G] call sprite.setHeight [I] send 'onheight' event [H] call parent.checkHeight ================================================================ LzText.installHeight: ================================================================ [B] set this.height [C] check if cached height has changed, updates cached height [E] if pixellock, compute integer value [G] call sprite.setHeight [H] call parent.checkHeight [I] send 'onheight' event On Thu, Mar 18, 2010 at 2:36 PM, P T Withington <[email protected]>wrote: > Is this really the best we can do? > > Now we have 3 different implementations of the height-setting code in > `<view>/$lzc$set_height`, `<view>/updateHeight`, and your new > `<text>/installHeight`. That just seems like we are asking for code skew > (and trouble). > > It's pretty clear that `<view>/$lzc$set_height` is being abused in the > <text> class (from the places you fixed that were calling the setter and > then immediately undoing the `hassetheight` flag); so it's great that you > are trying to fix that. > > But I don't think we should solve it by adding another copy of the > height-setting code. Can't we come up with a solution where there is a > single internal method that handles the setting of the height that gets > called from the external setter, and from _updateSize? Why does the <text> > class have to have a special method instead of just using > `<view>/updateHeight`? > > On 2010-03-18, at 13:06, Henry Minsky wrote: > > > Change 20100318-hqm-F by [email protected] on 2010-03-18 11:12:32 EDT > > in /Users/hqm/openlaszlo/trunk > > for http://svn.openlaszlo.org/openlaszlo/trunk > > > > Summary: fix for automatic height sizing of multiline text fields > > > > New Features: > > > > Bugs Fixed: LPP-8591 > > > > Technical Reviewer: ptw > > QA Reviewer: max > > Doc Reviewer: (pending) > > > > Documentation: > > > > Release Notes: > > > > Overview: > > > > This fixes a bug where autosizing multiline text fields whose content > height changes are > > improperly forcing hassetheight to true. > > > > [This fix does not address the original issue in the bug report, where > there the scrollHeight updating > > needs be forced. I should probably have created a new JIRA bug for this > fix ] > > > > > > > > When a multiline text field which is resizable (hassetheight == false) > > gets a content-changed event, this updates the height without > > resetting the hassetheight flag. > > > > > > > > > > Details: > > > > LzText.lzs: created a new method "installHeight" which sets the sprite > height, but > > does not side effect hassetheight flag > > > > LzText, LzInputText: call installHeight instead of the $lzc$set_height > setter when > > setting view height to it's text content height. > > > > Note: there is still some bug remaining with measuring the size of a > DHTML textfield, but > > that existed before this fix. > > > > Tests: > > > > smokecheck > > lzpix demo > > test/lfc/maxscroll.lzx > > > > test case below, the bounding box should expand vertically to just > enclose the text as you > > type. > > > > <canvas> > > <inputtext width="100" > > bgcolor="#cccccc" > > multiline="true">Hey there! This text should wrap onto > a few lines. > > </inputtext> > > </canvas> > > > > Files: > > M WEB-INF/lps/lfc/views/LzInputText.lzs > > M WEB-INF/lps/lfc/views/LzText.lzs > > > > > > Changeset: > http://svn.openlaszlo.org/openlaszlo/patches/20100318-hqm-F.tar > > > > -- Henry Minsky Software Architect [email protected]
