omcljs / om

ClojureScript interface to Facebook's React
6.65k stars 364 forks source link

Element delete rendering bug #143

Closed vseguip closed 10 years ago

vseguip commented 10 years ago

Hi I'm just dabling into web frontend programming. I've been experimenting a bit with Om/React combo and I've encountered the following "bug". As I understand it, the way Om/React works is by diffing the DOM shadow rendered state vs the real DOM and rerendering what is needed. The problem I'm encountering arises when we delete an element from the middle of the list. For instance if we have a list as so [1 2 3 4 5] and we render it like so:

<ul>
<li>1</li>
<li>3</li>
<li>3</li>
<li>4</li>
<li>5</li>
</ul>

if we delete an element in the middle of the list (say 3) Om/React will not actually remove the DOM node corresponding to <li>3</li> but will actually rerender the list removing the last node (corresponding to <li>5</li>). This causes weird things to happen specially when applying transitions. Instead of the <li>3</li> element fading out we will observe it replaced immediately by element <li>4</li>and the last <li> node fading out.

Following is minimal example I could come up (based on mies-om)

(ns om_list_bug.core
  (:require-macros [cljs.core.async.macros :refer [go]])
  (:require [om.core :as om :include-macros true]
            [om.dom :as dom :include-macros true]
            [cljs.core.async :refer [put! chan <!]]))
(enable-console-print!)

(def app-state
  (atom
    {:contacts
     [{:first "Ben" :last "Bitdiddle" :email "benb@mit.edu"}
      {:first "Alyssa" :middle-initial "P" :last "Hacker" :email "aphacker@mit.edu"}
      {:first "Eva" :middle "Lu" :last "Ator" :email "eval@mit.edu"}
      {:first "Louis" :last "Reasoner" :email "prolog@mit.edu"}
      {:first "Cy" :middle-initial "D" :last "Effect" :email "bugs@mit.edu"}
      {:first "Lem" :middle-initial "E" :last "Tweakit" :email "morebugs@mit.edu"}]}))

(defn middle-name [{:keys [middle middle-initial]}]
  (cond
    middle (str " " middle)
    middle-initial (str " " middle-initial ".")))

(defn display-name [{:keys [first last] :as contact}]
  (str last ", " first (middle-name contact)))

(defn contact-view [contact owner]
  (reify
    om/IRenderState
    (render-state [this {:keys [delete]}]
      (dom/li nil
        (dom/span nil (display-name contact))
        (dom/button #js {:onClick (fn [e] (put! delete @contact))} "Delete")))))

(defn contacts-view [app owner]
  (reify
    om/IInitState
    (init-state [_]
      {:delete (chan)})
    om/IWillMount
    (will-mount [_]
      (let [delete (om/get-state owner :delete)]
        (go (loop []
          (let [contact (<! delete)]
            (om/transact! app :contacts
              (fn [xs] (vec (remove #(= contact %) xs))))
            (recur))))))
    om/IRenderState
    (render-state [this {:keys [delete]}]
      (dom/div nil
        (dom/h2 nil "Contact list")
        (apply js/React.addons.CSSTransitionGroup  #js {:transitionName "example"
                                                                        :component dom/ul}
          (om/build-all contact-view (:contacts app)
            {:init-state {:delete delete}}))))))

(om/root
 contacts-view
 app-state
 {:target (. js/document (getElementById "app"))})

Index.html

<html>
  <style>
   .example {
     opacity: 0.01;
     transition: opacity 0.5s ease-in;
   }

   .example-enter.animatecard-enter-active {
     opacity: 1;
   }

   .example-leave {
     opacity: 1;
     transition: opacity .5s ease-in;
   }

   .example-leave.example-leave-active {
     opacity: 0.01;
   }   
  </style>
  <body>
    <div id="app"></div>
    <script src="http://fb.me/react-with-addons-0.9.0.js"></script>
    <script src="out/goog/base.js" type="text/javascript"></script>
    <script src="om_list_bug.js" type="text/javascript"></script>
    <script type="text/javascript">goog.require("om_list_bug.core");</script>
  </body>
</html>
swannodette commented 10 years ago

You need to provide a key in order for React to know that you deleted something. Please refer to the documentation of om.core/build as well as this.

the-kenny commented 10 years ago

also take a look at http://facebook.github.io/react/docs/reconciliation.html

On Wed, Mar 12, 2014 at 4:30 PM, David Nolen notifications@github.comwrote:

Closed #143 https://github.com/swannodette/om/issues/143.

— Reply to this email directly or view it on GitHubhttps://github.com/swannodette/om/issues/143 .

vseguip commented 10 years ago

Thanks a lot David, that worked like a charm. I hadn't understood exactly what that was used for when I first read that, I'm still very new to OM/React. Sorry for the noise.

leblowl commented 10 years ago

Thank you for clarifying this, I too had this issue

On Wed, Mar 12, 2014 at 9:09 AM, Vicent Seguí Pascual < notifications@github.com> wrote:

Thanks a lot David, that worked like a charm. I hadn't understood exactly what that was used for when I first read that, I'm still very new to OM/React. Sorry for the noise.

Reply to this email directly or view it on GitHubhttps://github.com/swannodette/om/issues/143#issuecomment-37427496 .