Skip to content

Commit

Permalink
Improve EditableField error handling (#374)
Browse files Browse the repository at this point in the history
* Improve EditableField error handling

* Revert out-of-scope change

* Add test
  • Loading branch information
jeffdaley authored Oct 20, 2023
1 parent 89852b6 commit 9032b26
Show file tree
Hide file tree
Showing 4 changed files with 85 additions and 2 deletions.
12 changes: 11 additions & 1 deletion web/app/components/editable-field.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import Component from "@glimmer/component";
import { tracked } from "@glimmer/tracking";
import { action } from "@ember/object";
import { next, schedule, scheduleOnce } from "@ember/runloop";
import { schedule, scheduleOnce } from "@ember/runloop";
import { assert } from "@ember/debug";
import { guidFor } from "@ember/object/internals";
import { HermesDocument, HermesUser } from "hermes/types/document";
import blinkElement from "hermes/utils/blink-element";

export const FOCUSABLE =
'button, [href], input, select, textarea, [tabindex]:not([tabindex="-1"])';
Expand Down Expand Up @@ -185,6 +186,7 @@ export default class EditableFieldComponent extends Component<EditableFieldCompo
*/
@action protected disableEditing() {
this.editingIsEnabled = false;
this.emptyValueErrorIsShown = false;
}

/**
Expand Down Expand Up @@ -255,9 +257,17 @@ export default class EditableFieldComponent extends Component<EditableFieldCompo
(newValue instanceof Array && newValue.length === 0)
) {
if (this.args.isRequired) {
if (this.emptyValueErrorIsShown) {
const error =
this.editingContainer?.querySelector(".hds-form-error");
blinkElement(error);
return;
}

this.emptyValueErrorIsShown = true;
return;
}

/**
* We don't consider an empty value to be a change
* if the initial value is undefined.
Expand Down
25 changes: 25 additions & 0 deletions web/app/utils/blink-element.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import Ember from "ember";

/**
* Blinks an element twice by toggling its visibility.
* Used for emphasis, such as to reiterate an
* already-visible form error.
*/
const DURATION = Ember.testing ? 0 : 100;

export default function blinkElement(element?: Element | null) {
if (!element) {
return;
}

for (let i = 0; i < 4; i++) {
// Alternate between hidden and visible
let visibility = i % 2 === 0 ? "hidden" : "visible";

setTimeout(() => {
if (element) {
element.setAttribute("style", `visibility: ${visibility}`);
}
}, i * DURATION);
}
}
32 changes: 31 additions & 1 deletion web/tests/integration/components/editable-field-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,39 @@ module("Integration | Component | editable-field", function (hooks) {
assert.dom(ERROR).doesNotExist();

await fillIn("textarea", "");
await triggerKeyEvent(document, "keydown", "Enter");
await click(SAVE_BUTTON);

assert.dom(ERROR).exists();

// Cancel and try again

await click(CANCEL_BUTTON);

assert.dom(ERROR).doesNotExist();

await click(FIELD_TOGGLE);

assert.dom(ERROR).doesNotExist("the error state resets on cancel");

// Cause another error

await fillIn("textarea", "");
await click(SAVE_BUTTON);

assert.dom(ERROR).exists();

// Save a valid value

await fillIn("textarea", "bar");
await click(SAVE_BUTTON);

assert.dom(ERROR).doesNotExist();

// Reenable edit mode

await click(FIELD_TOGGLE);

assert.dom(ERROR).doesNotExist("the error state resets on save");
});

test("it conditionally determines whether to wrap the read-only value in a button", async function (this: EditableFieldComponentTestContext, assert) {
Expand Down
18 changes: 18 additions & 0 deletions web/tests/unit/utils/blink-element-test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import { waitUntil } from "@ember/test-helpers";
import blinkElement from "hermes/utils/blink-element";
import { module, test } from "qunit";

module("Unit | Utility | blink-element", function () {
test("it blinks an element", async function (assert) {
assert.expect(0);

const div = document.createElement("div");

blinkElement(div);

await waitUntil(() => div.style.visibility === "hidden");
await waitUntil(() => div.style.visibility === "visible");
await waitUntil(() => div.style.visibility === "hidden");
await waitUntil(() => div.style.visibility === "visible");
});
});

0 comments on commit 9032b26

Please sign in to comment.