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

playground issues #597

Open
davekaro opened this issue Jan 15, 2023 · 6 comments · Fixed by #598 or #635
Open

playground issues #597

davekaro opened this issue Jan 15, 2023 · 6 comments · Fixed by #598 or #635
Labels
bug Something isn't working

Comments

@davekaro
Copy link

davekaro commented Jan 15, 2023

Hello,

I tried running the playground locally and noticed all sorts of weird issues. Are some of these not expected to work? The /grid example was worse, I couldn't get it to sort at all, and often ended up in states like this:
2

The root example

<ContainerBasedEvent
  isHorizontal={false}
  isIncludeOneContainer={false}
/>

only works if the vertical space is enough to fit the whole list. Otherwise it throws Uncaught TypeError: this._innerThresholdInViewport is null. I saw similar error in my own application, where if I didn't have more than 10 items in the list, I'd get an error that this._outerThresholdInViewport was null (but it was called this.V but tracing the code looked like this method. Then I noticed a variable private static _MAX_NUM_OF_SIBLINGS_BEFORE_DYNAMIC_VISIBILITY = 10; so if you have fewer elements than that, then _outerThresholdInViewport will always stay null.

For more context, I'm attempting to implement sorting without React. My project is a Rails app using Stimulus JS. I've probably wired up some things wrong, but since many of the demos in the playground seemed off too, I wondered if maybe some things are still in flight. Is this project in a production ready state? Thanks!

@jalal246
Copy link
Member

Hi @davekaro, Grid is definitely not ready yet. DFlex Grid implementation depends on each element's position and is unrelated to the CSS grid. Implementing the grid this way allows DFlex to form the entire layout from scratch away from Horizontal/Vertical strict lists. And because of this, there are many things to do to ship this feature. While it works in some cases, It is still a very early stage and takes a lot to be done. You can definitely use DFlex without React, but the project itself is still in the making.

Thanks for opening this issue and for the feedback.

@davekaro
Copy link
Author

Thanks for the perspective. Maybe a simpler start is asking if there is something broken with the build step, or if I'm misunderstanding something. Right now in my project, I've added dflex to package.json ("@dflex/dnd": "^3.7.0",) and I'm importing it like import { store, DnD } from "@dflex/dnd";. But once I start dragging I get this error:

Uncaught TypeError: this[t2] is not a function
    X dflex-dnd.mjs:1
    animatedListener dflex-dnd.mjs:1
    animatedScrollListener dflex-dnd.mjs:1
    ...

Looking at the source for animatedListener, it calls const isUpdated = this[setter](); where setter is a string passed in. Looking at the minified source in my web browser, I can see this.animatedListener.call(this,"_updateScrollCoordinates",this.$) so it should be calling the _updateScrollCoordinates function. However, in that same source file, there is no function named _updateScrollCoordinates - I guess because it's minified to another name? Or maybe it's been stripped out because it's not being called anywhere else?

@jalal246 jalal246 added the bug Something isn't working label Jan 16, 2023
@jalal246
Copy link
Member

jalal246 commented Jan 16, 2023

It's being stripped when building the minified version. It's passed as a string and Terser can't recognize it when minifying.

https://github.com/dflex-js/dflex/blob/fd641dde9244ab50c0041152c3c7289dfcb333d4/packages/dflex-core-instance/src/Container/DFlexScrollContainer.ts#LL518-L522C4

@jalal246
Copy link
Member

@davekaro I couldn't produce the same error you've got. I made a quick demo https://codesandbox.io/s/staging-shadow-9mytwo and it's working as expected. Still going to fix passing method reference instead of passing the named string anyway.

@jalal246 jalal246 linked a pull request Jan 17, 2023 that will close this issue
@davekaro
Copy link
Author

it's working as expected

I believe this is because looking at the source, it's using a non-minified version dev.js.

Screenshot 2023-01-17 092042

Your fix should work, I tried to test by referencing that commit in my package.json "@dflex/dnd": "git://github.com/dflex-js/dflex.git#146d11be260316dde0931dfbc89c6b30b16c84c1", but it npm install failed. If you have any suggestions on how to use the a non-released version, otherwise I'll wait for the next release. Thanks!

@jalal246
Copy link
Member

I published a patched version I hope it solves the issue. But if there's another bug let me know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants