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

When using MobX React, an observer decorated draggable component does not get rerendered until clicked #156

Closed
hoenth opened this issue Oct 30, 2017 · 16 comments

Comments

@hoenth
Copy link

hoenth commented Oct 30, 2017

Bug or feature request?

Bug

Expected behaviour

When a MobX React observer decorated draggable component has a state change (either through an interface interaction or directly changing the values in the observable store, and MobX issues a rerender of all observer components that care about the observable data, the elements that have changed should rerender.

Actual behaviour

The element does not rerender until it is clicked (as if to start a drag)

Steps to reproduce

Create an observer decorated component that accepts an observable object as a prop. Place the component in a draggable area. Change an attribute of the object (this should cause MobX to rerender the component) and verify that autorun runs.

Note that the component was not rerendered.

Click in an area of the draggable element that would cause Draggable to capture the event. Note that the element has rerendered.

Browser version

Demo

https://www.webpackbin.com/bins/-KxiZyKnJzY-13XSEhqT

You can drag and drop the elements, and click on the checkbox. However, the value does not change until you click into an area outside of the checkbox.

@Judahmeek
Copy link

I assisted @hoenth in creating the Demo and can confirm this is an issue.

@alexreardon
Copy link
Collaborator

It looks like something is blocking the render to children. The only way this can happen if a parent renders is if shouldComponentUpdate returns false. We have tests to ensure that the components render if their parents do. My suspicion is that the parent itself is not rendering. I can take a look in a few days but that information might be helpful

@alexreardon
Copy link
Collaborator

When I click the input the parent element is not rendered

https://www.webpackbin.com/bins/-KxlrpZw0lIcpkqKc4m6

I suspect your problem is with this line:

this.props.item.content = !this.props.item.content;

Changing props like this will not trigger a re-render in react. You cannot update your own props directly. I cannot see anything in the mobx-react docs that would allow this syntax to work

More info: https://stackoverflow.com/a/26089687/1374236

Please reopen this if needed

@hoenth
Copy link
Author

hoenth commented Oct 31, 2017

@alexreardon Thanks for taking a look at this. I have updated the webpackbin to demonstrate that the list is being rerendered. I am rendering two lists. In the first, each element is wrapped in draggable. In the second, the element is not wrapped in draggable.

When you click the checkbox in an element in the first list, you will notice that the content gets updated (rerendered) in the second list, but not in the first.

https://www.webpackbin.com/bins/-KxmVueDfBxRAnRjUX9K

It doesn't appear that I am able to re-open this issue.

@tobiasandersen
Copy link
Contributor

I think the problem here is that the observable prop isn't inside an observer. I.e. what you render here:

{(provided, snapshot) => (
  <div>
    <div
      ref={provided.innerRef}
      style={getItemStyle(
        provided.draggableStyle,
        snapshot.isDragging
      )}
      {...provided.dragHandleProps}
    >
      {this.props.item.content.toString()}
      <input
        type="checkbox"
        onClick={e => {
          e.stopPropagation();
          this.props.item.content = !this.props.item.content;
        }}/>
    </div>
    {provided.placeholder}
   </div>
 )}

must also be wrapped in an observer.

@hoenth
Copy link
Author

hoenth commented Oct 31, 2017

@tobiasandersen Thanks for your thoughts. Do you mean:

observer({(provided, snapshot) => (
     <div>
       <div
         ref={provided.innerRef}
         style={getItemStyle(
           provided.draggableStyle,
           snapshot.isDragging
         )}
         {...provided.dragHandleProps}
       >
         {this.props.item.content.toString()}
         <input
           type="checkbox"
           onClick={e => {
             e.stopPropagation();
             this.props.item.content = !this.props.item.content;
           }}/>
       </div>
       {provided.placeholder}
      </div>
    )}
)

@tobiasandersen
Copy link
Contributor

No:({(provided, snapshot) => /* This must be inside an observer */ }

@hoenth
Copy link
Author

hoenth commented Oct 31, 2017

@tobiasandersen That worked. I pulled the internals into a new component, wrapped that component in an observer and passed provided and snapshot as props.

Thanks for your help!

@alexreardon
Copy link
Collaborator

Glad we got to the bottom of this! Thanks everyone

@84pennies
Copy link

@tobiasandersen That worked. I pulled the internals into a new component, wrapped that component in an observer and passed provided and snapshot as props.

Thanks for your help!

Hey do you have an example of this code? Facing the same quirk...

@Obiwarn
Copy link

Obiwarn commented Nov 13, 2018

I ran into the same issue.
@alexreardon MobX is getting more and more popular. Maybe there should be a more obvious / elegant solution to this?

@jlindberg-oss
Copy link

jlindberg-oss commented Mar 21, 2019

@84pennies this is what worked for me:

export const ItemsList = observer(({provided, snapshot}) => {
    return store.itemsStore.items.map((item, index) => (
        <Draggable key={'' + item.id} draggableId={'' + item.id} index={index}>
          {(provided, snapshot) => (
            <div
              ref={provided.innerRef}
              {...provided.draggableProps}
              {...provided.dragHandleProps}
              style={getItemStyle(
                snapshot.isDragging,
                provided.draggableProps.style
              )}
            >
              <Item key={item.id} item={item} />
            </div>
          )}
        </Draggable>
    ))
  })

// Normally you would want to split things out into separate components.
// But in this example everything is just done in one place for simplicity
export const ItemsWrapper = (() =>
  <DragDropContext onDragEnd={onDragEnd}>
    <Droppable droppableId="droppable">
      {(provided, snapshot) => (
        <div
          {...provided.droppableProps}
          ref={provided.innerRef}
          style={getListStyle(snapshot.isDraggingOver)}
        >
          <ItemsList provided={provided} snapshot={snapshot} />
          {provided.placeholder}
        </div>
      )}
    </Droppable>
  </DragDropContext>
)

(The starting point for my code is the simple vertical list example at https://github.com/atlassian/react-beautiful-dnd/blob/master/docs/about/examples.md#basic-samples)

I guess the need to do it this way is explained by:
https://github.com/mobxjs/mobx/blob/gh-pages/docs/best/react.md#mobx-only-tracks-data-accessed-for-observer-components-if-they-are-directly-accessed-by-render

@another-guy
Copy link

tobiasandersen's solution works in 2020 when it comes to functional components as well.

@tthiatma
Copy link

tthiatma commented Aug 14, 2020

tobiasandersen's solution works in 2020 when it comes to functional components as well.

Could you provide example for functional component please

@dubinbin
Copy link

well, according to tobiasandersen's solution ,I modify the code and it work in my project, (this is a simple example code and i hide some business code :))

the outermost wrapper like it 👇

<DragDropContext onDragEnd={(result) => onDragEnd(result)}>
     <Droppable droppableId="MiddleModule">
        {(provided: DroppableProvided) => (
            <div ref={provided.innerRef}  {...provided.droppableProps}>      
                   <PanelWrap/>
             </div>
         )}
     </Droppable> 
</DragDropContext>

// next
export const PanelWrap = observer(() => {
    const { componentQueue } = useStore()  // data from mobx store

    return (
        <div>
            {componentQueue.map((Comp: IComp, index: number) => {
                const Component = Comp.component;
                return (
                    <Draggable draggableId={`draggable-${index}`} key={index} index={index}>
                        {(provided) => (
                            <PanelItem 
                                Comp={Comp} 
                                provided = {provided} 
                                Component={Component} 
                                index={index} 
                            />
                        )}
                    </Draggable>
                )
            })}
        </div>
    )
})


export const PanelItem = observer(() => {
      return (   
          <div ref={provided.innerRef}
                    {...provided.draggableProps}
                    {...provided.dragHandleProps}>
          // some business code
        </div>
      )
})


@16Bonte
Copy link

16Bonte commented Aug 1, 2021

Above solutions did not work for me but for some reason my component is updating if I convert observable object into classical JS structure with toJS from mobx

import { toJS } from "mobx";

return (
        <DragDropContext onDragEnd={onDragEnd}>
          <Droppable droppableId="droppable">
            {(provided, snapshot) => (
              <div
                {...provided.droppableProps}
                ref={provided.innerRef}
                style={getListStyle(snapshot.isDraggingOver)}
              >
                {toJS(servicesData).map((service, index) => {
                  return (
                    <Service
                      key={`service-${service.service_id}`}
                      service={service}
                    />
                  );
                })}
              </div>
            )}
          </Droppable>
        </DragDropContext>
      </Fragment>
);

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