Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce tracking of used and unused inputs with error message filtering #3090

Merged
merged 11 commits into from
May 3, 2024
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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 @@ -144,7 +139,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 @@ -183,12 +177,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 @@ -297,8 +285,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) }

isProfileEnabled(){ return this.sessionStorage.getItem(PHX_LV_PROFILE) === "true" }

isDebugEnabled(){ return this.sessionStorage.getItem(PHX_LV_DEBUG) === "true" }
Expand Down Expand Up @@ -880,7 +887,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 @@ -416,6 +431,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 @@ -450,6 +467,8 @@ export default class View {
patch.perform(isJoinPatch)
this.afterElementsRemoved(removedEls, pruneCids)


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

Expand Down Expand Up @@ -951,7 +970,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 @@ -1256,13 +1274,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 @@ -332,7 +332,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 @@ -398,7 +398,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 @@ -429,7 +429,7 @@ describe("JS", () => {
event: "username_changed",
type: "form",
uploads: {},
value: "username=&_target=username"
value: "_unused_username=&username=&_target=username"
})
done()
}
Expand Down Expand Up @@ -460,7 +460,7 @@ describe("JS", () => {
event: "username_changed",
type: "form",
uploads: {},
value: "username=&_target=username"
value: "_unused_username=&username=&_target=username"
})
done()
}
Expand Down