Appearance selector doesn't show appearance if datatype has a match

orbeon / orbeon-forms

Orbeon Forms is an open source web forms solution. It includes an XForms engine, the Form Builder web-based form editor, and the Form Runner runtime.

http://www.orbeon.com/

GNU Lesser General Public License v2.1

512 stars 220 forks source link

Appearance selector doesn't show appearance if datatype has a match #2298

Open ebruchez opened 9 years ago

ebruchez commented 9 years ago

Scenario:

update fr:tinymce
- add binding by datatype: fr|tinymce, xf|textarea:xxf-type('xf:HTMLFragment')
- template adds <bind type="xf:HTMLFragment"/>
insert Formatted Text
list of appearances only shows "Formatted Text"

I would like to see the "Character Counter" checkbox here.

ebruchez commented 9 years ago

Exact same case with the email field, which should also show the "Character Counter" appearance. Current rules:

xf|input:xxf-type('xf:email')
xf|input[appearance ~= character-counter]

The question is whether we can generally enable this. Does xf|input[appearance ~= character-counter] mean that this is supported for any datatype? I think so. If it is only supported for some datatypes, then the rule should be stricter, e.g. xf|input:xxf-type('xs:string')[appearance ~= character-counter].

ebruchez commented 9 years ago

Quickly check if it is an easy fix. If so, do for 4.10, otherwise postpone.

ebruchez commented 9 years ago

Some more:

[x] possibleAppearancesWithBindings needs to be changed. Currently, if there is any mapping found with a datatype, we only keep bindings with a datatype. This is incorrect and should be done per appearance.

For example, if we have xf|input:xxf-type('xf:email'), then xf|input[appearance ~= character-counter] is removed, but we should keep it.

OTOH if we have xf|input:xxf-type('xf:email') and xf|input, we do need to remove xf|input.
[ ] The direct binding for xf:character-counter never makes sense and should be removed. BUT we need to be careful because the CSS depends on the class xbl-fr-character-counter being present, and if we remove the direct binding the class is not placed.
[ ] The bindings for xf:character-counter need to be more specific. It makes sense to have the character counter for xf:string, xf:email, but not xs:date. So:
xf|input:xxf-type('xs:string')[appearance ~= character-counter], xf|input:xxf-type('xf:email')[appearance ~= character-counter], ...
[ ] newElementName must be fixed. With the above, it generates directly a fr:character-counter, but that must never be generated.

ebruchez commented 9 years ago

It's a bit more complicated than I thought. The idea, when handling datatypes at design time, is to introduce a substitution. For example, we substitute `fr:number for xf|input:xxf-type('xs:decimal').

In the case of character-counter, say we match on:

xf|input:xxf-type('xs:string')[appearance ~= character-counter]

The current code substitutes the direct binding fr|character-counter, which is clearly wrong and even causes a stack overflow at runtime. Clearly, another substitution is needed. But the runtime currently ignores all selectors containing datatypes. So we cannot simply NOT substitute because:

<xf:input appearance="character-counter">

will not match anything. And we cannot simply add the selector:

xf|input[appearance ~= character-counter]

because then the character-counter appearance would show in Form Builder for types like xs:date, etc.

ebruchez commented 9 years ago

Question: should there be a difference between cases like xf|input:xxf-type('xf:email') and xf|textarea[appearance ~= 'xxf:autosize'] where support is implemented natively, vs. cases where support is implemented with XBL components?

Issue: autosize textarea should only apply to xs:string type, so that it doesn't apply to the rich text area.

ebruchez commented 9 years ago

Current status: on local branch 2015-07-16-2298-2300-htmlfragment-etc there is some work, including the change to possibleAppearancesWithBindings. But we cannot push that change because otherwise the character-counter appearance shows for too many controls, like date controls, etc.

And we cannot push the change to the selectors for fr|character-counter, because we don't yet have a solution for the substitution.

ebruchez commented 9 years ago

Following discussion with @avernet today, it appears okay to have the XBL engine just erase the datatypes. If so:

the Form Builder substitution would be <xf:input appearance="character-counter">
the runtime would erase the datatype and honor the xf|input:xxf-type('xs:string')[appearance ~= character-counter] selector in this case

There can be problems, for example if we have, say, two bindings with selectors such as:

fr|foo → xf|input:xxf-type('xs:string')[appearance ~= gaga]
fr|bar → xf|input:xxf-type('xs:date')[appearance ~= gaga]

The erased selectors would be the same, so the runtime would just pick one.

We can issue a warning in such cases, and avoid writing selectors that conflict in this way until dynamic bindings are supported.

ebruchez commented 9 years ago

[x] modify runtime to keep selectors with datatype and appearance, and ignore datatype when matching
[ ] fix issue: character counter appearance on rich text produces an <xf:textarea>
[ ] more doc on substitutions

ebruchez commented 8 years ago

Based on the last comment, it seems that this is close from being done, depending on how hard fixing "character counter appearance on rich text produces an <xf:textarea>" is.

ebruchez commented 5 years ago

I think I am hitting an issue where adding xf|input:xxf-type('xf:email')[appearance ~= character-counter] to fr|character-counter shows the option in Form Builder, but setting it causes an error (which doesn't show in the logs for some reason).