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

Option elements shouldn't have keys, based on index #48

Merged
merged 4 commits into from
Oct 17, 2016
Merged

Commits on Sep 11, 2016

  1. Option elements shouldn't have keys, based on index

    Keys in options shouldn't depend on key index here : https://github.com/rkit/react-select2-wrapper/blob/master/src/components/Select2.js#L159
    ```jsx
    /* This */
    return (<option key={`option-${k}`} value={id} {...itemParams}>{text}</option>);
    /* should be changed to this: */
    return (<option key={`option-${id}`} value={id} {...itemParams}>{text}</option>);
    ```
    Otherwise it doesn't re-render options, when options list is changed.
    Boorj authored Sep 11, 2016
    Configuration menu
    Copy the full SHA
    9df3882 View commit details
    Browse the repository at this point in the history

Commits on Sep 13, 2016

  1. normal key

    Also we have to provide an ability use several options with same IDs, which is prohibited by react, but not strictly prohibited by select.
    so, best solution seems to be
    ```js
          const key = 'option-' + k + '-' + id;
          return (<option key={key} value={id} {...itemParams}>{text}</option>);
    ```
    And about sanitizing: was not sure, if react could use _any_ string as a key. Had to [check](https://facebook.github.io/react/docs/reconciliation.html#keys)
    So now it seems to solve the problem
    Boorj authored Sep 13, 2016
    Configuration menu
    Copy the full SHA
    d26c033 View commit details
    Browse the repository at this point in the history

Commits on Sep 16, 2016

  1. removed const key

    Is it better?
    Boorj authored Sep 16, 2016
    Configuration menu
    Copy the full SHA
    f30cbe9 View commit details
    Browse the repository at this point in the history

Commits on Oct 16, 2016

  1. leaved option-${id}

    that should work fine
    Boorj authored Oct 16, 2016
    Configuration menu
    Copy the full SHA
    ae572ad View commit details
    Browse the repository at this point in the history