Hi Justin et al.
Thanks for your compliments and your prompt response to my request for
review. I addressed your first two email points in the two jira's listed
in the subject line and have checked in those changes.
I think you have a good point about the test data in builder.js but we
should discuss it further - Michelle and I also chatted about this data
structure, so it is worth bringing this question to her attention as well.
As I'm sure you recall, the aim of the customBuild component is to to
populate the custom build page from the live data from the
build.properties file. For the demo that we've created, we provide two
ways to populate the page. The first technique provides the data via a
hard coded data structure
fluid.customBuild.demo.completeFluidInfusionData (which I guess is only
complete today and will likely change considerably over time). The
second technique provides the data live via the build properties file.
You can see code to use the two different techniques in CustomBuild.html
and will recall that we have been commenting out the specific
builderInit lines depending on which technique we want to use - the
stand alone data structure works well for testing without dealing with
any of the php code.
<snip>
//to use test data use the following
builderInit("#customBuild",
fluid.customBuild.demo.completeFluidInfusionData);
//to use data from infusion json files use the following
// builderInit("#customBuild", fluid.customBuild.dependencies);
</snip>
As I was working with the tests, I noted that we reproduced the exact
same data in the tests. I refactored to use only one copy of the data,
but perhaps it was a case of "premature optimization".
Note though, since this is a piece of demo code, the
fluid.customBuild.demo.completeFluidInfusionData data does not need to
change - it could stay the same even as infusion expands, since the data
for the 'live customBuilder' will always come from the build.properties
file and not from this data structure. However, I'm thinking that people
will be tempted to change the data as the real infusion expands, in
which case they will have to change the tests to reflect those
changes...and some of the hard coding will be a pain to change.
I did try to address this somewhat by providing some constants for the
indexes of the specific modules in the data which you wrote tests
around. In customBuild-tests.js you can find:
var INLINE_EDIT_INDEX = 7;
var PAGER_INDEX = 8;
var PROGRESS_INDEX = 9;
var REORDERER_INDEX = 10;
var UIOPTIONS_INDEX = 12;
So, I think that outlines all of the details around this data
structure...what do you think...should it simply be duplicated in both
the tests and the demo to prevent people from breaking the tests
inadvertently? I'm not convinced, but honestly don't think it is
critical one way or the other.
Laurel
Justin wrote:
Hello,
I've looked over some of your commits, please see my comments below.
customBuild.js
============
* the code for checkElementArray and unCheckElementArray seem to
be very similar. You could probably abstract that out into a
general function that is called with different parameters
* This is one of those things I thought about a while ago, but
forgot about. The onModelChange event should probably be renamed
to afterModelChanged or something like that.
customBuild-tests.js
================
* What is the strategy going forward for
fluid.customBuild.demo.completeFluidInfusionData. It probably
shouldn't pull data in from builder.js but have some test data
within the tests directory, or will this be pulling in some live
data in the future.
* Depending on the above, you may want to look into not having the
module indexes hardcoded for your tests.
It's looking good. Hope the feedback helps.
- Justin
begin:vcard
fn:Laurel A. Williams
n:Williams;Laurel
org:Faculty of Information, University of Toronto;Adaptive Technology Resource Centre (ATRC)
email;internet:[email protected]
title:Accessibility Software Designer
x-mozilla-html:TRUE
version:2.1
end:vcard
_______________________________________________________
fluid-work mailing list - [email protected]
To unsubscribe, change settings or access archives,
see http://fluidproject.org/mailman/listinfo/fluid-work
