approved

On Wed, Jul 14, 2010 at 8:13 PM, Max Carlson <[email protected]> wrote:

> Change 20100709-maxcarlson-M by maxcarl...@friendly on 2010-07-09 10:25:23
> PDT
>    in /Users/maxcarlson/openlaszlo/trunk-clean
>    for http://svn.openlaszlo.org/openlaszlo/trunk
>
> Summary: UPDATED: Move selection managers and command to lzx includes
>
> Bugs Fixed: LPP-9180 - Move non-essential parts of the LFC to LZX includes
> (partial)
>
> Release Notes: command, selectionmanager and dataselectionmanager have been
> moved out of the LFC and into LZX auto includes, found at
> lps/components/utils/.  Any other components that use the autoinclude
> mechanism will need to be updated to manually include
> dataselectionmanager.lzx, selectionmanager.lzx and/or command.lzx
>
> Technical Reviewer: ptw
> QA Reviewer: hminsky
>
> Details: Updated to address Andre's comments:
>
> - Some additional release notes are required. For example where to find the
> selection managers, in which cases explicit includes are now necessary, etc.
>
> Done.
>
> - dataselectionmanager uses lfc-internal methods (e.g.
> datapath#setSelected, DataElement#sel), so special care needs to be taken to
> document that now non-lfc classes use these fields/methods
>
> I added notes to the relevant fields to LzDataNode and LzDatapath and
> marked them public.
>
> - Why is as3 excluded here?
> > +<script>
> > +if ($as3) {
> > +} else {
> > +    var LzSelectionManager = lz.selectionmanager;  // publish for
> compatibility
> > +}
> > +</script>
>
> I removed those.
>
> - "lastRangeStart" not "lastRangeStar"
> > + <!---
> > +        @access private
> > +      -->
> > + <attribute name="lastRangeStar"/>
>
> Fixed.
>
> - Why did you remove the "@type *" doc-comment?
> > + <!---
> > +        @access private
> > +      -->
> > + <attribute name="lastRangeStar"/>
>
> Fixed.
>
> - Why did you remove the return types for all methods? For example:
> > + <method name="__LZaddToSelection" args="d:*, o:LzView">
> instead of
> <method name="__LZaddToSelection" args="d:*, o:LzView" returns="void">
>
> Fixed.
>
> - I'm not sure that this ("o /*:LzView*/") works for lzx2js2doc.xsl (see
> "processargs" template), have you tested this, that means did you rebuild
> the reference guide?
> > + <method name="isSelected" args="o /*:LzView*/">
>
> It _should_ work, but if this breaks the reference guide I'll address it.
>
> - I'd prefer to keep the CDATA start at the end of the line:
> > + <method name="__LZaddToSelection" args="d:*, o:LzView">
> > + <![CDATA[
> > +        ]]>
> > + </method>
>
>
> like:
> <method name="..." args="..."><![CDATA[
>  ...
> ]]></method>
>
> Unfortunately we don't seem to have a consistent code style for CDATA
> blocks, both types (CDATA directly following the <method> element,  and
> CDATA on the next line) are used in the components directory at the same
> frequency.
>
> Done.
>
> - This is a clear public API change, also see the related JIRA bugs:
> LPP-4982, LPP-7814
> > -var keys = null;
> > [...]
> > +<attribute name="key" value="null"/>;
>
> - Both methods are now broken, but see above about the key/keys issue:
> > +<method name="destroy">
> >      var oldKeys = this.keys;
>
> > +<method name="keysToString">
> > + <![CDATA[
> >    var s= "";
> >    var keys = this.keys;
>
> I fixed this to work the way it did before, which is confusing.
>
> - Why does the dhtml kernel uses a plain object to store font information,
> whereas the swf runtimes use the LzFont class?
>
> LzFont has a bunch of extra stuff that used to be initialized by the server
> for compiled fonts.  That wasn't relevant for DHTML.
>
> - If you want to restructure / re-modularize the lfc,  you should also
> consider to rename or move other files. For example the "controllers" and
> "helpers" directories should be renamed, or the files moved to other
> directories. "controllers" will now only contain the animator classes
> lz.animator and lz.animatorgroup, so maybe "animators" is a better name. The
> same goes for "helpers", the directory will now only contain the lz.state,
> maybe the file could be moved into a different directory. For bonus points,
> you could also move LzUrl.lzs and LzContextMenu.lzs out of the "services"
> directory, because both files don't actually contain any service classes at
> all. Do you have any bigger plans how to change the lfc at this point or did
> you just thought: "Well, the selection-managers and layout classes don't
> seem to be necessary for the lfc, so let's move them out?" I'd be interested
> to hear your ideas!
>
> More the latter.  I want to trim down the LFC as much as possible.  I'm
> planning to move a bunch of the compatibility adapters (deprecated methods)
> in LzView to a separate mixin as well - so folks can get the compatibility
> shims if they want it on a class-by-class basis.
>
> I'll address the changes to services, and rename controllers and helpers,
> etc. in a subsequent change.
>
> Let me know your thoughts about how we could best slim down the LFC and
> make it work better in resource-constrained environments like mobile!
>
> Otherwise:
>
> lfc-undeclared - Remove explicit definitions for command, selectionmanager
> and dataselectionmanager
>
> kernel/LzFont,Library - Move to kernel, and only include for swf - DHTML
> doesn't use this
>
> LaszloView - Fix doc comment
>
> LzCommand,LzSelectionManager,LzDataSelectionManager - Move to
> lps/components/utils/, rewrite to use LZX syntax
>
> data/LzDataNode,LzDatapath - Explicitly make setSelected() and sel
> attributes public now that they're used in LZX.
>
> lzx-autoincludes - Add command, selectionmanager and dataselectionmanager
> entries
>
> debugger.lzx - Explicitly include command.lzx
>
> datalistselector - Explicitly include dataselectionmanager
>
> listselector - Explicitly include selectionmanager
>
> Tests: Component sampler and debugger run as before.
>
> Files:
> M       WEB-INF/lps/schema/lfc-undeclared.lzx
> A  +    WEB-INF/lps/lfc/kernel/LzFont.lzs
> M       WEB-INF/lps/lfc/kernel/Library.lzs
> M       WEB-INF/lps/lfc/views/LaszloView.lzs
> D       WEB-INF/lps/lfc/helpers/LzFont.lzs
> D       WEB-INF/lps/lfc/helpers/LzDataSelectionManager.lzs
> M       WEB-INF/lps/lfc/helpers/Library.lzs
> D       WEB-INF/lps/lfc/helpers/LzCommand.lzs
> D       WEB-INF/lps/lfc/helpers/LzSelectionManager.lzs
> M       WEB-INF/lps/lfc/data/LzDataNode.lzs
> M       WEB-INF/lps/lfc/data/LzDatapath.lzs
> M       WEB-INF/lps/misc/lzx-autoincludes.properties
> M       lps/components/debugger/debugger.lzx
> A  +    lps/components/utils/dataselectionmanager.lzx
> A  +    lps/components/utils/command.lzx
> A  +    lps/components/utils/selectionmanager.lzx
> M       lps/components/base/datalistselector.lzx
> M       lps/components/base/listselector.lzx
>
> Changeset:
> http://svn.openlaszlo.org/openlaszlo/patches/20100709-maxcarlson-M.tar
>



-- 
Henry Minsky
Software Architect
[email protected]

Reply via email to