You are here: Foswiki>Tasks Web>Item11833 (05 Jul 2015, GeorgeClark)Edit Attach

Item11833: FORMFIELD does not render a field using renderForDisplay whereas SEARCH does

pencil
Priority: Normal
Current State: Closed
Released In: 2.0.0
Target Release: major
Applies To: Engine
Component: DataForms, FORMFIELD
Branches: trunk master
Reported By: ArthurClemens
Waiting For:
Last Change By: GeorgeClark
Surprisingly,
%FORMFIELD{"MyDate" topic="MyTopic"}%
does not end up calling renderForDisplay from the field definition. It shows the plain stored field value. That would mean for instance that date field would be shown with the time zone string, instead of the time rendered in your local time zone.

But...
%SEARCH{
   "form.name='DateTimeForm'"
   type="query"
   format="$formfield(MyDate)"
}%
does use it, so the value is rendered according to its type.

Now, FORMFIELD calls Render::renderFORMFIELD, where the (too) simple value substitution takes place:
$text =~ s/\$value/$value/go;

SEARCH calls Meta::renderFormFieldForDisplay, which looks up the form, finds the field definition, and calls renderForDisplay.

Of course this takes more code, more time. But if performance was the big issue here I would expect that SEARCH would take the (undesired) faster approach. Now it looks like a bug in FORMFIELD.

-- ArthurClemens - 07 May 2012

Yes. And what about %QUERY?

Actually, we need both: renderForDisplay as well a way to access the original value. These two might diverge significantly, e.g. when using the +value feature.

-- MichaelDaum - 08 May 2012

Good points.

QUERY gives the same output as FORMFIELD (in Foswiki::Query::Node::_getField: return $data->{ $this->{params}[0] };)

Perhaps next to renderValueForDisplay there should be a rawValueForDisplay.

-- ArthurClemens - 08 May 2012

Proposal created: ConsistentFormFieldValues.

-- ArthurClemens - 09 May 2012

OK, done. I lifted the deprecation notice on META because it implements features that are required in templates, but will never be implemented in QUERY (for example, display values)

-- CrawfordCurrie - 03 Apr 2014

I am afraid the recent changes broke a couple of extensions.

This is related to removing renderForDisplay (a renderForEdit is still there).

We will have to find a way to allow extensions to be backwards compatible ....

-- MichaelDaum - 03 Apr 2014

I didn't change any public APIs, so nothing should be broken. The way for extensions to be backward compatible is for them to only call public APIs (and if a public API does not exist, then one has to be added, otherwise extensions will break).

-- CrawfordCurrie - 03 Apr 2014

Apologies; I did change a public API, without realising it. Re-opened - the spec may need to change frown, sad smile

No, I didn't, and it doesn't.

-- CrawfordCurrie - 03 Apr 2014

Affected plugins:

... and a few ones not yet released (SocialFormfieldsPlugin, PasswordPlugin).

They all need a way to function on 1.1.x as well as 1.2.x

-- MichaelDaum - 03 Apr 2014

I don't understand; either a field type specifies a getDisplayValue which returns a mapped value (or it derives from a type which does), or it doesn't in which case $value and $value(display) are synonymous.

As we discussed on IRC, I reverted the interpretation of $value as the mapped value which had crept into trunk, making it incompatible with 1.1.x. This only impacts +values types. Since trunk has never been released, the behaviour is now consistent across all releases (including the forthcoming 1.2). Unless you can explain otherwise?

BTW your remark above about removal of renderForDisplay. renderForDisplay has not been removed, it can still be overridden in a field type. However the concept of a mapped value has been elevated to the FieldDefinition superclass, which now calls getDisplayValue in the subclass to expand $value(display).

Without knowing what additional problems you may have, I can't comment further than that.

-- CrawfordCurrie - 04 Apr 2014

renderForDisplay is not only used to render a raw value as it is mapped in +values. The other custom formfields require a different way to render a raw value. For example a rating formfield renders as a star rating widget; user and topic formfields display as a link; a color formfield displays the color code in as a background color to the code etc.

This now has to be moved to getDisplayValue so that $value(display) picks it up.

However, older foswiki engines don't call getDisplayValue. They use renderForDisplay solely. For compatibility reasons the code for custom formfields now follows this pattern to make it consistent across releases:

sub renderForDisplay {
   my ($this, $format, $value, $attrs) = @_;

   my $displayValue = $this->getDisplayValue($value);

   $format =~ s/\$value\(display\)/$displayValue/g;
   $format =~ s/\$value/$value/g;

   return $this->SUPER::renderForDisplay($format, $value, $attrs);
}

sub getDisplayValue {
    my ($this, $value) = @_;

    return display code for $value;

    # example for color:
    # return "<span class='jqFarbtasticFG' style='background-color:$value;width:$this->{size}em'>$value</span>";
}

FlexFormPlugin as well as MetaDataPlugin have a different way to deal with raw vs display value (this has been discussed on the feature proposal already I think). They use $value and $origvalue where the latter expands to the raw value and the former to the value rendered for display purposes. So this is the opposite of what has been decided in the feature proposal. For those plugins, it has been so since the beginning, so I can't change this lightheartedly for backwards compatibility reasons. These plugins now work around the cores of different releases making sure that the right variable expands to the intended value as before.

-- MichaelDaum - 04 Apr 2014

While working on other display formats ($value(json), $value(list)) I've come to the conclusion that getDisplayValue() should really only be used to get the value-mapped result and don't render any markup whatsoever.

-- MichaelDaum - 17 Sep 2014

This has a large number of checkins - is it in a releasable state for 1.2?

-- GeorgeClark - 23 Dec 2014

Yes

-- MichaelDaum - 23 Dec 2014
 
Topic revision: r39 - 05 Jul 2015, GeorgeClark
The copyright of the content on this website is held by the contributing authors, except where stated elsewhere. See Copyright Statement. Creative Commons License    Legal Imprint    Privacy Policy