diff --git a/assets/js/phoenix_live_view/constants.js b/assets/js/phoenix_live_view/constants.js index 64e4ac2700..ba82da2fc7 100644 --- a/assets/js/phoenix_live_view/constants.js +++ b/assets/js/phoenix_live_view/constants.js @@ -37,6 +37,8 @@ export const PHX_ROOT_ID = "data-phx-root-id" export const PHX_VIEWPORT_TOP = "viewport-top" export const PHX_VIEWPORT_BOTTOM = "viewport-bottom" export const PHX_TRIGGER_ACTION = "trigger-action" +export const PHX_FEEDBACK_FOR = "feedback-for" +export const PHX_FEEDBACK_GROUP = "feedback-group" export const PHX_HAS_FOCUSED = "phx-has-focused" export const FOCUSABLE_INPUTS = ["text", "textarea", "number", "email", "password", "search", "tel", "url", "date", "time", "datetime-local", "color", "range"] export const CHECKABLE_INPUTS = ["checkbox", "radio"] diff --git a/assets/js/phoenix_live_view/dom.js b/assets/js/phoenix_live_view/dom.js index 9005d9983b..953606b453 100644 --- a/assets/js/phoenix_live_view/dom.js +++ b/assets/js/phoenix_live_view/dom.js @@ -342,11 +342,6 @@ let DOM = { JS.addOrRemoveClasses(container, [PHX_NO_FEEDBACK_CLASS], []) }, - isUsedInput(el){ - return(el.nodeType === Node.ELEMENT_NODE && - (this.private(el, PHX_HAS_FOCUSED) || this.private(el, PHX_HAS_SUBMITTED))) - }, - shouldHideFeedback(container, nameOrGroup, phxFeedbackGroup){ const query = `[name="${nameOrGroup}"], [name="${nameOrGroup}[]"], @@ -369,10 +364,14 @@ let DOM = { return query }, - resetForm(form){ + resetForm(form, phxFeedbackFor, phxFeedbackGroup){ Array.from(form.elements).forEach(input => { + let query = this.feedbackSelector(input, phxFeedbackFor, phxFeedbackGroup) this.deletePrivate(input, PHX_HAS_FOCUSED) this.deletePrivate(input, PHX_HAS_SUBMITTED) + this.all(document, query, feedbackEl => { + JS.addOrRemoveClasses(feedbackEl, [PHX_NO_FEEDBACK_CLASS], []) + }) }) }, diff --git a/assets/js/phoenix_live_view/dom_patch.js b/assets/js/phoenix_live_view/dom_patch.js index 90c282981b..ea439044bf 100644 --- a/assets/js/phoenix_live_view/dom_patch.js +++ b/assets/js/phoenix_live_view/dom_patch.js @@ -1,6 +1,8 @@ import { PHX_COMPONENT, PHX_DISABLE_WITH, + PHX_FEEDBACK_FOR, + PHX_FEEDBACK_GROUP, PHX_PRUNE, PHX_ROOT_ID, PHX_SESSION, @@ -51,7 +53,6 @@ export default class DOMPatch { this.cidPatch = isCid(this.targetCID) this.pendingRemoves = [] this.phxRemove = this.liveSocket.binding("remove") - this.targetContainer = this.isCIDPatch() ? this.targetCIDContainer(html) : container this.callbacks = { beforeadded: [], beforeupdated: [], beforephxChildAdded: [], afteradded: [], afterupdated: [], afterdiscarded: [], afterphxChildAdded: [], @@ -78,17 +79,21 @@ export default class DOMPatch { } perform(isJoinPatch){ - let {view, liveSocket, html, container, targetContainer} = this + let {view, liveSocket, container, html} = this + let targetContainer = this.isCIDPatch() ? this.targetCIDContainer(html) : container if(this.isCIDPatch() && !targetContainer){ return } let focused = liveSocket.getActiveElement() let {selectionStart, selectionEnd} = focused && DOM.hasSelectionRange(focused) ? focused : {} let phxUpdate = liveSocket.binding(PHX_UPDATE) + let phxFeedbackFor = liveSocket.binding(PHX_FEEDBACK_FOR) + let phxFeedbackGroup = liveSocket.binding(PHX_FEEDBACK_GROUP) let disableWith = liveSocket.binding(PHX_DISABLE_WITH) let phxViewportTop = liveSocket.binding(PHX_VIEWPORT_TOP) let phxViewportBottom = liveSocket.binding(PHX_VIEWPORT_BOTTOM) let phxTriggerExternal = liveSocket.binding(PHX_TRIGGER_ACTION) let added = [] + let feedbackContainers = [] let updates = [] let appendPrependUpdates = [] @@ -143,6 +148,7 @@ export default class DOMPatch { }, onNodeAdded: (el) => { if(el.getAttribute){ this.maybeReOrderStream(el, true) } + if(DOM.isFeedbackContainer(el, phxFeedbackFor)) feedbackContainers.push(el) // hack to fix Safari handling of img srcset and video tags if(el instanceof HTMLImageElement && el.srcset){ @@ -181,6 +187,12 @@ export default class DOMPatch { }, onBeforeElUpdated: (fromEl, toEl) => { DOM.maybeAddPrivateHooks(toEl, phxViewportTop, phxViewportBottom) + // mark both from and to els as feedback containers, as we don't know yet which one will be used + // and we also need to remove the phx-no-feedback class when the phx-feedback-for attribute is removed + if(DOM.isFeedbackContainer(fromEl, phxFeedbackFor) || DOM.isFeedbackContainer(toEl, phxFeedbackFor)){ + feedbackContainers.push(fromEl) + feedbackContainers.push(toEl) + } DOM.cleanChildNodes(toEl, phxUpdate) if(this.skipCIDSibling(toEl)){ // if this is a live component used in a stream, we may need to reorder it @@ -297,6 +309,8 @@ export default class DOMPatch { }) } + DOM.maybeHideFeedback(targetContainer, feedbackContainers, phxFeedbackFor, phxFeedbackGroup) + liveSocket.silenceEvents(() => DOM.restoreFocus(focused, selectionStart, selectionEnd)) DOM.dispatchEvent(document, "phx:update") added.forEach(el => this.trackAfter("added", el)) diff --git a/assets/js/phoenix_live_view/index.js b/assets/js/phoenix_live_view/index.js index a40a4842a1..bfde82e78d 100644 --- a/assets/js/phoenix_live_view/index.js +++ b/assets/js/phoenix_live_view/index.js @@ -7,8 +7,7 @@ See the hexdocs at `https://hexdocs.pm/phoenix_live_view` for documentation. */ -import LiveSocket, {isUsedInput} from "./live_socket" +import LiveSocket from "./live_socket" export { - LiveSocket, - isUsedInput + LiveSocket } diff --git a/assets/js/phoenix_live_view/live_socket.js b/assets/js/phoenix_live_view/live_socket.js index 4e9aa9e8ca..43776012e4 100644 --- a/assets/js/phoenix_live_view/live_socket.js +++ b/assets/js/phoenix_live_view/live_socket.js @@ -95,6 +95,8 @@ import { PHX_THROTTLE, PHX_TRACK_UPLOADS, PHX_SESSION, + PHX_FEEDBACK_FOR, + PHX_FEEDBACK_GROUP, RELOAD_JITTER_MIN, RELOAD_JITTER_MAX, PHX_REF, @@ -115,8 +117,6 @@ import LiveUploader from "./live_uploader" import View from "./view" import JS from "./js" -export let isUsedInput = (el) => DOM.isUsedInput(el) - export default class LiveSocket { constructor(url, phxSocket, opts = {}){ this.unloaded = false @@ -889,7 +889,7 @@ export default class LiveSocket { } this.on("reset", (e) => { let form = e.target - DOM.resetForm(form) + DOM.resetForm(form, this.binding(PHX_FEEDBACK_FOR), this.binding(PHX_FEEDBACK_GROUP)) let input = Array.from(form.elements).find(el => el.type === "reset") if(input){ // wait until next tick to get updated input value diff --git a/assets/js/phoenix_live_view/view.js b/assets/js/phoenix_live_view/view.js index 53fbffd141..3ba6af04f2 100644 --- a/assets/js/phoenix_live_view/view.js +++ b/assets/js/phoenix_live_view/view.js @@ -13,6 +13,8 @@ import { PHX_ERROR_CLASS, PHX_CLIENT_ERROR_CLASS, PHX_SERVER_ERROR_CLASS, + PHX_FEEDBACK_FOR, + PHX_FEEDBACK_GROUP, PHX_HAS_FOCUSED, PHX_HAS_SUBMITTED, PHX_HOOK, @@ -55,17 +57,6 @@ import Rendered from "./rendered" import ViewHook from "./view_hook" import JS from "./js" -export let prependFormDataKey = (key, prefix) => { - let isArray = key.endsWith("[]") - // Remove the "[]" if it's an array - let baseKey = isArray ? key.slice(0, -2) : key - // Replace last occurrence of key before a closing bracket or the end with key plus suffix - baseKey = baseKey.replace(/([^\[\]]+)(\]?$)/, `${prefix}$1$2`) - // Add back the "[]" if it was an array - if(isArray){ baseKey += "[]" } - return baseKey -} - let serializeForm = (form, metadata, onlyNames = []) => { const {submitter, ...meta} = metadata @@ -99,15 +90,8 @@ let serializeForm = (form, metadata, onlyNames = []) => { const params = new URLSearchParams() - let elements = Array.from(form.elements) for(let [key, val] of formData.entries()){ if(onlyNames.length === 0 || onlyNames.indexOf(key) >= 0){ - let inputs = elements.filter(input => input.name === key) - let isUnused = !inputs.some(input => (DOM.private(input, PHX_HAS_FOCUSED) || DOM.private(input, PHX_HAS_SUBMITTED))) - let hidden = inputs.every(input => input.type === "hidden") - if(isUnused && !(submitter && submitter.name == key) && !hidden){ - params.append(prependFormDataKey(key, "_unused_"), "") - } params.append(key, val) } } @@ -445,8 +429,6 @@ export default class View { let phxChildrenAdded = false let updatedHookIds = new Set() - this.liveSocket.triggerDOM("onPatchStart", [patch.targetContainer]) - patch.after("added", el => { this.liveSocket.triggerDOM("onNodeAdded", [el]) let phxViewportTop = this.binding(PHX_VIEWPORT_TOP) @@ -481,8 +463,6 @@ export default class View { patch.perform(isJoinPatch) this.afterElementsRemoved(removedEls, pruneCids) - - this.liveSocket.triggerDOM("onPatchEnd", [patch.targetContainer]) return phxChildrenAdded } @@ -1048,6 +1028,7 @@ export default class View { cid: cid } this.pushWithReply(refGenerator, "event", event, resp => { + DOM.showError(inputEl, this.liveSocket.binding(PHX_FEEDBACK_FOR), this.liveSocket.binding(PHX_FEEDBACK_GROUP)) if(DOM.isUploadInput(inputEl) && DOM.isAutoUpload(inputEl)){ if(LiveUploader.filesAwaitingPreflight(inputEl).length > 0){ let [ref, _els] = refGenerator() @@ -1360,10 +1341,13 @@ export default class View { submitForm(form, targetCtx, phxEvent, submitter, opts = {}){ DOM.putPrivate(form, PHX_HAS_SUBMITTED, true) + const phxFeedbackFor = this.liveSocket.binding(PHX_FEEDBACK_FOR) + const phxFeedbackGroup = this.liveSocket.binding(PHX_FEEDBACK_GROUP) const inputs = Array.from(form.elements) inputs.forEach(input => DOM.putPrivate(input, PHX_HAS_SUBMITTED, true)) this.liveSocket.blurActiveElement(this) this.pushFormSubmit(form, targetCtx, phxEvent, submitter, opts, () => { + inputs.forEach(input => DOM.showError(input, phxFeedbackFor, phxFeedbackGroup)) this.liveSocket.restorePreviouslyActiveFocus() }) } diff --git a/assets/test/js_test.js b/assets/test/js_test.js index c2dd12aa11..75c479b9da 100644 --- a/assets/test/js_test.js +++ b/assets/test/js_test.js @@ -333,7 +333,7 @@ describe("JS", () => { expect(Array.from(modal2.classList)).toEqual(["modal", "class1"]) JS.exec("click", toggle.getAttribute("phx-click"), view, toggle) jest.runAllTimers() - + expect(Array.from(modal1.classList)).toEqual(["modal"]) expect(Array.from(modal2.classList)).toEqual(["modal"]) done() @@ -418,7 +418,7 @@ describe("JS", () => { event: "validate", type: "form", uploads: {}, - value: "_unused_username=&username=&_unused_other=&other=&_target=username" + value: "username=&other=&_target=username" }) done() } @@ -449,7 +449,7 @@ describe("JS", () => { event: "username_changed", type: "form", uploads: {}, - value: "_unused_username=&username=&_target=username" + value: "username=&_target=username" }) done() } @@ -480,7 +480,7 @@ describe("JS", () => { event: "username_changed", type: "form", uploads: {}, - value: "_unused_username=&username=&_target=username" + value: "username=&_target=username" }) done() } diff --git a/assets/test/view_test.js b/assets/test/view_test.js index d7a066eb44..f18e475812 100644 --- a/assets/test/view_test.js +++ b/assets/test/view_test.js @@ -6,16 +6,11 @@ import View from "phoenix_live_view/view" import { PHX_LOADING_CLASS, PHX_ERROR_CLASS, - PHX_SERVER_ERROR_CLASS, - PHX_HAS_FOCUSED + PHX_SERVER_ERROR_CLASS } from "phoenix_live_view/constants" import {tag, simulateJoinedView, stubChannel, rootContainer, liveViewDOM, simulateVisibility} from "./test_helpers" -let simulateUsedInput = (input) => { - DOM.putPrivate(input, PHX_HAS_FOCUSED, true) -} - describe("View + DOM", function(){ beforeEach(() => { submitBefore = HTMLFormElement.prototype.submit @@ -212,13 +207,13 @@ describe("View + DOM", function(){ let liveSocket = new LiveSocket("/live", Socket) let el = liveViewDOM() let input = el.querySelector("input") - simulateUsedInput(input) + let view = simulateJoinedView(el, liveSocket) let channelStub = { push(_evt, payload, _timeout){ expect(payload.type).toBe("form") expect(payload.event).toBeDefined() - expect(payload.value).toBe("increment=1&_unused_note=¬e=2&_target=increment") + expect(payload.value).toBe("increment=1¬e=2&_target=increment") return { receive(){ return this } } @@ -1020,6 +1015,7 @@ describe("View + Component", function(){ }) test("pushInput", function(done){ + expect.assertions(6) let html = `
` + let liveSocket = new LiveSocket("/live", Socket) + let el = liveViewDOM(html) + let view = simulateJoinedView(el, liveSocket, html) + let channelStub = { + validate: "", + nextValidate(payload){ + this.validate = Object.entries(payload) + .map(([key, value]) => `${encodeURIComponent(key)}=${value ? encodeURIComponent(value) : ""}`) + .join("&") + }, + push(_evt, payload, _timeout){ + expect(payload.value).toBe(this.validate) + return { + receive(status, cb){ + if(status === "ok"){ + let diff = { + s: [` + + `], + fingerprint: 345 + } + cb({diff: diff}) + return this + } else { + return this + } + } + } + } + } + view.channel = channelStub + + let first_name = view.el.querySelector("#first_name") + let last_name = view.el.querySelector("#last_name") + let email = view.el.querySelector("#email") + view.channel.nextValidate({"user[first_name]": null, "user[last_name]": null, "user[email]": null, "_target": "user[email]"}) + // we have to set this manually since it's set by a change event that would require more plumbing with the liveSocket in the test to hook up + DOM.putPrivate(email, "phx-has-focused", true) + view.pushInput(email, el, null, "validate", {_target: email.name}) + window.requestAnimationFrame(() => { + expect(el.querySelector(`[phx-feedback-for="${email.name}"]`).classList.contains("phx-no-feedback")).toBeFalsy() + expect(el.querySelector(`[phx-feedback-for="${first_name.name}"]`).classList.contains("phx-no-feedback")).toBeTruthy() + expect(el.querySelector(`[phx-feedback-for="${last_name.name}"]`).classList.contains("phx-no-feedback")).toBeTruthy() + expect(el.querySelector("[phx-feedback-for=\"mygroup\"]").classList.contains("phx-no-feedback")).toBeTruthy() + + view.channel.nextValidate({"user[first_name]": null, "user[last_name]": null, "user[email]": null, "_target": "user[first_name]"}) + DOM.putPrivate(first_name, "phx-has-focused", true) + view.pushInput(first_name, el, null, "validate", {_target: first_name.name}) + window.requestAnimationFrame(() => { + expect(el.querySelector(`[phx-feedback-for="${first_name.name}"`).classList.contains("phx-no-feedback")).toBeFalsy() + expect(el.querySelector(`[phx-feedback-for="${last_name.name}"`).classList.contains("phx-no-feedback")).toBeTruthy() + // first_name was focused, the feedback-group should not have the phx-no-feedback class + expect(el.querySelector("[phx-feedback-for=\"mygroup\"]").classList.contains("phx-no-feedback")).toBeFalsy() + done() + }) + }) + }) + + test("pushInput sets phx-no-feedback class on feedback elements for multiple select", function(done){ + // a multiple select name attribute contains trailing square brackets [] to capture multiple options + let multiple_select_name = "user[allergies][]" + // the phx-feedback-for attribute typically doesn't contain trailing brackets + // because its value is often set with the result of Phoenix.HTML.input_name/2 + let multiple_select_phx_feedback_for_name = "user[allergies]" + let html = + `` + let liveSocket = new LiveSocket("/live", Socket) + let el = liveViewDOM(html) + let view = simulateJoinedView(el, liveSocket, html) + let channelStub = { + push(_evt, _payload, _timeout){ + return { + receive(_status, cb){ + let diff = { + s: [` + + `], + fingerprint: 345 + } + cb({diff}) + return this + } + } + } + } + view.channel = channelStub + + let first_name_input = view.el.querySelector("input#first_name") + let allergies_select = view.el.querySelector("select#allergies") + // we have to set this manually since it's set by a change event that would require more plumbing with the liveSocket in the test to hook up + DOM.putPrivate(first_name_input, "phx-has-focused", true) + view.pushInput(first_name_input, el, null, "validate", {_target: "user[first_name]"}) + window.requestAnimationFrame(() => { + expect(el.querySelector(`span[phx-feedback-for="user[first_name]"`).classList.contains("phx-no-feedback")).toBeFalsy() + expect(el.querySelector(`span[phx-feedback-for="user[allergies]"`).classList.contains("phx-no-feedback")).toBeTruthy() + + DOM.putPrivate(allergies_select, "phx-has-focused", true) + view.pushInput(allergies_select, el, null, "validate", {_target: "user[allergies][]"}) + window.requestAnimationFrame(() => { + expect(el.querySelector(`span[phx-feedback-for="user[first_name]"`).classList.contains("phx-no-feedback")).toBeFalsy() + expect(el.querySelector(`span[phx-feedback-for="user[allergies]"`).classList.contains("phx-no-feedback")).toBeFalsy() done() }) }) @@ -1281,4 +1438,4 @@ describe("DOM", function(){ expect(target.checked).toEqual(true) expect(target.id).toEqual("bar") }) -}) +}) \ No newline at end of file diff --git a/lib/phoenix_component.ex b/lib/phoenix_component.ex index 31e94c08fd..b2a5af5d21 100644 --- a/lib/phoenix_component.ex +++ b/lib/phoenix_component.ex @@ -1455,12 +1455,6 @@ defmodule Phoenix.Component do * `:as` - the `name` prefix to be used in form inputs * `:id` - the `id` prefix to be used in form inputs * `:errors` - keyword list of errors (used by maps exclusively) - * `:action` - The action that was taken against the form. This value can be - used to distinguish between different operations such as the user typing - into a form for validation, or submitting a form for a database insert. - For example: `to_form(changeset, action: :validate)`, - or `to_form(changeset, action: :save)`. The provided action is passed - to the underlying `Phoenix.HTML.FormData` implementation options. The underlying data may accept additional options when converted to forms. For example, a map accepts `:errors` @@ -1476,8 +1470,7 @@ defmodule Phoenix.Component do form options. Errors in a form are only displayed if the changeset's `action` - field is set (and it is not set to `:ignore`) and can be filtered - by whether the fields have been used on the client or not. Refer to + field is set (and it is not set to `:ignore`). Refer to [a note on :errors for more information](#form/1-a-note-on-errors). """ def to_form(data_or_params, options \\ []) @@ -1498,17 +1491,9 @@ defmodule Phoenix.Component do {_as, options} = Keyword.pop(options, :as) {errors, options} = Keyword.pop(options, :errors, data.errors) - {action, options} = Keyword.pop(options, :action) options = Keyword.merge(data.options, options) - %Phoenix.HTML.Form{ - data - | action: action, - errors: errors, - id: id, - name: name, - options: options - } + %{data | errors: errors, id: id, name: name, options: options} end def to_form(data, options) do @@ -1536,62 +1521,6 @@ defmodule Phoenix.Component do Phoenix.HTML.FormData.to_form(data, options) end - @doc """ - Returns the errors for the form field if the field was used by the client. - - Used inputs are only those inputs that have been focused, interacted with, or - submitted by the client. For LiveView, this is used to filter errors from the - `Phoenix.HTML.FormData` implementation to avoid showing "field can't be blank" - in scenarios where the client hasn't yet interacted with specific fields. - - Used inputs are tracked internally by the client sending a sibling key - derived from each input name, which indicates the inputs that remain unused - on the client. For example, a form with email and title fields where only the - title has been modifed so far on the client, would send the following payload: - - %{ - "title" => "new title", - "email" => "", - "_unused_email" => "" - } - - The `_unused_email` key indicates that the email field has not been used by the - client, which is used to filter errors from the UI. - - ## Examples - - For example, imagine in your template you render a title and email input. - On initial load the end-user begins typing the title field. The client will send - the entire form payload to the server with the typed title and an empty email. - - The `Phoenix.HTML.FormData` implementation will consider an empty email in - this scenario as invalid, but the user shouldn't see the error because they - haven't yet used the email input. To handle this, `used_input?/1` can be used to - filter errors from the client by referencing param metadata to distinguish between - used and unused input fields. For non-LiveViews, all inputs are considered used. - - ```heex - - -<%= error %>
-<%= error %>
-