Merge pull request #173 from capitec/Set-reflect-to-false-on-value-pr…

…op-on-pin-and-password-field

Update Password and Pin Field to not reflect value attribute in the DOM

src/core/OmniFormElement.ts

@@ -137,7 +137,6 @@ export class OmniFormElement extends OmniElement {
         }
 
         this.value = '';
-
         // Dispatch standard DOM event to cater for single clear.
         this.dispatchEvent(
             new Event('change', {
@@ -241,44 +240,49 @@ export class OmniFormElement extends OmniElement {
                     border-color: var(--omni-form-focussed-border-color, var(--omni-primary-active-color));
                 }
 
-                :host([value]:not([value=''])) .layout  > .label,
+                :host([value]:not([value=''])) .container:not(.no-float-label) .layout  > .label,
+                .container.float-label .layout  > .label,
                 .layout:focus-within > .label:not(.focused-static)
                 {
                     top: 0px;
                     margin-left: var(--omni-form-focussed-label-margin-left, 10px);
                 }
-
+                
                 .layout:focus-within > .label {
                     color: var(--omni-form-focussed-label-color, var(--omni-primary-color));
                 }
-
-                :host([value]:not([value=''])) .layout  > .label.error,
+                
+                :host([value]:not([value=''])) .container:not(.no-float-label) .layout  > .label.error,
+                .container.float-label .layout  > .label.error,
                 .layout:focus-within > .label.error {
                     color: var(--omni-form-focussed-error-label-color, var(--omni-error-font-color));
                 }
-            
-                :host([value]:not([value=''])) .layout  > .label > div::before,
+                
+                :host([value]:not([value=''])) .container:not(.no-float-label) .layout  > .label > div::before,
+                .container.float-label .layout  > .label > div::before,
                 .layout:focus-within > .label:not(.focused-static) > div::before
                 {
                     content: "";
-					display: block;           
-					background-color: var(--omni-form-focussed-label-background-color, var(--omni-background-color));
-					position: absolute;
-					left: calc(var(--omni-form-focussed-label-padding-left, 3px) * -1);
-					right: calc(var(--omni-form-focussed-label-padding-right, 3px) * -1);
-    				height: 50%;
-					z-index: -1;
+                    display: block;           
+                    background-color: var(--omni-form-focussed-label-background-color, var(--omni-background-color));
+                    position: absolute;
+                    left: calc(var(--omni-form-focussed-label-padding-left, 3px) * -1);
+                    right: calc(var(--omni-form-focussed-label-padding-right, 3px) * -1);
+                    height: 50%;
+                    z-index: -1;
                     top:50%;
                     width: calc(100% + var(--omni-form-focussed-label-padding-left, 3px) + var(--omni-form-focussed-label-padding-right, 3px));
                 }
-
-                :host([value]:not([value=''])) .layout.disabled  > .label > div::before,
+                
+                :host([value]:not([value=''])) .container:not(.no-float-label) .layout.disabled  > .label > div::before,
+                .container.float-label .layout.disabled  > .label > div::before,
                 .layout.disabled:focus-within > .label > div::before 
                 {
                     background-color: var(--omni-form-focussed-label-disabled-background-color, var(--omni-disabled-background-color));
                 }
-
-                :host([value]:not([value=''])) .layout  > .label > div,
+                
+                :host([value]:not([value=''])) .container:not(.no-float-label) .layout  > .label > div,
+                .container.float-label .layout  > .label > div,
                 .layout:focus-within > .label > div {
                     transform: scale(var(--omni-form-focussed-label-transform-scale), 0.9);
                     transform-origin: center left;

src/password-field/PasswordField.spec.ts

@@ -48,6 +48,30 @@ test(`Password Field - Visual and Behaviour`, async ({ page }) => {
     });
 });
 
+test(`Password Field - Behaviour`, async ({ page }) => {
+    await withCoverage(page, async () => {
+        await page.goto('/components/password-field/');
+        await page.evaluate(() => document.fonts.ready);
+
+        const passwordField = page.locator('[data-testid]').first();
+        await expect(passwordField).toHaveScreenshot('password-field-initial.png');
+
+        const showSlotElement = passwordField.locator('slot[name=show]');
+        await showSlotElement.click();
+
+        const hideSlotElement = passwordField.locator('slot[name=hide]');
+        await hideSlotElement.click();
+
+        const inputField = passwordField.locator('#inputField');
+
+        const valueUpdate = 'Value Update';
+        await passwordField.evaluate((p: PasswordField, valueUpdate) => (p.value = valueUpdate), valueUpdate);
+
+        await expect(inputField).toHaveValue(valueUpdate);
+        await expect(passwordField).toHaveScreenshot('password-field-value-update.png');
+    });
+});
+
 test('Password Field - Label Behaviour', testLabelBehaviour('omni-password-field'));
 test('Password Field - Hint Behaviour', testHintBehaviour('omni-password-field'));
 test('Password Field - Error Behaviour', testErrorBehaviour('omni-password-field'));

src/password-field/PasswordField.ts

@@ -1,7 +1,6 @@
 import { css, html } from 'lit';
 import { customElement, property, query, state } from 'lit/decorators.js';
 import { ClassInfo, classMap } from 'lit/directives/class-map.js';
-import { live } from 'lit/directives/live.js';
 import { ifDefined, OmniFormElement } from '../core/OmniFormElement.js';
 
 import '../icons/EyeHidden.icon.js';
@@ -60,6 +59,11 @@ export class PasswordField extends OmniFormElement {
      */
     @state() protected type: 'password' | 'text' = 'password';
 
+    /**
+     * Override for the value property inherited from the OmniFormElement component with reflect set to false.
+     */
+    @property({ type: String, reflect: false }) override value?: string;
+
     /**
      * Disables native on screen keyboards for the component.
      * @attr [no-native-keyboard]
@@ -68,6 +72,9 @@ export class PasswordField extends OmniFormElement {
 
     @query('#inputField')
     private _inputElement?: HTMLInputElement;
+    @query('.container')
+    private container?: HTMLDivElement;
+    private _value? = '';
 
     override connectedCallback() {
         super.connectedCallback();
@@ -79,6 +86,39 @@ export class PasswordField extends OmniFormElement {
         });
     }
 
+    protected override async firstUpdated(): Promise<void> {
+        this._setInputValue();
+    }
+
+    constructor() {
+        super();
+        this._value = this.value ?? '';
+        Object.defineProperty(this, 'value', {
+            get: () => {
+                return this._value;
+            },
+            set: (v) => {
+                this._value = v ?? '';
+                this._setInputValue();
+                if (this._value) {
+                    this.container?.classList?.add('float-label');
+                    this.container?.classList?.remove('no-float-label');
+                } else {
+                    this.container?.classList?.remove('float-label');
+                    this.container?.classList?.add('no-float-label');
+                }
+                this.requestUpdate();
+            }
+        });
+    }
+
+    // Set the value of the input component.
+    _setInputValue() {
+        if (this._inputElement) {
+            this._inputElement.value = this.value as string;
+        }
+    }
+
     _focusInput() {
         const input = this._inputElement;
         if (input) {
@@ -207,7 +247,6 @@ export class PasswordField extends OmniFormElement {
         id="inputField"
         .type="${this.type}"
         inputmode="${ifDefined(this.noNativeKeyboard ? 'none' : undefined)}"
-        .value=${live(this.value as string)}
         ?readOnly=${this.disabled}
         tabindex="${this.disabled ? -1 : 0}" />
     `;

src/pin-field/PinField.spec.ts

@@ -48,6 +48,41 @@ test(`Pin Field - Visual and Behaviour`, async ({ page }) => {
     });
 });
 
+test(`Pin Field - Behaviour`, async ({ page }) => {
+    await withCoverage(page, async () => {
+        await page.goto('/components/pin-field/');
+        await page.evaluate(() => document.fonts.ready);
+
+        const pinField = page.locator('[data-testid]').first();
+        await expect(pinField).toHaveScreenshot('pin-field-initial.png');
+
+        const showSlotElement = pinField.locator('slot[name=show]');
+        await expect(showSlotElement).toHaveCount(1);
+        await showSlotElement.click();
+        await expect(pinField).toHaveScreenshot('pin-field-show.png');
+
+        const hideSlotElement = pinField.locator('slot[name=hide]');
+        await expect(hideSlotElement).toHaveCount(1);
+        await hideSlotElement.click();
+        await expect(pinField).toHaveScreenshot('pin-field-hide.png');
+
+        const inputField = pinField.locator('#inputField');
+
+        const number = '1234';
+        await pinField.evaluate((p: PinField, number) => (p.value = number), number);
+        await expect(inputField).toHaveValue(number);
+        await showSlotElement.click();
+        await expect(pinField).toHaveScreenshot('pin-field-evaluate.png');
+
+        const invalidNumber = '56abc78';
+        await pinField.evaluate((p: PinField, invalidNumber) => (p.value = invalidNumber), invalidNumber);
+        await expect(pinField).toHaveScreenshot('pin-field-evaluate-invalid.png');
+
+        await expect(inputField).toHaveValue('5678');
+        await expect(pinField).toHaveScreenshot('pin-field-invalid-value.png');
+    });
+});
+
 test(`Pin Field - Max Length Behaviour`, async ({ page }) => {
     await withCoverage(page, async () => {
         await page.goto('/components/pin-field/');

src/pin-field/PinField.ts

@@ -65,6 +65,11 @@ export class PinField extends OmniFormElement {
      */
     @property({ type: Boolean, reflect: true, attribute: 'no-native-keyboard' }) noNativeKeyboard?: boolean;
 
+    /**
+     * Override for the value property inherited from the OmniFormElement component with reflect set to false.
+     */
+    @property({ type: String, reflect: false }) override value?: string;
+
     /**
      * Maximum character input length.
      * @attr [max-length]
@@ -73,9 +78,12 @@ export class PinField extends OmniFormElement {
 
     @query('#inputField')
     private _inputElement?: HTMLInputElement;
+    @query('.container')
+    private container?: HTMLDivElement;
     private showPin?: boolean = false;
     // eslint-disable-next-line @typescript-eslint/no-explicit-any
     private isWebkit?: boolean;
+    private _value? = '';
 
     override connectedCallback() {
         super.connectedCallback();
@@ -99,14 +107,7 @@ export class PinField extends OmniFormElement {
             this.type = 'password';
         }
 
-        this._sanitiseValue(this.value as string);
-    }
-
-    override async attributeChangedCallback(name: string, _old: string | null, value: string | null): Promise<void> {
-        super.attributeChangedCallback(name, _old, value);
-        if (name === 'value') {
-            this._sanitiseValue(value as string);
-        }
+        this._sanitiseValue();
     }
 
     override focus(options?: FocusOptions | undefined): void {
@@ -117,16 +118,38 @@ export class PinField extends OmniFormElement {
         }
     }
 
-    // Checks if the value provided is numeric, if valid set the value property and input element value if not value remove the value attribute
-    _sanitiseValue(value: string) {
-        if (value) {
-            if (!this._isNumber(value as string)) {
-                this.removeAttribute('value');
-            } else if (this.maxLength && (value as string).length > this.maxLength) {
-                this.value = value?.slice(0, this.maxLength) as string;
+    constructor() {
+        super();
+        this._value = this.value ?? '';
+        Object.defineProperty(this, 'value', {
+            get: () => {
+                return this._value;
+            },
+            set: (v) => {
+                this._value = v ?? '';
+                this._sanitiseValue();
+                if (this._value) {
+                    this.container?.classList?.add('float-label');
+                    this.container?.classList?.remove('no-float-label');
+                } else {
+                    this.container?.classList?.remove('float-label');
+                    this.container?.classList?.add('no-float-label');
+                }
+                this.requestUpdate();
+            }
+        });
+    }
+
+    // Check if the value provided is valid and shorten according to the max length if provided, if there is invalid alpha characters they are removed.
+    _sanitiseValue() {
+        if (this.value) {
+            if (this.maxLength && (this.value as string).length > this.maxLength) {
+                this._value = this.value?.slice(0, this.maxLength) as string;
             }
         }
 
+        this._value = this.value?.toString()?.replace(/[^\d]/gi, '');
+
         if (this._inputElement) {
             this._inputElement.value = this.value as string;
         }
@@ -266,6 +289,7 @@ export class PinField extends OmniFormElement {
                 input[type='number'] {
                     -moz-appearance: textfield; /* Firefox */
                 }
+
             `
         ];
     }

src/select/Select.spec.ts

@@ -87,6 +87,7 @@ test(`Select - Async Per Item Behaviour`, async ({ page, isMobile }) => {
         }, displayItems);
 
         await selectComponent.click();
+        // Added for cases where the items do not load in time before taking a screenshot.
         await page.waitForTimeout(200);
 
         await expect(selectComponent).toHaveScreenshot('select-open.png');
@@ -195,6 +196,7 @@ test(`Select - Selection Render Behaviour`, async ({ page, isMobile }) => {
         const item = selectComponent.locator('.item').first();
         await expect(item).toHaveCount(1);
         await item.click();
+        // Added timeout to wait for items to render.
         await page.waitForTimeout(100);
 
         const selectionRender = selectComponent.locator('omni-render-element');