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

Input looses focus #10

Closed
peletiah opened this issue Jul 3, 2016 · 13 comments
Closed

Input looses focus #10

peletiah opened this issue Jul 3, 2016 · 13 comments

Comments

@peletiah
Copy link

peletiah commented Jul 3, 2016

Found your library via Stackoverflow, it's very useful for my current project, so thanks for releasing it as Open Source Software!

I've copied your userlist-example over to my build-env and it works, mostly. There's just one issue - I loose focus after every letter entered in the input-fields.

I thought the problem might be caused by the modal-overlay - but after implementing nestedLink-methods in my own project, I have the same issue with loosing focus in the input-fields. And I don't use a modal.
After every letter I enter, the cursor is gone and I have to click in the input-field again to enter the next letter.

The issue is probably caused by the linkToObject.at-method, which I'm using to autosubmit the input-value in conjunction with onChange, in the first input-field in this example:

render() {
    const linked = Link.all ( this, 'sequence', 'command', 'info' ),
          infoLink   = this.props.sortData.at( 'info')

    const setValue = (x, e) => e.target.value

    return (
      <div { ...this.props }>
        <span className="sequence-order">{linked.sequence.value}</span>
          <input value={ linked.info.value } onChange={ infoLink.action( setValue ) }/>
          <input valueLink={ linked.info }/>
      </div>
    );
  }

The issue doesn't appear in the second-input-field, where I'm using a "simple" valueLink. But since I don't want to add a submit-button to commit its value to the parent, I prefer the more elaborate mechanism with onChange.

I'm on:

  • react@15.1.0
  • react-dom@15.1.0
@gaperton
Copy link

gaperton commented Jul 5, 2016

Really weird. Let me clarify, please.

This one

<input value={ linked.info.value } onChange={ infoLink.action( setValue ) }/>

Is the one you lose focus on? Or the second one?

It looks suspicious, btw. You're taking the value from one place, but writing user's input to different place. Shouldn't it look like this?

const jointLink = Link.value( linked.info.value, x => {
    linked.info.set( x );
    infoLink.set( x );
});

...

<Input valueLink={ jointLink } />

Here we construct ad-hoc link wrapper around linked.info, which keeps infoLink in sync when set with a value.

@gaperton
Copy link

gaperton commented Jul 5, 2016

Does this Link.value help? I can add link.onChange( ) hooks to make this stuff simpler. So, it will look just like:

linked.info.onChange( x => infoLink.set( x ) );

@gaperton
Copy link

gaperton commented Jul 5, 2016

Nice idea, will do it anyway. Just let me know if it helps for your case.

@gaperton
Copy link

gaperton commented Jul 5, 2016

Another note. Standard <input> will not support valueLink prop in the next React version, and sometimes it behaves weird.

Try to use Input component from tags.jsx instead, and check if it will help.

import { Input } from 'valuelink/tags'

tags.jsx is also the perfect place for your custom form controls styling, if you copy it over to your project.

@peletiah
Copy link
Author

peletiah commented Jul 5, 2016

Ah, the jointLink-example is a lot simpler! But it does not fix the focus-issue. But I investigated a bit more in the meantime and I guess the problem are changing keys on every re-render. I'm using this in combination with react-anything-sortable.

linkToObject.map() breaks the behaviour of my Sortable-component and I need to add a dynamic key to it to get Items to be sortable again. This dynamic key is probably the cause for the focus-issue. I'll post some example-code in my next comment, just want to get out this information while I have your attention.

Thanks for pointing out the upper-case Input, I didn't see the difference between the examples and my code and was wondering why I'd still get the depreciation-warning :-)

gaperton pushed a commit that referenced this issue Jul 5, 2016
@gaperton
Copy link

gaperton commented Jul 5, 2016

Okay. npm update, and now you can do just linked.info.onChange( x => infoLink.set( x ) ).

@gaperton
Copy link

gaperton commented Jul 5, 2016

Yes, changing keys in the lists will cause React to recreate inputs. To fix that you probably need to add some permanent ids to you objects which you're sorting when you create objects, if you don't have them yet.

{
    _id : _.uniqueId( 'x' ),
   ...
}

or just

let _count = 0;
...
{
    _id : _count++
    ...
}

Then, use this item._id as key.

@gaperton
Copy link

gaperton commented Jul 5, 2016

Example would be good, since problem seems to be tricky.

@peletiah
Copy link
Author

peletiah commented Jul 5, 2016

Give me a few more minutes...

@gaperton
Copy link

gaperton commented Jul 5, 2016

Okay. Meanwhile, the docs for new link.onChange and link.pipe. I changed API a bit, as previous implementation introduced memory leak due to the fact that links are cached, and loosing validation error.

So, that's final API (1.3.10). Seems to works fine.

const jointLink = linked.info.onChange( x => infoLink.set( x ) )

https://github.com/Volicon/NestedLink/#-linkonchange-callback--any--void---link

@peletiah
Copy link
Author

peletiah commented Jul 5, 2016

Ok, it took me a while to reproduce the error with a simplified example, here's the example-code where the sorting works but the focus issue is present (Due to an ever-changing key in <Sortable>):

import React from 'react';
import Sortable from 'react-anything-sortable';
import { sortable } from 'react-anything-sortable';
import Link from 'valuelink'
import { Input, TextArea, Select, Radio, Checkbox } from 'valuelink/tags'
import shortid from 'shortid'

@sortable
class DemoHOCItem extends React.Component {

  constructor() {
      super();
      this.state = {
        info : ''
      };
    }

 componentWillMount() {
    console.log(this.props.sortData.value)
    this.setState( this.props.sortData.value );
  }


  render() {
    const linked = Link.all( this, 'sequence', 'info', 'other' ),
      infoLink   = this.props.sortData.at( 'info')

    const jointLink = Link.value( linked.info.value, x => {
      linked.info.set( x );
      infoLink.set( x);
    });


    return (
      <div {...this.props}>
        <span>{linked.sequence.value}</span>
        <Input valueLink={ jointLink }></Input>
      </div>
    );
  }
}

export default DemoHOCItem;

class Test extends React.Component {
  constructor() {
    super();
    this.state = {
      items: []
    };
  }

  componentDidMount() {
    console.log('Loading Route Sequences from server')
    fetch('http://uberfl.us/sortable.json')
    .then(
      response => {
        return response.json()
      }
    )
    .then(
      items => {
        this.setState(items)
        console.log(items)
      }
    )
  };

  handleSort(sortedArray) {
    console.log('Entering handleSort')
    var newItems = []
    var i=1
    console.log(sortedArray)
    sortedArray.map(function(item) {item.value.sequence=i, ++i, newItems.push(item.value)})
    console.log(newItems)
    this.setState({
      items: newItems
    });
  }

  render() {

    const itemsLink = Link.state( this, 'items' );


    function renderWithSortable(itemLink, index) {
      return (
        <DemoHOCItem className="vertical" sortData={ itemLink } key={ ['item',index].join('_') }>
          {console.log('Rendering Items')}
          {itemLink.sequence}
        </DemoHOCItem>
      );
    }

     return (
      <div className="demo-container">
         <Sortable className="vertical-container" onSort={::this.handleSort} key={shortid.generate()} dynamic>
          {itemsLink.map(renderWithSortable, this)}
        </Sortable>
      </div>
    );
  }
};

export default Test;

It is very fickle. Without the "dynamic"-property and a dynamic key in <Sortable> the items are not rendered. With the dynamic key the focus-issue appears.

But I got it working, following your advice, by using an incrementing key:

class Test extends React.Component {
  constructor() {
    super();
    this.state = {
      items: [],
      _sortableKey: 0
    };
  }

[..]
  handleSort(sortedArray) {
    console.log('Entering handleSort')
    this.state._sortableKey++;
    var newItems = []
    var i=1
    console.log(sortedArray)
    sortedArray.map(function(item) {item.value.sequence=i, ++i, newItems.push(item.value)})
    console.log(newItems)
    this.setState({
      items: newItems
    });
  }

[..]


render() {

    const itemsLink = Link.state( this, 'items' );

       return (
      <div className="demo-container">
         <Sortable className="vertical-container" onSort={::this.handleSort} key={ this.state._sortableKey } dynamic>
          {itemsLink.map(renderWithSortable, this)}
        </Sortable>
      </div>
    );
  }
};

Wohoo!!! Great stuff, thanks for your kind help!

@peletiah peletiah closed this as completed Jul 5, 2016
@gaperton
Copy link

gaperton commented Jul 5, 2016

This Sortable component is very tricky. I can't quickly understand how it works.

What about general sorting, React needs to know that element is moved. So, every element must have persistent index generated once and used as key in react list.

like this:

const list = [{ name: 'a', _id : 0}, { name: 'b', _id: 'whatever' }, ... ];

In your code, I think it should be enough to do it when you fetch elements from the server.
Then, no matter which permutation made on the list, React knows where every particular element is located, and should be able to preserve state.

So, you're supposed to generate list like this:

list.map( element => <div key={ element._id} > ... </div>

Id just must be unique value on this DOM level. If you already have something unique in the element (like image url) you can just use it as key.

It should work in the same way with link.map as well as with regular array.map. But again, this Sortable thingy does some dark magic in rendering, so I'm not completely sure if what I'm talking about is the case.

@gaperton
Copy link

gaperton commented Jul 5, 2016

Btw. With my recent changes, you can shorten this:

    const jointLink = Link.value( linked.info.value, x => {
      linked.info.set( x );
      infoLink.set( x);
    });

to this:

    const jointLink = linked.info.onChange( x => infoLink.set( x ) );

Thank you for your input. Hope you'll enjoy the library. Let me know if there will be some other problems.

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