Hi Anastasia - I think the API change in itself is harmless here since
the function has already been documented as non-API forming. However,
I'm against the particular change suggested here since we have so far
been at least moderately careful to avoid "that-ist leak" in the
InlineEdit infrastructure, restricting our utility functions to only
expecting the minimum information in their argument lists, rather than
leaking a reference to the entire component "that" which then makes the
expectations of the function unclear.
What I would recommend here is to take the opportunity for a bit of
"opportunistic refactoring". The best times for this kind of thing are
exactly times like the present - where some additional requirement
increases costs within the component and makes its architectural
problems more impactful. In this case, there are some comments attached
to some strings in the options block of this form:
// this is here for backwards API compatibility, but should be
in the strings block
defaultViewText: "Click here to edit",
etc.
A good move here would be to write some code to actually copy these
options into the strings block after initialisation, and to rewrite the
InlineEdit implementation so that the versions in the strings block are
the ones used throughout. You can then supply the signature to
bindHighlightHandler as (element, displayModeRenderer, styles, strings)
which I think would be a reasonable and communicative signature.
Although "displayModeRenderer" is I think a quite misleading name for
this DOM element argument.
The string options accepted at top level can then be marked as properly
deprecated and scheduled for removal in some future version of the API
(perhaps 2.0)
Cheers,
Antranig
On 16/07/2012 22:40, Cheetham, Anastasia wrote:
I'm working on implementing a change to InlineEdit requested by the designers:
the ability to change the invitation text when focus lands on the field.
http://issues.fluidproject.org/browse/FLUID-4725
The implementation requires a small API change to one of the functions,
fluid.inlineEdit.bindHighlightHandler(), changing the last parameter:
- fluid.inlineEdit.bindHighlightHandler = function (element,
displayModeRenderer, styles) {
+ fluid.inlineEdit.bindHighlightHandler = function (element,
displayModeRenderer, that) {
This function binds keyboard focus and blur event handlers to an element. It is
not part of the 'public' API, and I suspect that it's not something that is
being overridden by implementations: the function basically adds and removes
classes, so customization would likely occur by adjusting the styles themselves
or overriding the classnames.
I'm wondering how the community feels about this API change. Does anyone want
to point out reasons not to make the change?
_______________________________________________________
fluid-work mailing list - [email protected]
To unsubscribe, change settings or access archives,
see http://lists.idrc.ocad.ca/mailman/listinfo/fluid-work
