Open-Assistant
Open-Assistant copied to clipboard
#437 sortableItem draggable
fix for #437
I will leave this to @AbdBarho as I haven't looked into dnd-kit at all
One thing that might be a problem - the "..." button I added previously for long texts will not work with this change. Got a codespaces span up to verify.
One thing that might be a problem - the "..." button I added previously for long texts will not work with this change. Got a codespaces span up to verify.
yeah, with the ...
it is complicated since the DnD listens on pointer down, and since it listens on the entire subtree you can literally drag & drop through the modal, which is not great.
Possible solutions:
-
event.stopPropagation()
- change the layout / position of the expand or
...
button.
@jojopirker @kostiak what do you think?
I never used DnDKit before so I'm not sure...
The reason I added it as a ...
in the first place was because the whole row wasn't selectable like it will be after this PR. I have done some work with combining two buttons on top of each other like that in the past and it's not worth it. I propose we keep the text shortening as-is but move the button somewhere else. Or change the layout somehow.
@AbdBarho @jojopirker Any suggestions on where we can put the button now? I was initially thinking of putting it at the end, but that will just take a lot of space and will not be needed most of the time.
Another, harder solution might be to remove the button and somehow detect when you start moving a component but don't actually change it's position, and trigger the modal on that.
Maybe even turn it to some kind of tooltip on hover?
@kostiak, that later comment is what I have toyed with in the video demo I posted in the website channel (https://discordapp.com/channels/1055935572465700980/1055940465536532571/1061258548400619560).
I've pushed what I have to my fork (https://github.com/LAION-AI/Open-Assistant/compare/main...othrayte:Open-Assistant:collapse-on-sort) but keep in mind it was an experiment so there is a lot of other mess in there from other experiments. The main change I think we'll need is setting the activationConstraint on the sensors:
useSensor(MouseSensor, { activationConstraint: { distance: 4 } }),
useSensor(TouchSensor, { activationConstraint: { distance: 4 } }),
That allows us to receive click events which in my branch I tested using to expand and collapse the items by clicking anywhere.
Are we moving forward with this PR? Or have we fixed the underlying problem with other PRs?
This PR has been quiet for about a week now. I think the underlying problem has already been fixed so I'm going to close this for now. If it's going to be resumed, please re-open.
just checked, no need to re-open! thx!