Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(form-core): Implement conversion of name attribute value to string type #2279

Merged
5 changes: 5 additions & 0 deletions .changeset/olive-meals-explode.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@lion/ui': patch
---

[form-core] Updated behavior of name attribute in FormRegisteringMixin to convert values to string type
9 changes: 0 additions & 9 deletions packages/ui/components/form-core/src/FormControlMixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ const FormControlMixinImplementation = superclass =>
/** @type {any} */
static get properties() {
return {
name: { type: String, reflect: true },
readOnly: { type: Boolean, attribute: 'readonly', reflect: true },
label: String, // FIXME: { attribute: false } breaks a bunch of tests, but shouldn't...
labelSrOnly: { type: Boolean, attribute: 'label-sr-only', reflect: true },
Expand Down Expand Up @@ -156,14 +155,6 @@ const FormControlMixinImplementation = superclass =>
constructor() {
super();

/**
* The name the element will be registered with to the .formElements collection
* of the parent. Also, it serves as the key of key/value pairs in
* modelValue/serializedValue objects
* @type {string}
*/
this.name = '';

/**
* A Boolean attribute which, if present, indicates that the user should not be able to edit
* the value of the input. The difference between disabled and readonly is that read-only
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,14 @@ const FormRegisteringMixinImplementation = superclass =>
class extends superclass {
constructor() {
super();
/**
* The name the element will be registered with to the .formElements collection
* of the parent. Also, it serves as the key of key/value pairs in
* modelValue/serializedValue objects
* @type {string}
*/
this.name = '';

/**
* The registrar this FormControl registers to, Usually a descendant of FormGroup or
* ChoiceGroup
Expand All @@ -37,8 +45,28 @@ const FormRegisteringMixinImplementation = superclass =>
this.allowCrossRootRegistration = false;
}

/**
* Name attribute for the control.
* @type {string}
*/
get name() {
return this.__name || '';
}

/**
* Converts values provided for the `name` attribute to string type.
* Mimics the native `input` behavior.
* @param {string} newName
*/
set name(newName) {
const oldName = this.name;
this.__name = newName.toString();
this.requestUpdate('name', oldName);
}

static get properties() {
return {
name: { type: String, reflect: true },
tlouisse marked this conversation as resolved.
Show resolved Hide resolved
allowCrossRootRegistration: { type: Boolean, attribute: 'allow-cross-root-registration' },
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,23 @@ export const runRegistrationSuite = customConfig => {
expect(eventSpy).to.have.been.calledOnce;
expect(eventSpy.getCall(0).args[0].composed).to.equal(false);
});
it('accepts a name attribute and converts the values provided to a string', async () => {
const elAttr = /** @type {RegisteringClass} */ (
await fixture(html`<${childTag} name=${5}></${childTag}>`)
);

expect(elAttr.hasAttribute('name')).to.be.true;
expect(elAttr.name).to.be.a('string');
expect(elAttr.name).to.equal('5', 'as an attribute');

const elProp = /** @type {RegisteringClass} */ (
await fixture(html`<${childTag} .name=${5}></${childTag}>`)
);

expect(elProp.hasAttribute('name')).to.be.true;
expect(elProp.name).to.be.a('string');
expect(elProp.name).to.equal('5', 'as a property');
});
});
describe('FormRegistrarPortalMixin', () => {
it('forwards registrations to the .registrationTarget', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,6 @@ export declare interface HTMLElementWithValue extends HTMLElement {
export declare class FormControlHost {
static get styles(): CSSResultArray;
static get properties(): {
name: {
type: StringConstructor;
reflect: boolean;
};
readOnly: {
type: BooleanConstructor;
attribute: string;
Expand Down Expand Up @@ -75,7 +71,7 @@ export declare class FormControlHost {

/**
* The name the element will be registered with to the .formElements collection
* of the parent. Also, it serves as the key of key/value pairs in
* of the parent. Also, it serves as the keny of key/value pairs in
tlouisse marked this conversation as resolved.
Show resolved Hide resolved
* modelValue/serializedValue objects
*/
name: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,13 @@ import { LitElement } from 'lit';
import { FormRegistrarHost } from './FormRegistrarMixinTypes.js';

export declare class FormRegisteringHost {
/**
* The name the host is registered with to a parent
*/
name: string;
static get properties(): {
name: {
type: StringConstructor;
reflect: boolean;
};
}

/**
* To encourage accessibility best practices, `form-element-register` events
* do not pierce through shadow roots. This forces the developer to create form groups and fieldsets that
Expand All @@ -17,12 +20,22 @@ export declare class FormRegisteringHost {
*/
allowCrossRootRegistration: boolean;

/**
* The name the element will be registered with to the .formElements collection
* of the parent. Also, it serves as the key of key/value pairs in
* modelValue/serializedValue objects
*/
get name(): string;
set name(arg: any);

/**
* The registrar this FormControl registers to, Usually a descendant of FormGroup or
* ChoiceGroup
*/
protected _parentFormGroup: FormRegistrarHost | undefined;

private __name: string;

private __unregisterFormElement: void;
}

Expand Down