Skip to content

Commit

Permalink
Introduce tracking of used and unused inputs with error message filte…
Browse files Browse the repository at this point in the history
…ring (phoenixframework#3090)

Co-authored-by: José Valim <[email protected]>
  • Loading branch information
chrismccord and josevalim authored May 3, 2024
1 parent 8b40375 commit d912468
Show file tree
Hide file tree
Showing 19 changed files with 224 additions and 14,977 deletions.
2 changes: 0 additions & 2 deletions assets/js/phoenix_live_view/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,6 @@ 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"]
Expand Down
11 changes: 6 additions & 5 deletions assets/js/phoenix_live_view/dom.js
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,11 @@ 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}[]"],
Expand All @@ -362,14 +367,10 @@ let DOM = {
return query
},

resetForm(form, phxFeedbackFor, phxFeedbackGroup){
resetForm(form){
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], [])
})
})
},

Expand Down
18 changes: 2 additions & 16 deletions assets/js/phoenix_live_view/dom_patch.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
import {
PHX_COMPONENT,
PHX_DISABLE_WITH,
PHX_FEEDBACK_FOR,
PHX_FEEDBACK_GROUP,
PHX_PRUNE,
PHX_ROOT_ID,
PHX_SESSION,
Expand Down Expand Up @@ -53,6 +51,7 @@ 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: [],
Expand All @@ -79,21 +78,17 @@ export default class DOMPatch {
}

perform(isJoinPatch){
let {view, liveSocket, container, html} = this
let targetContainer = this.isCIDPatch() ? this.targetCIDContainer(html) : container
let {view, liveSocket, html, container, targetContainer} = this
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 = []

Expand Down Expand Up @@ -148,7 +143,6 @@ 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){
Expand Down Expand Up @@ -187,12 +181,6 @@ 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
Expand Down Expand Up @@ -301,8 +289,6 @@ 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))
Expand Down
5 changes: 3 additions & 2 deletions assets/js/phoenix_live_view/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ See the hexdocs at `https://hexdocs.pm/phoenix_live_view` for documentation.
*/

import LiveSocket from "./live_socket"
import LiveSocket, {isUsedInput} from "./live_socket"
export {
LiveSocket
LiveSocket,
isUsedInput
}
15 changes: 11 additions & 4 deletions assets/js/phoenix_live_view/live_socket.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,6 @@ import {
PHX_THROTTLE,
PHX_TRACK_UPLOADS,
PHX_SESSION,
PHX_FEEDBACK_FOR,
PHX_FEEDBACK_GROUP,
RELOAD_JITTER_MIN,
RELOAD_JITTER_MAX,
PHX_REF,
Expand All @@ -117,6 +115,8 @@ 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
Expand Down Expand Up @@ -158,7 +158,12 @@ export default class LiveSocket {
this.localStorage = opts.localStorage || window.localStorage
this.sessionStorage = opts.sessionStorage || window.sessionStorage
this.boundTopLevelEvents = false
this.domCallbacks = Object.assign({onNodeAdded: closure(), onBeforeElUpdated: closure()}, opts.dom || {})
this.domCallbacks = Object.assign({
onPatchStart: closure(),
onPatchEnd: closure(),
onNodeAdded: closure(),
onBeforeElUpdated: closure()},
opts.dom || {})
this.transitions = new TransitionSet()
window.addEventListener("pagehide", _e => {
this.unloaded = true
Expand All @@ -173,6 +178,8 @@ export default class LiveSocket {

// public

isUsedInput(el){ return isUsedInput(el) }

version(){ return LV_VSN }

isProfileEnabled(){ return this.sessionStorage.getItem(PHX_LV_PROFILE) === "true" }
Expand Down Expand Up @@ -882,7 +889,7 @@ export default class LiveSocket {
}
this.on("reset", (e) => {
let form = e.target
DOM.resetForm(form, this.binding(PHX_FEEDBACK_FOR), this.binding(PHX_FEEDBACK_GROUP))
DOM.resetForm(form)
let input = Array.from(form.elements).find(el => el.type === "reset")
if(input){
// wait until next tick to get updated input value
Expand Down
27 changes: 21 additions & 6 deletions assets/js/phoenix_live_view/view.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@ 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,
Expand Down Expand Up @@ -57,6 +55,17 @@ 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(/(\w+)(\]?$)/, `${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

Expand Down Expand Up @@ -90,8 +99,14 @@ 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 input = elements.find(input => input.name === key)
let isUnused = !(DOM.private(input, PHX_HAS_FOCUSED) || DOM.private(input, PHX_HAS_SUBMITTED))
if(isUnused && !(submitter && submitter.name == key)){
params.append(prependFormDataKey(key, "_unused_"), "")
}
params.append(key, val)
}
}
Expand Down Expand Up @@ -432,6 +447,8 @@ 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)
Expand Down Expand Up @@ -466,6 +483,8 @@ export default class View {
patch.perform(isJoinPatch)
this.afterElementsRemoved(removedEls, pruneCids)


this.liveSocket.triggerDOM("onPatchEnd", [patch.targetContainer])
return phxChildrenAdded
}

Expand Down Expand Up @@ -967,7 +986,6 @@ 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()
Expand Down Expand Up @@ -1272,13 +1290,10 @@ 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()
})
}
Expand Down
8 changes: 4 additions & 4 deletions assets/test/js_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -418,7 +418,7 @@ describe("JS", () => {
event: "validate",
type: "form",
uploads: {},
value: "username=&other=&_target=username"
value: "_unused_username=&username=&_unused_other=&other=&_target=username"
})
done()
}
Expand Down Expand Up @@ -449,7 +449,7 @@ describe("JS", () => {
event: "username_changed",
type: "form",
uploads: {},
value: "username=&_target=username"
value: "_unused_username=&username=&_target=username"
})
done()
}
Expand Down Expand Up @@ -480,7 +480,7 @@ describe("JS", () => {
event: "username_changed",
type: "form",
uploads: {},
value: "username=&_target=username"
value: "_unused_username=&username=&_target=username"
})
done()
}
Expand Down
Loading

0 comments on commit d912468

Please sign in to comment.