Refactored <GhBenefitItem>

refs https://github.com/TryGhost/Ghost/issues/14101

- migrated component to Glimmer
- swapped usage of `ValidationState` mixin for `{{validation-status}}` modifier
  - updated modifier to accept custom error/success classes
- removed unnecessary/unused code in the `gh-benefit-item.js` backing class

ghost/admin/app/components/gh-benefit-item.hbs

@@ -1,39 +1,46 @@
-{{#unless this.benefitItem.isNew}}
+<div
+    class="gh-blognav-item {{unless @benefitItem.isNew "gh-blognav-item--sortable"}}"
+    {{validation-status errorClass="gh-blognav-item--error" errors=@benefitItem.errors}}
+    ...attributes
+>
+    {{#unless @benefitItem.isNew}}
         <span class="gh-blognav-grab">
             {{svg-jar "grab"}}
             <span class="sr-only">Reorder</span>
         </span>
-{{/unless}}
+    {{/unless}}
 
-<div class="gh-blognav-line {{unless this.name "placeholder"}}">
+    <div class="gh-blognav-line {{unless @benefitItem.name "placeholder"}}">
         {{svg-jar "check-2"}}
         <span
             class="gh-blognav-label"
-        {{validation-status errors=this.benefitItem.errors property="name" hasValidated=this.benefitItem.hasValidated}}
+            {{validation-status errors=@benefitItem.errors property="name" hasValidated=@benefitItem.hasValidated}}
         >
             <GhTrimFocusInput
-            @shouldFocus={{this.benefitItem.last}}
+                @shouldFocus={{@benefitItem.last}}
                 @placeholder={{if this.isFreeTier "Access to all public posts" "Expert analysis"}}
-            @value={{readonly this.name}}
+                @value={{readonly @benefitItem.name}}
-            @input={{action "updateLabel" value="target.value"}}
+                @input={{this.handleLabelInput}}
-            @keyPress={{action "clearLabelErrors"}}
+                @keyPress={{this.clearLabelErrors}}
                 @stopEnterKeyDownPropagation={{true}}
-            @focus-out={{action "updateLabel" this.name}}
+                @focus-out={{fn this.updateLabel @benefitItem.name}}
                 data-test-input="benefit-label" />
 
             <GhErrorMessage
-            @errors={{this.benefitItem.errors}}
+                @errors={{@benefitItem.errors}}
                 @property="name"
                 data-test-error="benefit-label" />
         </span>
-</div>
+    </div>
 
-{{#if this.benefitItem.isNew}}
+    {{#if @benefitItem.isNew}}
-    <button type="button" class="gh-blognav-add" {{action "addItem" this.benefitItem}} data-test-button="add-benefit">
+        <button type="button" class="gh-blognav-add" {{on "click" (fn @addItem @benefitItem)}} data-test-button="add-benefit">
             {{svg-jar "plus"}}<span class="sr-only">Add</span>
         </button>
-{{else}}
+    {{else}}
-    <button type="button" class="gh-blognav-delete" {{action "deleteItem" this.benefitItem}} data-test-button="delete-benefit">
+        <button type="button" class="gh-blognav-delete" {{on "click" (fn @deleteItem @benefitItem)}} data-test-button="delete-benefit">
             {{svg-jar "trash"}}<span class="sr-only">Delete</span>
         </button>
-{{/if}}
+    {{/if}}
+
+</div>

ghost/admin/app/components/gh-benefit-item.js

@@ -1,58 +1,19 @@
-import Component from '@ember/component';
+import Component from '@glimmer/component';
-import ValidationState from 'ghost-admin/mixins/validation-state';
+import {action} from '@ember/object';
-import boundOneWay from 'ghost-admin/utils/bound-one-way';
-import {computed} from '@ember/object';
-import {readOnly} from '@ember/object/computed';
-import {run} from '@ember/runloop';
 
-export default Component.extend(ValidationState, {
+export default class GhBenefitItem extends Component {
-    classNames: 'gh-blognav-item',
+    @action
-    classNameBindings: ['errorClass', 'benefitItem.isNew::gh-blognav-item--sortable'],
+    handleLabelInput(event) {
-
+        this.updateLabel(event.target.value);
-    new: false,
+    }
-
-    // closure actions
-    addItem() {},
-    deleteItem() {},
-    updateLabel() {},
-    name: boundOneWay('benefitItem.name'),
-
-    errors: readOnly('benefitItem.errors'),
-
-    errorClass: computed('hasError', function () {
-        return this.hasError ? 'gh-blognav-item--error' : '';
-    }),
-
-    actions: {
-        addItem(item) {
-            this.addItem(item);
-        },
-
-        deleteItem(item) {
-            this.deleteItem(item);
-        },
 
+    @action
     updateLabel(value) {
-            this.set('name', value);
+        this.args.updateLabel(value, this.args.benefitItem);
-            return this.updateLabel(value, this.benefitItem);
+    }
-        },
 
+    @action
     clearLabelErrors() {
-            if (this.get('benefitItem.errors')) {
+        this.args.benefitItem.errors?.remove('name');
-                this.get('benefitItem.errors').remove('name');
     }
-        }
+}
-    },
-
-    keyPress(event) {
-        // enter key
-        if (event.keyCode === 13) {
-            event.preventDefault();
-            if (this.get('benefitItem.isNew')) {
-                run.scheduleOnce('actions', this, this.send, 'addItem', this.benefitItem);
-            } else {
-                run.scheduleOnce('actions', this, this.send, 'focusItem', this.benefitItem);
-            }
-        }
-    }
-});

ghost/admin/app/modifiers/validation-status.js

@@ -1,12 +1,18 @@
 import Modifier from 'ember-modifier';
 import {isEmpty} from '@ember/utils';
 
-const errorClass = 'error';
+const ERROR_CLASS = 'error';
-const successClass = 'success';
+const SUCCESS_CLASS = 'success';
 
 export default class ValidationStatusModifier extends Modifier {
-    modify(element, positional, {errors, property, hasValidated}) {
+    modify(element, positional, {errors, property, hasValidated, errorClass = ERROR_CLASS, successClass = SUCCESS_CLASS}) {
-        const validationClass = this.errorClass(errors, property, hasValidated);
+        const hasError = this.hasError(errors, property, hasValidated);
+
+        let validationClass = '';
+
+        if (!property || hasValidated?.includes(property)) {
+            validationClass = hasError ? errorClass : successClass;
+        }
 
         element.classList.remove(errorClass);
         element.classList.remove(successClass);
@@ -16,16 +22,6 @@ export default class ValidationStatusModifier extends Modifier {
         }
     }
 
-    errorClass(errors, property, hasValidated) {
-        const hasError = this.hasError(errors, property, hasValidated);
-
-        if (hasValidated && hasValidated.includes(property)) {
-            return hasError ? errorClass : successClass;
-        } else {
-            return '';
-        }
-    }
-
     hasError(errors, property, hasValidated) {
         // if we aren't looking at a specific property we always want an error class
         if (!property && errors && !errors.get('isEmpty')) {

ghost/admin/tests/integration/modifiers/validation-status-test.js

@@ -0,0 +1,129 @@
+import DS from 'ember-data'; // eslint-disable-line
+import EmberObject from '@ember/object';
+import hbs from 'htmlbars-inline-precompile';
+import {expect} from 'chai';
+import {find, render} from '@ember/test-helpers';
+import {settled} from '@ember/test-helpers';
+import {setupRenderingTest} from 'ember-mocha';
+
+const {Errors} = DS;
+
+describe('Integration: Modifier: validation-status', function () {
+    setupRenderingTest();
+
+    this.beforeEach(function () {
+        let testObject = EmberObject.create();
+        testObject.name = 'Test';
+        testObject.hasValidated = [];
+        testObject.errors = Errors.create();
+
+        this.set('testObject', testObject);
+    });
+
+    it('handles missing params', async function () {
+        await render(hbs`<div class="test" {{validation-status}}></div>`);
+        expect(find('.test')).to.exist;
+    });
+
+    describe('with errors/property/hasValidated params', function () {
+        it('has no success/error class by default', async function () {
+            await render(hbs`
+                <div
+                    class="test"
+                    {{validation-status
+                        property="name"
+                        errors=this.testObject.errors
+                        hasValidated=this.testObject.hasValidated
+                    }}
+                ></div>
+            `);
+
+            expect(find('.test').classList).to.have.length(1);
+        });
+
+        it('has success class when valid', async function () {
+            await render(hbs`
+                <div
+                    class="test"
+                    {{validation-status
+                        property="name"
+                        errors=this.testObject.errors
+                        hasValidated=this.testObject.hasValidated
+                    }}
+                ></div>
+            `);
+
+            this.testObject.hasValidated.pushObject('name'); // pushObject vs push because this is an EmberArray and we're testing tracking
+            await settled();
+
+            expect(find('.test')).to.have.class('success');
+            expect(find('.test')).to.not.have.class('error');
+        });
+
+        it('has error class when invalid', async function () {
+            await render(hbs`
+                <div
+                    class="test"
+                    {{validation-status
+                        property="name"
+                        errors=this.testObject.errors
+                        hasValidated=this.testObject.hasValidated
+                    }}
+                ></div>
+            `);
+
+            this.testObject.hasValidated.pushObject('name'); // pushObject vs push because this is an EmberArray and we're testing tracking
+            this.testObject.errors.add('name', 'has error');
+            await settled();
+
+            expect(find('.test')).to.have.class('error');
+            expect(find('.test')).to.not.have.class('success');
+        });
+
+        it('always has error class when no property is passed', async function () {
+            await render(hbs`
+                <div
+                    class="test"
+                    {{validation-status
+                        errors=this.testObject.errors
+                        hasValidated=this.testObject.hasValidated
+                    }}
+                ></div>
+            `);
+
+            this.testObject.hasValidated.pushObject('different'); // pushObject vs push because this is an EmberArray and we're testing tracking
+            this.testObject.errors.add('different', 'has error');
+            await settled();
+
+            expect(find('.test')).to.have.class('error');
+            expect(find('.test')).to.not.have.class('success');
+        });
+
+        it('can have custom success/error classes', async function () {
+            await render(hbs`
+                <div
+                    class="test"
+                    {{validation-status
+                        property="name"
+                        errors=this.testObject.errors
+                        hasValidated=this.testObject.hasValidated
+                        errorClass="custom-error"
+                        successClass="custom-success"
+                    }}
+                ></div>
+            `);
+
+            this.testObject.hasValidated.pushObject('name'); // pushObject vs push because this is an EmberArray and we're testing tracking
+            await settled();
+
+            expect(find('.test')).to.have.class('custom-success');
+            expect(find('.test')).to.not.have.class('success');
+
+            this.testObject.errors.add('name', 'invalid');
+            await settled();
+
+            expect(find('.test')).to.have.class('custom-error');
+            expect(find('.test')).to.not.have.class('error');
+        });
+    });
+});