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.

Sign up for GitHub

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

Jump to bottom

Fix input type=text selection reset. #224

Merged
merged 3 commits into from
Jun 4, 2017
Merged

Fix input type=text selection reset. #224

merged 3 commits into from
Jun 4, 2017

Conversation

jorgebucaran
Copy link
Owner

@jorgebucaran jorgebucaran commented Jun 3, 2017 •
edited

ᕕ( ᐛ )ᕗ 🎉

Related #137.
Ping @mrkstwrt.

@codecov
Copy link

codecov bot commented Jun 3, 2017 •
edited

Codecov Report

Merging #224 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #224   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           4      4           
  Lines         163    163           
  Branches       43     44    +1     
=====================================
  Hits          163    163
Impacted Files Coverage Δ
src/app.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 71ce88b...2c527fe. Read the comment docs.

FlorianWendelborn
FlorianWendelborn reviewed Jun 3, 2017
View reviewed changes
src/app.js
@@ -145,11 +145,13 @@ export default function(app) {
function updateElementData(element, oldData, data) {
for (var name in merge(oldData, data)) {
var value = data[name]
var oldValue = oldData[name]
var oldValue = name === "value" || name === "checked"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why check for checked and value? Any explanation?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are the two "dynamic" attributes of a component whose value can change when the user interacts with the component. So, when diffing an element's data, we want to look at the real DOM (not the stuff in oldData).

I hope that makes sense.

@jorgebucaran jorgebucaran merged commit 4f6daae into jorgebucaran:master Jun 4, 2017
@zaceno
Copy link
Contributor

zaceno commented Jun 17, 2017

@jbucaran -- will this fix be coming to picodom as well (or is it not necessary?)

@jorgebucaran
Copy link
Owner Author

@zaceno Yeah, already fixed in picodom. jorgebucaran/superfine#6

jorgebucaran added a commit that referenced this pull request Jan 7, 2018
@jorgebucaran
Merge pull request #224 from jbucaran/master
57e4f6c 
Fix input type=text selection reset.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FlorianWendelborn FlorianWendelborn FlorianWendelborn left review comments

@ngryman ngryman ngryman approved these changes

@zaceno zaceno Awaiting requested review from zaceno

@lukejacksonn lukejacksonn Awaiting requested review from lukejacksonn

@selfup selfup Awaiting requested review from selfup

@benjamminj benjamminj Awaiting requested review from benjamminj

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@jorgebucaran @zaceno @ngryman @FlorianWendelborn @jojibucaran