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"
_______________________________________________________
fluid-work mailing list - [email protected]
To unsubscribe, change settings or access archives,
see http://fluidproject.org/mailman/listinfo/fluid-work
