Motivation
This started as a bug item
Item11833: FORMFIELD does not render a field using renderForDisplay whereas SEARCH does. And this came up because I noticed that the new
datetime
fields are getting rendered differently.
Each form field has a method
renderForDisplay
(and in trunk also exists
renderValueForDisplay
). So a
datetime
field can be rendered in local time (hiding the time zone offset part). This is what you would want most of the time, for example, in a
SEARCH
.
The problem is:
-
renderForDisplay
is not called everywhere
- if it where called everywhere, there would be an occasional need to get the raw field value
Where
renderForDisplay
is called:
the topic data form (in view template) |
yes |
using $formfield in SEARCH |
yes |
using FORMFIELD |
no |
using QUERY |
no |
It is more visible for multi-value fields. Let's say I have this field definition:
| Select | select+values | 1 | ,Ein=one,Zwei=two,Drei=three | | |
The Select field gets rendered as:
the topic data form (in view template) |
Drei |
using $formfield in SEARCH |
Drei |
using FORMFIELD |
three |
using QUERY |
three |
I think this is a bug that justifies a change in rendering (for the better).
As mentioned in the bug item: if performance was the big issue here I would expect that
SEARCH
would take the (undesired) faster approach of not rendering the form field.
Description and Documentation
Although this is a bug, we cannot predict that current applications won't break if we change the current value handling. The foolproof-to-clumsy-inherited-code approach is to add new attributes to get the rendered value:
- For
FORMFIELD
:
- Keep the
FORMFIELD
attribute $value
to show the raw field value
- Add a new
FORMFIELD
attribute $viewvalue
to show the rendered field value; let it call the field's renderForDisplay
method
- For
QUERY
:
- Keep the
QUERY
attribute $value
to show the raw field value
- Add a new
QUERY
attribute $viewvalue
to show the rendered field value; let it call the field's renderForDisplay
method
- For
SEARCH
:
- Document
$formfield(name)
that this is the rendered field value
- Add a new
$formfieldvalue(name)
attribute to retrieve the raw field value; retrieve the raw value
--
Contributors: ArthurClemens - 09 May 2012, 14 May 2012
Discussion
The issue is there, definitely.
But I have quite some concerns to change
FROMFIELD's
default. It might be argued that
QUERY
is more of a low level query operator, so it would be reasonable to return the
stored value rather than the way it is displayed for users.
Fact remains that neither
FORMFIELD
nor
QUERY
can render a formfield value as intended to be displayed for the user according its definitions in a DataForm or a custom
FormField
implemented by some plugin.
In
FlexFormPlugin this issue has been addressed by always using
renderForDisplay
, and then provide an
$origvalue
to be used in format strings.
In
MetaDataPlugin the original value is accessible using
$orig<fieldName>
in format strings.
So for the core I'd propose to do something similar, instead of changing the default.
--
MichaelDaum - 10 May 2012
What do you mean by "something similar"? Always return the rendered value like your plugins, or add a new parameter to show the rendered value?
I think the issue is: is this a bug that should be fixed (so we display the rendered value), or are we adding a new feature (so we add a new attribute to show the rendered value)?
--
ArthurClemens - 10 May 2012
Alas for backwards compatibility reasons this bug needs to be seen as a feature. That's not to say we cannot add a feature to allow better control. Whether that's extra FORMFIELD or QUERY options, enhancing the DataForm definition in some way or all of the above.
Alas, this relates to deeper problems that need to be addressed. I have sets of DataForms as part of work flows. In one work flow state I capture a date. In another state that date is now a label (so it cannot be changed during form edit). As a label it will not be seen as a date and will never call the date version of
renderForDisplay
to be rendered accordingly. The simplest idea here would be to allow label in a DataForm not as a type in it's own right but as a modifier of others e.g. date+label, so you know it's a date even though the UI will not let you edit it. (If there's a smarter way of doing this already please let me know.)
I believe that
AllowTypedData needs to introduce strict types (as distinct from existing loose types) which properly consider rendering. Indeed this should carefully consider how to make DataForm fields more MVC.
However, to the proposal at hand. I believe the only way would be adding extra parameters to allow appropriate control in a consistent way as possible, which will necessarily be hampered by backwards compatibility issues.
Actually, I now realise that for the specific case of creating a new DataForm field type. You could presumably register this type as insisting of renderForDisplay as the default and changing code in QUERY/FORMFIELD to use this option, but only for types declared as such.
There's another complication relating to my date+label thoughts, what about number+select? That is to say the the type is at heart a number and should be rendered as such, but it's also a select type for editing where a discrete range of numbers are offered.
--
JulianLevens - 10 May 2012
Please give me some insights in the problem of backward compatibility: do you have apps that use the raw value from
FORMFIELD
or
QUERY
and that would break if we start to show the rendered value?
--
ArthurClemens - 10 May 2012
Arthur, yep, that's exactly the case. Lots of apps have been done relying on those two to behave as they do now: return the raw value as stored. For
QUERY
I am not sure whether this is a bug at all.
--
MichaelDaum - 10 May 2012
I can only say possibly. I do not have the time to investigate. However, can you be sure that no FW site will be affected by such a change? That's why to me, we must provide option within the various facilities we offer to control this macro by macro, $formfield by $formfield, etc by etc. In addition, a config option to allow the default to stay the same as now or always render, that way the admin can control when/if to change the default behaviour.
--
JulianLevens - 10 May 2012
Sounds like config hell.
--
MichaelDaum - 10 May 2012
Well if it looks like config hell and sounds like config hell and walks like config hell then ...
I agree and I retract the suggestion to add a config option here. Better consistent future behaviour should only be achieved by shiny new datatypes or options added where needed under user/dev control.
--
JulianLevens - 10 May 2012
The alternative foolproof-to-clumsy-inherited-code approach is to add new attributes to get the rendered value:
- For
FORMFIELD
:
- Keep the
FORMFIELD
attribute $value
to show the raw field value
- Add a new
FORMFIELD
attribute $viewvalue
to show the rendered field value
- For
QUERY
:
- Keep the
QUERY
attribute $value
to show the raw field value
- Add a new
QUERY
attribute $viewvalue
to show the rendered field value
- For
SEARCH
:
- Document
$formfield(name)
that this is the rendered field value
- Add a new
$formfieldvalue(name)
attribute to retrieve the raw field value
--
ArthurClemens - 10 May 2012
That looks good.
--
MichaelDaum - 11 May 2012
I've updated the proposal and reset the clock.
--
ArthurClemens - 14 May 2012
it doesn't look so good when you consider that it breaks compatibility with 1.1.4 and earlier, in which
$formfield
always renders the raw value. The fix checked in for
Tasks.Item12466 does things slightly differently, by adding a parameter to
$formfield
to retrieve the
display value. Extending this concept to the other two applications we get:
- For
FORMFIELD
:
- Keep the
FORMFIELD
attribute $value
to show the raw field value done
- Add a new
FORMFIELD
attribute $value(display)
to show the rendered field value
- For
SEARCH
:
- Document
$formfield(name)
that this is the raw field value done in Tasks.Item12466
- Add a new
$formfield(name,display)
attribute to retrieve the mapped field value done in Tasks.Item12466
- For
META{"formfield"
:
Where items are marked
done the behaviour is as for 1.1.4, 1.1.9 and 1.2.0. 1.1.5 through 1.1.8 are all broken.
Reset the clock.
--
CrawfordCurrie - 16 Apr 2013
Is this feature closed / merged to core?
Item12466 is marked closed, but the proposal is still under construction.
--
GeorgeClark - 02 Apr 2014
The items I marked in red above remain to be done. I removed
ArthurClemens from the
CommittedDeveloper setting, as he clearly isn't.
--
CrawfordCurrie - 02 Apr 2014
This proposal is in a bad state as it is accepted, has got a date of commitment but no committed developer anymore.
--
MichaelDaum - 02 Apr 2014
Actually it's in an excellent state, because I've been working on it. Though I am not committed.
--
CrawfordCurrie - 02 Apr 2014
Final analysis: I have consistently implemented '$value(display)' for FORMFIELD and META, though they lack the additional formatting capabilities of SEARCH (line braking and character mapping). There is no way to do a '$value(display)' style format for QUERY, because QUERY has no information about the form definition when it is serialising the result of the query. There really needs to be a new proposal that adds a
format
parameter to QUERY to do that. Until then, FORMFIELD has to stay.
--
CrawfordCurrie - 03 Apr 2014
This broke a couple of extensions implementing own formfield types.
--
MichaelDaum - 03 Apr 2014