Skip to content

Commit

Permalink
fix(form-core): Implement conversion of name attribute value to strin…
Browse files Browse the repository at this point in the history
…g type
  • Loading branch information
donecarlo committed May 22, 2024
1 parent 08a1cb1 commit 57597bb
Show file tree
Hide file tree
Showing 6 changed files with 67 additions and 17 deletions.
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 },
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
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

0 comments on commit 57597bb

Please sign in to comment.