Review for UIOptions FLUID-4235

[Antranig Basman]

Hi there, I am casting an eye over this branch as requested -
Commit is at 
https://github.com/michelled/infusion/commit/643cf21ebd8a8a15282243e0d97b2d0dc3898b31#commitcomment-398030
UIOptions.js at
https://github.com/michelled/infusion/blob/2819dbd94349c1b080a26278b156a85057355d98/src/webapp/components/uiOptions/js/UIOptions.js
UIEnhancer.js at
https://github.com/michelled/infusion/blob/643cf21ebd8a8a15282243e0d97b2d0dc3898b31/src/webapp/components/uiOptions/js/UIEnhancer.js

For UIOptions.js:
Yes, I support cindyli's comment that dependency design has failed here - there should not be a reference to 
something called "UIEnhancer" from UIOptions. The enhancer should only appear at the level where there is a 
required shared dependence, for example, in fluid.uiOptions.preview. [see suggestion below]

One resolution would be to reorganise the lines 242-245 to be delivered by a demands block, that only acted 
when a {uiEnhancer} was in scope. It is still problematic that so many defaults are defined within 
UIEnhancer itself - for example, the settingsStore and the defaultSiteSettings etc. I would move these out 
into a 3rd, largely dependence-free component called something like UIOptionsStore - they would be injected 
into both UIEnhancer and UIOptions from some shared location. See UIEnhancer.js:264 comment below

I would actually refactor this area quite significantly - uiOptions.preview should not be a subcomponent of 
uiOptions but actually a container/sibling of it. *OR* the thing currently called uiOptions should actually 
be renamed uiOptionsDialog - and this is the thing which has the link JUST to settingsStore and nothing 
else. Then the name "uiOptions" can be used for the "orchestrating component" which contains a preview, a 
uiOptionsDialog, a settingsStore and a uiEnhancer as direct children. Almost all the members which are 
currently on uiOptions (basically all the "controls") would go onto uiOptionsDialog.

lines 90 can be replaced by the all-new "renderOnInit" flag :P

I believe that line 427, that.refreshView() is redundant? I think it is gone from cindyli's branch anyway...


For UIEnhancer.js:
The sequence at line 330 is somewhat similar to the way "fluid.uploader" forwards its options and is 
something we need to produce a proper framework pattern for.

line 264: This should just be specified as "fluid.uiOptions.store" and only resolved as "cookieStore" via a 
demands block. Right now, someone advising this implementation would have to issue a demands block against 
"fluid.uiEnhancer.cookieStore"