MithrilJS / mithril.js

A JavaScript Framework for Building Brilliant Applications
https://mithril.js.org
MIT License
14.01k stars 926 forks source link

Implement some sort of React's restoreSelection() #782

Closed avesus closed 8 years ago

avesus commented 9 years ago

React uses restoreSelection() method to restore selection when complex reordering makes happen. https://github.com/facebook/react/blob/master/src/renderers/dom/client/ReactInputSelection.js

Mithril doesn't, which leads to focus and selection lost when reordering happens, i.e.

Step 1:

<textarea id="1"/>
<textarea id="2"/>

Focus in 1, some text selected.

Step 2: change sort order

<textarea id="2"/>
<textarea id="1"/>

Focus in 1, some text selected. All works as expected.

Step 3: change sort order again

<textarea id="1"/>
<textarea id="2"/>

Focus lost, selection lost.

renancouto commented 9 years ago

Are you using the key attribute like this?

avesus commented 9 years ago

Of course.

avesus commented 9 years ago

Focus lost in the last element in list, because it actually destroyed, and a new replacing one is created before the former first element.

dead-claudia commented 9 years ago

This sounds more like a bug than a feature request. I don't know much about Mithril's diff algorithm, though, but that's where the bug most likely is. The diff should still respect the key name, especially if it's in the same relative section. Is the component itself being replaced at this point? You can use this as an attribute to check for that:

// Your return `view` for the container holding the list
m("div", {
  // props...
  config: function (elem, isInit, context) {
    context.onunload = function () {
      console.log("Element unloaded!")
    }
  },
}, body)
avesus commented 9 years ago

:-) overheated trying force component to use the config attribute. Too hot topic too (maybe it is useful to merge attributes passed to a component with components internal "container" attributes (with <div>, <ul> etc.)?.. Wait, I'll try the component recreation check in 10 minutes.

avesus commented 9 years ago

It actually calls config() over and over instead of reusing the existing one. context.unload is never called. One important thing to mention: I use diff strategy when changing routes. The case decribed above is very artifical one, sorry. In my case "resorting" occurs when changing routes :-)

avesus commented 9 years ago

I have tested it again. Selection and focus lost. Ever with context.retain = true. Of course, config() called over and over on re-render - it is by design. First call on first element creation. I think that there is a bug: element actually destroyed, but config callback not called.

avesus commented 9 years ago

@impinball The component is NOT replaced. I hope on the next week to prepare a syntetic example to test, maybe creating a PR.

rafalp commented 9 years ago

FYI, this issue is also making implementation of fields validated as user enters value impossible, as input results in adding CSS class to input's container (eg. .has-error in case of bootstrap) removing focus from input.

Nope, in my case it was adding aria-describedby="feedbackId" to input that caused focus loss.

lhorie commented 8 years ago

document.activeElement is set if required after diff in rewrite branch