wavesoft / dot-dom

.dom is a tiny (512 byte) template engine that uses virtual DOM and some of react principles
Apache License 2.0
806 stars 53 forks source link

Trouble with onkeyup #10

Closed maxidr closed 7 years ago

maxidr commented 7 years ago

Hi!, I'm started to playing with the library and I found a strange behaviour when I try to use the onkeyup event.

const {div, p, input, label} = H;

const Input = ({ value, onchange }) => input({ 
    type: 'text', value: value || '', 
    onkeyup: e => onchange(e.target.value)
})

const html = (props, state, setState) => div(
  label('Enter your name:'),
  H(Input, { value: state.name, onchange: (value) => setState({ name: value }) }),
  p.label(state.name ? `Hello ${state.name}` : '')
)

R(H(html), document.body)

Working code: https://jsfiddle.net/utrfb1cq/2/

The example works well, but when I'm focused into the input and press left arrow in the keyboard (or try to select the typed word without using the mouse) the cursor always go back to the end of the text. I suppose that have any relation with the redraw of the virtual node, but I not sure.

wavesoft commented 7 years ago

Hey @maxidr and thank you for your feedback!

Unfortunately, it looks that by assigning the new value property the selection range changes. This means that you should carry it along, like so:

const Input = ({ value, selectionStart, selectionEnd, onchange }) => input({ 
    type: 'text', value: value || '', 
  selectionStart: selectionStart || 0,
  selectionEnd: selectionEnd || 0,
    onkeyup: e => onchange(e.target.value, e.target.selectionStart, e.target.selectionEnd)
})

const html = (props, state, setState) => div(
    label('Enter your name:'),
  H(Input, { value: state.name, selectionStart: state.selectionStart, selectionEnd: state.selectionEnd, onchange: (value, selectionStart, selectionEnd) => setState({ name: value, selectionStart: selectionStart, selectionEnd: selectionEnd }) }),
  p.label(state.name ? `Hello ${state.name}` : '')
)

But this is kind of counter-intuitive and that definitely needs to be part of the core. I would try my best to find a solution to fix this problem while keeping the 512 byte limitation, but any contribution or brainstorming is more than welcome 😄

wavesoft commented 7 years ago

The solution was actually simpler than I expected. It should be fixed with the #13 pr

wavesoft commented 7 years ago

Merged! Expect it fixed on the 0.2.2 release 👍