Skip to content

Commit

Permalink
Consider all form inputs for feedback reset if any one is updated (#3072
Browse files Browse the repository at this point in the history
)

Partial fix for #3071
  • Loading branch information
chrismccord committed Feb 1, 2024
1 parent 7d19dea commit 345ffac
Show file tree
Hide file tree
Showing 8 changed files with 236 additions and 156 deletions.
26 changes: 14 additions & 12 deletions assets/js/phoenix_live_view/dom.js
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ let DOM = {
}
},

maybeHideFeedback(container, inputs, phxFeedbackFor, phxFeedbackGroup){
maybeHideFeedback(container, forms, phxFeedbackFor, phxFeedbackGroup){
let feedbacks = []
// if there are multiple inputs with the same name
// (for example the default checkbox renders a hidden input as well)
Expand All @@ -301,17 +301,19 @@ let DOM = {
// an entry in this object will be true if NO input in the group has been focused yet
let feedbackGroups = {}

inputs.forEach(input => {
const group = input.getAttribute(phxFeedbackGroup)
// initialize the group to true if it doesn't exist
if(group && !(group in feedbackGroups)) feedbackGroups[group] = true
// initialize the focused state to false if it doesn't exist
if(!(input.name in inputNamesFocused)) inputNamesFocused[input.name] = false
if(this.private(input, PHX_HAS_FOCUSED) || this.private(input, PHX_HAS_SUBMITTED)){
inputNamesFocused[input.name] = true
// the input was focused, therefore the group will NOT get phx-no-feedback
if(group) feedbackGroups[group] = false
}
forms.forEach(form => {
Array.from(form.elements).forEach(input => {
const group = input.getAttribute(phxFeedbackGroup)
// initialize the group to true if it doesn't exist
if(group && !(group in feedbackGroups)){ feedbackGroups[group] = true }
// initialize the focused state to false if it doesn't exist
if(!(input.name in inputNamesFocused)){ inputNamesFocused[input.name] = false }
if(this.private(input, PHX_HAS_FOCUSED) || this.private(input, PHX_HAS_SUBMITTED)){
inputNamesFocused[input.name] = true
// the input was focused, therefore the group will NOT get phx-no-feedback
if(group){ feedbackGroups[group] = false }
}
})
})

for(const [name, focused] of Object.entries(inputNamesFocused)){
Expand Down
14 changes: 7 additions & 7 deletions assets/js/phoenix_live_view/dom_patch.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ export default class DOMPatch {
let phxViewportBottom = liveSocket.binding(PHX_VIEWPORT_BOTTOM)
let phxTriggerExternal = liveSocket.binding(PHX_TRIGGER_ACTION)
let added = []
let trackedInputs = []
let trackedForms = new Set()
let updates = []
let appendPrependUpdates = []

Expand Down Expand Up @@ -188,7 +188,7 @@ export default class DOMPatch {
}

if(el.getAttribute && el.getAttribute("name") && DOM.isFormInput(el)){
trackedInputs.push(el)
trackedForms.add(el.form)
}
// nested view handling
if((DOM.isPhxChild(el) && view.ownsElement(el)) || DOM.isPhxSticky(el) && view.ownsElement(el.parentNode)){
Expand Down Expand Up @@ -263,7 +263,7 @@ export default class DOMPatch {
DOM.syncAttrsToProps(fromEl)
updates.push(fromEl)
DOM.applyStickyOperations(fromEl)
trackedInputs.push(fromEl)
trackedForms.add(fromEl.form)
return false
} else {
// blur focused select if it changed so native UI is updated (ie safari won't update visible options)
Expand All @@ -275,7 +275,7 @@ export default class DOMPatch {
DOM.syncAttrsToProps(toEl)
DOM.applyStickyOperations(toEl)
if(toEl.getAttribute("name") && DOM.isFormInput(toEl)){
trackedInputs.push(toEl)
trackedForms.add(toEl.form)

This comment has been minimized.

Copy link
@eldano

eldano Feb 2, 2024

Contributor

I'm having an issue and it looks like it's related to this line.
toEl.form is null in my case. Not sure exactly how to provide a minimal example to reproduce.
Is toEl part of the form at this point? Shouldn't it be fromEl?

This comment has been minimized.

Copy link
@SteffenDE

SteffenDE Feb 2, 2024

Collaborator

Should be fixed now

}
this.trackBefore("updated", fromEl, toEl)
return true
Expand All @@ -292,7 +292,7 @@ export default class DOMPatch {
})
}

DOM.maybeHideFeedback(targetContainer, trackedInputs, phxFeedbackFor, phxFeedbackGroup)
DOM.maybeHideFeedback(targetContainer, trackedForms, phxFeedbackFor, phxFeedbackGroup)

liveSocket.silenceEvents(() => DOM.restoreFocus(focused, selectionStart, selectionEnd))
DOM.dispatchEvent(document, "phx:update")
Expand Down Expand Up @@ -351,10 +351,10 @@ export default class DOMPatch {
maybeReOrderStream(el, isNew){
let {ref, streamAt, reset} = this.getStreamInsert(el)
if(streamAt === undefined){ return }

// we need to set the PHX_STREAM_REF here as well as addChild is invoked only for parents
this.setStreamRef(el, ref)

if(!reset && !isNew){
// we only reorder if the element is new or it's a stream reset
return
Expand Down
112 changes: 69 additions & 43 deletions priv/static/phoenix_live_view.cjs.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions priv/static/phoenix_live_view.cjs.js.map

Large diffs are not rendered by default.

Loading

0 comments on commit 345ffac

Please sign in to comment.