madvas / cljs-react-material-ui

Clojurescript library for using material-ui.com
Eclipse Public License 1.0
205 stars 32 forks source link

Issue splitting Table into multiple components with Reagent #16

Open luposlip opened 8 years ago

luposlip commented 8 years ago

Hi there!

Generally cljs-react-material-ui 0.2.19 seems pretty solid from my point of view, using it with reagent 0.6.0-rc.

I have a situation where I create a Table (based on data from a re-frame subscription). When I have all the code for creating the rui/table in the same component fn it works without an issue.

But when I split the component into a component fn calculating the output for the table and an inner component fn for each row a couple of odd things happen:

  1. The onRowSelection defined for the rui/table doesn't trigger. I fixed this by using an on-click handler for each rui/table-row instead
  2. I get this error when I select a row: material-ui.inc.js:777 Uncaught Invariant Violation: Objects are not valid as a React child

My components are as follows:

(defn thing-row [thing-data click-handler is-selected]
  (let [thing (:thing thing-data)
        thing-id (:thing/id thing)]
    ^{:key thing-id}        
    [rui/table-row {:selectable true
                    :selected (is-selected)
                    :hoverable true
                    :on-click click-handler}
      [rui/table-row-column small-cell-styles (:distance thing-data)]
      [rui/table-row-column (:thing/name thing)]
      [rui/table-row-column thing-id]]))

(defn things []  
  (let [the-things (subscribe [:things])
        the-thing (subscribe [:thing])]
    (fn []
      (let [nearby (:some-things @the-things)
            others (:other-things @the-things)
            all-things (into nearby others)]
      [rui/table
        [rui/table-header {:displaySelectAll false
                           :adjustForCheckbox false}
          [rui/table-row
            [rui/table-header-column small-cell-styles "Distance"]
            [rui/table-header-column "Name"]
            [rui/table-header-column "ID"]]]
        [rui/table-body {:displayRowCheckbox true
                         :deselectOnClickaway false
                         :showRowHover true}
          (for [thing-data all-things]
              [thing-row thing-data
                       #(do (dispatch [:thing thing-data])
                            (dispatch [:push {:event-data [:location/update (:location thing-data)]}]))
                       #(= (thing-data-id @the-thing) (thing-data-id thing-data))])]]))))

The complete stack trace is:

invariant   @   material-ui.inc.js:777
traverseAllChildrenImpl @   material-ui.inc.js:11786
traverseAllChildrenImpl @   material-ui.inc.js:11742
traverseAllChildrenImpl @   material-ui.inc.js:11742
traverseAllChildren @   material-ui.inc.js:11814
forEachChildren @   material-ui.inc.js:11533
(anonymous function)    @   material-ui.inc.js:54187
calculatePreselectedRows    @   material-ui.inc.js:54196
componentWillReceiveProps   @   material-ui.inc.js:54099
updateComponent @   material-ui.inc.js:13708
receiveComponent    @   material-ui.inc.js:13641
receiveComponent    @   material-ui.inc.js:4156
updateChildren  @   material-ui.inc.js:12911
_reconcilerUpdateChildren   @   material-ui.inc.js:12464
_updateChildren @   material-ui.inc.js:12566
updateChildren  @   material-ui.inc.js:12554
_updateDOMChildren  @   material-ui.inc.js:8200
updateComponent @   material-ui.inc.js:8029
receiveComponent    @   material-ui.inc.js:7985
receiveComponent    @   material-ui.inc.js:4156
updateChildren  @   material-ui.inc.js:12911
_reconcilerUpdateChildren   @   material-ui.inc.js:12464
_updateChildren @   material-ui.inc.js:12566
updateChildren  @   material-ui.inc.js:12554
_updateDOMChildren  @   material-ui.inc.js:8200
updateComponent @   material-ui.inc.js:8029
receiveComponent    @   material-ui.inc.js:7985
receiveComponent    @   material-ui.inc.js:4156
updateChildren  @   material-ui.inc.js:12911
_reconcilerUpdateChildren   @   material-ui.inc.js:12464
_updateChildren @   material-ui.inc.js:12566
updateChildren  @   material-ui.inc.js:12554
_updateDOMChildren  @   material-ui.inc.js:8200
updateComponent @   material-ui.inc.js:8029
receiveComponent    @   material-ui.inc.js:7985
receiveComponent    @   material-ui.inc.js:4156
_updateRenderedComponent    @   material-ui.inc.js:13844
_performComponentUpdate @   material-ui.inc.js:13822
updateComponent @   material-ui.inc.js:13741
receiveComponent    @   material-ui.inc.js:13641
receiveComponent    @   material-ui.inc.js:4156
_updateRenderedComponent    @   material-ui.inc.js:13844
_performComponentUpdate @   material-ui.inc.js:13822
updateComponent @   material-ui.inc.js:13741
performUpdateIfNecessary    @   material-ui.inc.js:13655
performUpdateIfNecessary    @   material-ui.inc.js:4190
runBatchedUpdates   @   material-ui.inc.js:3782
perform @   material-ui.inc.js:5267
perform @   material-ui.inc.js:5267
perform @   material-ui.inc.js:3721
flushBatchedUpdates @   material-ui.inc.js:3804
closeAll    @   material-ui.inc.js:5333
perform @   material-ui.inc.js:5280
batchedUpdates  @   material-ui.inc.js:15742
enqueueUpdate   @   material-ui.inc.js:3832
enqueueUpdate   @   material-ui.inc.js:14647
enqueueForceUpdate

Am i doing something wrong?

Best, Henrik

luposlip commented 8 years ago

UPDATE:

Just for your info: I've tried to replace my component fns for generating the Table and the TableRow's with code from @DaveWM's https://github.com/DaveWM/reagent-material-ui. And it immediately works:

(defn thing-row [thing-data click-handler is-selected]
  (let [thing (:thing thing-data)
        thing-id (:thing/id thing)]
    [TableRow {:selectable true
               :selected (is-selected)
               :hoverable true
               :on-click click-handler}
      [TableRowColumn small-cell-styles (:distance thing-data)]
      [TableRowColumn (:thing/name thing)]
      [TableRowColumn thing-id]]))

(defn things []  
  (let [the-things (subscribe [:things])
        the-thing (subscribe [:thing])]
    (fn []
      (let [nearby (:some-things @the-things)
            others (:other-things @the-things)
            all-things (into nearby others)]
      [Table
        [TableHeader {:displaySelectAll false
                      :adjustForCheckbox false}
          [TableRow
            [TableHeaderColumn small-cell-styles "Distance"]
            [TableHeaderColumn "Name"]
            [TableHeaderColumn "ID"]]]
        [TableBody {:displayRowCheckbox true
                    :deselectOnClickaway false
                    :showRowHover true}
          (for [thing-data all-things]
              ^{:key (thing-data-id thing-data)}
              [thing-row thing-data
                       #(do (dispatch [:thing thing-data])
                            (dispatch [:push {:event-data [:location/update (:location thing-data)]}]))
                       #(= (thing-data-id @the-thing) (thing-data-id thing-data))])]]))))

And yes, I have tried to move the :key meta data from the inner to the outer component fn also when using your library! ;)

Best, Henrik

luposlip commented 8 years ago

ANOTHER UPDATE: Hmm... I removed the @DaveWM library and reverted back to the version of my code using your library.. And now the stack trace disappeared!

luposlip commented 8 years ago

But the first issue remains - the onRowSelection callback still isn't triggered when splitting into 2 component fns. Reopening.

madvas commented 7 years ago

It's possible that material-ui doesn't want to allow table-body to have direct child other than table-row. But that's okay. If you need to reuse some piece of code in reagent in such case, you need to call it as a regular function. So instead of [thing-row ...] you'd use (thing-row ...)