EZP-27577: Creates <ez-field-edit>

[StephaneDiot]

JIRA: https://jira.ez.no/browse/EZP-27577

Description

Creates ez-field-edit custom element. Currently this component only handles fields inputs validity.

A bit of CSS for the validation part in ezsystems/hybrid-platform-ui#55

Tests

Demo and unit tested

[dpobel]

since custom-style and share styles are not included, is and include attributes should be removed

[dpobel]

CS issue: stars should be aligned with the first star in the previous line (example https://github.com/ezsystems/hybrid-platform-ui-core-components/blob/master/js/ez-notification.js#L3) and there should be only one space between the star beginning the line and the text

[dpobel]

missing doc

[dpobel]

this should probably be delayed to connectedCallback (meaning you don't need to define a constructor function)

[dpobel]

there's a logical issue here. The selected label can be attached to another input so in that case most likely, the class ez-invalid will be added to ez-field-edit. I think you iterate over formElement.labels (which means the label must have a for attribute, those are missing in your test)

[dpobel]

that also means you should have a test with a fixture where an input have several labels

[dpobel]

I have some doubts if this is really working if node1 and node2 have a different depth. You should improve the test various cases.

[dpobel]

related to comment about search the common ancestor, those fixtures are too simple. Having the label and the input with the same parent node is clearly an over simplification

[dpobel]

id="input1" is already used above, make sure to have unique ids (and by the way, label should have a for attribute)

[dpobel]

put this string and Multiple inputs in a h1 or something like that, this would help using the demo

[dpobel]

you should put the invalid class in a const to avoid repeating it and also, I'm not super fan of that class, it's maybe a tad too generic. ez-invalid-input maybe ?

[dpobel]

you should avoid that because this relies on the order in the fixture. Here you should rather use:

elementWithMultipleInputs.querySelectorAll('#lowestCommonParent1 input').value = 'invalid value';

or given you use the #lowestCommonParent1 element below:

const ancestor = elementWithMultipleInputs.querySelector('#lowestCommonParent1');

ancestor.querySelector('input').value = 'invalid value';

[StephaneDiot]

updated @dpobel

[dpobel]

the class has changed, ez-invalid-input and it now adds the class on the first common label and input ancestor.

[dpobel]

This is the standard mechanism, no need to add that

[dpobel]

not on labels

[dpobel]

parent nodes

[dpobel]

that's a bad name, _getAncestors maybe ?

[dpobel]

not some most appropriate usage of a, put a span or a div

[dpobel]

same as above regarding a usage

[dpobel]

missing a space :)

[dpobel]

this does not what you want to do here :) querySelectorAll returns a NodeList on which you add a value property with the value 42 ie you are not setting the input value :)

[dpobel]

the test outline is not super clear, it seems like some cases are not tested or mixed.
I would write something like:

describe('`ez-invalid-input` class', function () {
    describe('unique input', function () {
        describe('with label', function () {
            it('should be set on lowest common ancestor');
            it('should be remove from lowest common ancestor');

            describe('label deeper than input', function () {
                it('should be set on lowest common ancestor');
                it('should be remove from lowest common ancestor');
            });

            describe('input deeper than label', function () {
                it('should be set on lowest common ancestor');
                it('should be remove from lowest common ancestor');
            });
        });

        describe('without label', function () {
             it('should be set on parent node');
             it('should be remove from parent node');
        });
    });

    describe('multiple inputs', function () {
        describe('with label', function () {
            it('should be set on lowest common ancestor');
            it('should be remove from lowest common ancestor');

            describe('label deeper than input', function () {
                it('should be set on lowest common ancestor');
                it('should be remove from lowest common ancestor');
            });

            describe('input deeper than label', function () {
                it('should be set on lowest common ancestor');
                it('should be remove from lowest common ancestor');
            });
        });

        describe('without label', function () {
             it('should be set on parent node');
             it('should be remove from parent node');
        });
    });
});

[dpobel]

use input instead of formElement, that way this function can be defined outside of the forEach loop

[dpobel]

this is quite complicated, given you loop over inputs, why not computing the validity:

let globalValidity = true;

this.querySelectorAll('input, select, textarea').forEach((formElement) => {
    // rest of the code
    globalValidity = (globalValidity && valid);
});
this.invalid = globalValidity;

[dpobel]

until