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

selected attribute is not updated properly #72

Closed
cideM opened this issue Jun 26, 2017 · 2 comments
Closed

selected attribute is not updated properly #72

cideM opened this issue Jun 26, 2017 · 2 comments

Comments

@cideM
Copy link

cideM commented Jun 26, 2017

The repo where this bug happens is https://github.com/cideM/tinyfinancial on the details branch (https://github.com/cideM/tinyfinancial/blob/details/src/client/views/Details/index.js). But it requires MongoDB and one would have to remove the facebook authentication. But maybe I'll be able to explain my issue through code examples as well.

I have a controlled <select> and onchange a choo state variable is updated, the view function is rerendered and the newly selected <option> should now be selected. The actual behavior is very confusing though:

When the function renders the first time the DOM looks like this

<select value="2017">
   <option value="2017" selected="selected">2017</option>
   <option value="2016">2016</option>
</select>

If the user select 2016, the DOM looks like this

<select value="2016">
  <option value="2017" selected="">2017</option>
  <option value="2016" selected="selected">2016</option>
</select>

Note the leftover selected="".

Now here are a few console.log statements that I put in updateAttribute of morphdom

function updateAttribute (newNode, oldNode, name) {
  console.log(newNode)
  console.log(newNode.selected)
  console.log(oldNode)
  console.log(oldNode.selected)

When the user chooses 2016, the log statements evaluate to this:
<option value="2017">2017</option>
true why? :[
<option value="2017" selected="">2017</option>
false Seems okay I guess

and
<option value="2016" selected="selected">2016</option>
false O_o
<option value="2016" selected="selected">2016</option>
false Ó_ò

It's late and it's been confusing, so maybe this is totally expected, but I don't it. The booleans seem off though. My intuition says something is mutating the nodes in between other actions but that doesn't sound right. And that getAttribute and properties are not always the same is known but that it would cause such a bug also sounds odd. Lastly, maybe I am just making a grave mistake here somewhere?

I was actually able to fix this by changing updateAttributes to

function updateAttribute (newNode, oldNode, name) {
  if (newNode.getAttribute(name) !== oldNode.getAttribute(name)) {
    oldNode.setAttribute(name, newNode.getAttribute(name))
    if (newNode.getAttribute(name)) {
      oldNode.setAttribute(name, '')
    } else {
      oldNode.removeAttribute(name, '')
    }
  }
}

Note that I replaced the calls to node.selected with node.getAttribute('selected'). I didn't fork and run tests and I didn't manually check anything else because I really need to sleep now. But please let me know
a) if my bug is known and there's a workaround I couldn't find
b) my "discoveries" are expected behavior and I just don't it

@soyuka
Copy link

soyuka commented Jun 27, 2017

Should have been fixed by #60

@cideM
Copy link
Author

cideM commented Jun 27, 2017

Memo to myself: check pull requests not just issues. Sorry for the redundant post! It did indeed fix my problem.

@cideM cideM closed this as completed Jun 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants