Skip to content

Comments

Single source of truth#76

Merged
hopsoft merged 8 commits intostimulusreflex:masterfrom
leastbad:truth
Oct 28, 2019
Merged

Single source of truth#76
hopsoft merged 8 commits intostimulusreflex:masterfrom
leastbad:truth

Conversation

@leastbad
Copy link
Contributor

@leastbad leastbad commented Oct 19, 2019

Type of PR (feature, enhancement, bug fix, etc.)

Enhancement

Description

Proposed fix for the problem of multiple users clobbering each other by typing into the same input at the same time. Concept is that if a textual input element has focus, morphdom should never overwrite the input's current value even when it deviates from the server's rendered updated.

Why should this be added

While not a complete solve for multi-user concurrency, this imposes some sanity on an otherwise imperfect status quo.

Checklist

  • My code follows the style guidelines of this project
  • Checks (StandardRB & Prettier-Standard) are passing

@hopsoft
Copy link
Contributor

hopsoft commented Oct 19, 2019 •
edited
Loading

See #77 as it's related. There was a bug that assumed that connection identifiers are always Rails model instances or at least an object with an id. This prevented securing the web socket broadcasts as documented here: https://docs.stimulusreflex.com/security#encrypted-session-cookies

Note that version 2.1.4 fixes this bug.

@leastbad
Copy link
Contributor Author

leastbad commented Oct 19, 2019

Great news that #77 was patched, although technically this PR was coming regardless. The server should not be able to overwrite the contents of an input element while you're typing into it.

hopsoft
hopsoft previously approved these changes Oct 20, 2019
View reviewed changes
hopsoft
hopsoft reviewed Oct 21, 2019
View reviewed changes
@hopsoft hopsoft dismissed their stale review October 21, 2019 14:06

Rethinking this

@hopsoft
Copy link
Contributor

hopsoft commented Oct 21, 2019

I'm currently building a chat demo on the expo project and am seeing a usecase where we actually do want the input clobbered when the reflex updates the DOM. 🤔

@leastbad
Copy link
Contributor Author

leastbad commented Oct 21, 2019

https://hexdocs.pm/phoenix_live_view/Phoenix.LiveView.html#module-custom-dom-patching ???

@leastbad
Copy link
Contributor Author

leastbad commented Oct 24, 2019

Just wanted to check in on this and see where your head is at. I'm happy to make the changes you suggested, but I wanted to make sure that we're still on the same page about it being a good idea.

@hopsoft
Copy link
Contributor

hopsoft commented Oct 24, 2019

I still think this is a good idea. If someone want's to take action on one of these input fields (as my chat example above) they can still do so via a lifecycle method.

@hopsoft hopsoft mentioned this pull request Oct 27, 2019
Merged
leastbad
leastbad commented Oct 28, 2019
View reviewed changes
@hopsoft hopsoft merged commit 9da48d9 into stimulusreflex:master Oct 28, 2019
@leastbad
Copy link
Contributor Author

leastbad commented Oct 28, 2019

Oh, awesome! Seemed like you were on the fence.

@leastbad leastbad deleted the truth branch October 28, 2019 18:11
@andrewmcodes andrewmcodes added the enhancement New feature or request label Apr 11, 2020
@leastbad leastbad mentioned this pull request Apr 19, 2020
Closed
@julianrubisch julianrubisch mentioned this pull request Sep 8, 2021
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hopsoft hopsoft hopsoft approved these changes

Assignees

No one assigned

Labels

enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@leastbad @hopsoft @andrewmcodes