Giter Site home page Giter Site logo

Comments (25)

jalal246 avatar jalal246 commented on May 12, 2024

Adding an element dynamically after registration is not going to work because the new element won't be configured correctly in the store. So when you are adding a new one it registers as a new container (new branch) instead of adding it to its siblings. That's why it has a wrong index. You can debug it on the console $DFlex find DOMGen and then _branches to check if it's registered correctly. Or store.getSerializedElm(id)

How do you add the new element in the runtime? The ideal situation is to unregister the register again.

  React.useEffect(() => {
    if (taskRef.current) {
      store.register({ id });
    }

    return () => {
      store.unregister(id);
    };
  }, [taskRef.current]);

from dflex.

xdivby0 avatar xdivby0 commented on May 12, 2024

Thanks Jalal!

I'm using the default code from this example. In particular, this is already here:

  React.useEffect(() => {
    if (dndCompRef.current) {
      store.register({ id, depth, readonly });
    }

    return () => {
      store.unregister(id);
    };
  }, [dndCompRef.current]);

What do need to unregister when? I tried (on adding a new element) dismounting all element's DnDComponent, waiting 500ms for good measure, so everything is cleaned up, and then remounting all components (including the dynamically added one). Same error message, I think I am missing something here.

from dflex.

jalal246 avatar jalal246 commented on May 12, 2024

If the container has ["id-1"-"id-2"] and I am mutating a new element id-3 then I should unregister the entire branch and refresh with the new element register/unregister. I guess maybe check if you have the right key for each component if you iterate with the map and make sure the newly added element has a unique id.

from dflex.

xdivby0 avatar xdivby0 commented on May 12, 2024

How do I unregister the entire branch? I saw that I can do store.getContainerByID(id), but not sure how to continue. Just tried unmounting the parent-container and re-adding it back in with one more item, but still no success (same issue).

And yes, the keys that I use for the map part are unique, I just displayed them next to each item for double-checking.

from dflex.

jalal246 avatar jalal246 commented on May 12, 2024

Unregister the components will automatically update the branch

 React.useEffect(() => {
    return () => {
    // Does this actually work?
    // maybe add a console message to check it.
    store.unregister(id);
    const value = store.getSerializedElm(id)
    console.log(value)    // value should be null.
    };
  }, [dndCompRef]);

from dflex.

xdivby0 avatar xdivby0 commented on May 12, 2024

Just tried that and I didn't see something else than null logs, each item was successfully unregistered (and later re-registered)

from dflex.

jalal246 avatar jalal246 commented on May 12, 2024

I'll try to debug it locally on my end and see if it's a bug or not.

from dflex.

xdivby0 avatar xdivby0 commented on May 12, 2024

I'll try to debug it locally on my end and see if it's a bug or not.

Very thankful for your time Jalal, I'm here on standby, let me know if I can help somehow.

from dflex.

jalal246 avatar jalal246 commented on May 12, 2024

#608 Should fix this issue.

from dflex.

xdivby0 avatar xdivby0 commented on May 12, 2024

#608 Should fix this issue.

Thanks Jalal, I noticed two issues:

  1. this breaks everyone's code who trusted the default of commitAfterEndingDrag to be true (which is still documented so in the docs, but doesn't seem to be the case anymore). I tried setting it explicitly, but didn't work either. Then I saw your change in DraggableInterface.ts:33 and next few lines. I tried enabling containerTransition which didn't do anything for me either (also couldn't find anything on that option in the docs).

  2. adding a new element doesn't break the existing ones but I can't move the newly created item. This is the error:

Uncaught TypeError: this.draggedElm is undefined
    ut dflex-dnd.mjs:1
    dt dflex-dnd.mjs:1
    ft dflex-dnd.mjs:1
    Et dflex-dnd.mjs:1
    onMouseDown DnDComponent.tsx:83
    callCallback2 React
    sentryWrapped helpers.ts:98
    React 14
    sentryWrapped helpers.ts:98
    _wrapEventTarget trycatch.ts:231
    instrumentDOM instrument.ts:536
    React 8
    hydrate entry.client.tsx:29
    startTransition React
    hydrate entry.client.tsx:28
    requestIdleCallback handler* entry.client.tsx:39
2 dflex-dnd.mjs:1:30127
    ut dflex-dnd.mjs:1
    dt dflex-dnd.mjs:1
    ft dflex-dnd.mjs:1
    Et dflex-dnd.mjs:1
    onMouseDown DnDComponent.tsx:83
    callCallback2 React
    sentryWrapped helpers.ts:98
    React 14
    bind_applyFunctionN self-hosted:1683
    dispatchDiscreteEvent self-hosted:1640
    sentryWrapped helpers.ts:98
    (Async: EventListener.handleEvent)
    _wrapEventTarget trycatch.ts:231
    instrumentDOM instrument.ts:536
    React 4
    forEach self-hosted:4909
    React 4
    hydrate entry.client.tsx:29
    startTransition React
    hydrate entry.client.tsx:28
    (Async: requestIdleCallback handler)
    <anonymous> entry.client.tsx:39

The first line (this.draggedElm is undefined) makes me think I forgot something. Still having the store.register and unregister inside the useState.

from dflex.

jalal246 avatar jalal246 commented on May 12, 2024

Is this code similar to this one: https://github.com/dflex-js/dflex/blob/main/packages/dflex-dnd-playground/src/components/stream/ContinuouslyUpdatingTodo.tsx

It's important to have a unique key for react that doesn't depend on the array index.

from dflex.

xdivby0 avatar xdivby0 commented on May 12, 2024

Yes, just tried reducing my example to a minimum:

{items.map((item, index) => (
    <DnDComponent Component="div" key={item.id} registerInput={{id: item.id}}>
      <div>
        blub
      </div>
    </DnDComponent>
))}

the DnDComponent is 99% from your example in the docs.

The item.id are generated by a collision resistant function and are unique + don't depend on the array index.

from dflex.

jalal246 avatar jalal246 commented on May 12, 2024

If the key is the same in each iteration then react won't remove elements with identical keys. Wich means partial updating of the DOM. To enforce a total removal of elements from tree you should tell React by providing new keys so the entire children would be unmount/mount.

const items = [{id: 0}, {id: 1}, {id: 2}]
const newItems = [{id: 0}, {id: 1}, {id: 2}, {id: 3}]

Then react will keep elements 0, 1, and 2 and insert 3.

What we need is a total removal:

const items = [{id: 0}, {id: 1}, {id: 2}]
const newItems = [{id: 00}, {id: 01}, {id: 02}, {id: 03}]

Then react will insert entire new brand children.

from dflex.

xdivby0 avatar xdivby0 commented on May 12, 2024

So to stream in a new element, we have to make all sibling elements unregister first. Got it. I got success by unmounting all elements for a short timeout so that all of them are getting remounted at once. Seems to work now.

But still, why can't I make it reconcile? I have the commit option set to enableAfterEndingDrag: true, still the order is only remained via the transform property. I even tried calling store.commit() in the onMouseUp function, still no success. Am I missing something here?

from dflex.

xdivby0 avatar xdivby0 commented on May 12, 2024

@jalal246 do you have any idea? It's almost working, just the commit won't work

from dflex.

jalal246 avatar jalal246 commented on May 12, 2024

I'll look into it. But if it's not working then it should throw an error. Also, it seems your process.env is not set. Setting it to development might help with error messages and debugging.

from dflex.

xdivby0 avatar xdivby0 commented on May 12, 2024

What do you mean process.env? Do you mean NODE_ENV? As this is a frontend package you can't just expect it to be run inside process.env.NODE_ENV environment :/

I can't set process.env for an app that gets executed only inside the browser. Yes, you can hack something together in React, but not so easy in Remix for example. I'll try to create a temporary workaround just for logging, but I think you should be careful with this

from dflex.

jalal246 avatar jalal246 commented on May 12, 2024

Running remix build compiles using the value of process.env.NODE_ENV if it corresponds with a valid mode: "production", "development" or "test". If the value of process.env.NODE_ENV is invalid, "production" is used as a default.

https://remix.run/docs/en/v1/guides/envvars

from dflex.

xdivby0 avatar xdivby0 commented on May 12, 2024

The env variables are not in the frontend of Remix, they are only existent in the backend. Your package is run on the client where process.env is not defined (the browser doesn't know about process.env). But the argument is not about how I get it there, there probably are some hacky ways. The issue is that NODE_ENV is not something you can count on in the frontend (in general, not just for Remix).

Having said this, of course this is an open source project, and I am not paying for dflex. If you tell me this issue is not your priority at the moment and you are not going to spend time looking into why the reconciliation broke from 3.7.1 to 3.8.0 that's okay too, I'll just use up/down arrows for now then. I was just wondering because it worked fine before and I saw some commits regarding the enableAfterEndingDrag default and thought it would be easier to spot.

from dflex.

jalal246 avatar jalal246 commented on May 12, 2024

Your app run through bundler. To help you debug your code DFlex provides a production version and a development version. The same goes for React as well. Otherwise, you won't be able to get error messages because it's a production version. I'll suggest reading the documentation posted above again. To debug and find the issue it would be helpful to produce the bug. Same for #604.

from dflex.

jalal246 avatar jalal246 commented on May 12, 2024

Also, I am working on a better approach to deal with elements registered/unregistered but I am not sure when I am going to finish and merge. Hopefully soon.

from dflex.

xdivby0 avatar xdivby0 commented on May 12, 2024

Your app run through bundler. To help you debug your code DFlex provides a production version and a development version. The same goes for React as well. Otherwise, you won't be able to get error messages because it's a production version. I'll suggest reading the documentation posted above again. To debug and find the issue it would be helpful to produce the bug. Same for #604.

As I said, Remix doesn't expose Envs to the frontend, especially not directly as process.env.something. See the next section of the mentioned docs above.

I don't think I can make process.env.NODE_ENV be development for your package, sorry.

Also, I am working on a better approach to deal with elements registered/unregistered but I am not sure when I am going to finish and merge. Hopefully soon.

As for this, the issue with adding the new element to the store is resolved. As you said, I had to unmount and remount every sibling. Now that works. Just the commiting doesn't work, it's just behaving as if I'd disabled commit after ending drag. Not even the store.commit() does anything (and doesn't display errors, not even minimized errors).

from dflex.

jalal246 avatar jalal246 commented on May 12, 2024

doesn't display errors, not even minimized error.

It won't if it's a production bundle that's the point of shipping two bundles.

from dflex.

xdivby0 avatar xdivby0 commented on May 12, 2024

doesn't display errors, not even minimized error.

It won't if it's a production bundle that's the point of shipping two bundles.

Hmm well, that's difficult then. I don't think we can solve this with remix as process.env is not available in the frontend.

from dflex.

jalal246 avatar jalal246 commented on May 12, 2024

@xdivby0 if this issue is not fixed by the current release feel free to reopen it.

from dflex.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.