fusion-form icon indicating copy to clipboard operation
fusion-form copied to clipboard

BUGFIX: Ad id attribute to form fields

Open jonnitto opened this issue 1 year ago • 7 comments

Closes #85

jonnitto avatar Aug 29 '23 19:08 jonnitto

Spec says that about identifiers: see https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/id

Technically, the value for an id attribute may contain any character, except whitespace characters. However, to avoid inadvertent errors, only ASCII letters, digits, '_', and '-' should be used, and the value for an id attribute should start with a letter.

mficzel avatar Oct 06 '23 14:10 mficzel

Not sure if I'm correct, but wouldn't this change lead to the id attribute value potentially starting with a number? Shouldn't it be 'field_' + field.getName() as it is/will be in PagerTiger?

lorenzulrich avatar Nov 16 '23 15:11 lorenzulrich

Having an id attribute is the only way to associate an input with a label. Therefore, if a form field is used with a label, there is no way around an id attribute. I checked on other solutions, such as TYPO3 Powermail, TYPO3 Form Framework, WordPress Gravity Forms, all of them have id attributes by default. Personally, I think it's a no-brainer to just have an id attribute.

But if we don't want this in Neos.Fusion.Form, we should at least be able to activate the id attribute:

showIdAttribute = false

        renderer = afx`
            <input
                id={props.showIdAttribute ? field.getName() : ''}

(Or similar)

lorenzulrich avatar Nov 17 '23 14:11 lorenzulrich

We should have a logic that ensures uniqueness for id's or use the optIn as suggested. Without that we can easily cause issues for documents that render multiple forms and wich very likely have some fields in common.

Maybe we allow to set the id on the fieldContainer and only after that is done render it for label and field.

mficzel avatar Nov 17 '23 16:11 mficzel

Yes, the uniqueness should be established using a "namespace", in Neos.Fusion.Form, this is done using the namespace property:

prototype(My.Site:FooForm) < prototype(Neos.Fusion.Form:Runtime.RuntimeForm) {
    namespace = 'foo_form'

field.getName() always returns the namespaced field name, therefore I think it would be (almost) safe to use as identifier.

lorenzulrich avatar Nov 17 '23 20:11 lorenzulrich

@mficzel You merged https://github.com/sitegeist/Sitegeist.PaperTiger/pull/18 in the meantime, but this one is still unmerged. How should be progress here?

lorenzulrich avatar Dec 07 '23 22:12 lorenzulrich

"Self correction": https://github.com/sitegeist/Sitegeist.PaperTiger/pull/18 doesn't depend on this issue, it works well as long as there is only one form (or unique form field names) on a page.

lorenzulrich avatar Jan 08 '24 14:01 lorenzulrich