-
Notifications
You must be signed in to change notification settings - Fork 108
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
feat: added axis limitation option #593
base: master
Are you sure you want to change the base?
Conversation
Thanks for this. How should it affect keyboard dragging and the accessibility features? |
For keyboard dragging I see to possibilities:
(I think both are equally viable, but it should be noted in the readme depending on which we decide on) For accessibility, I need to take a look, I didn't think about that, maybe have the axis indicated in the screen reading? I'm not sure if that is necessary though |
If you have a few zones on the screen this feature can make some of them
unreachable (e.g axis is set to y and there is another zone to the side).
With keyboard you'd still be able to get there.
…On Thu, Aug 1, 2024, 22:54 Travis Lane ***@***.***> wrote:
Thanks for this. How should it affect keyboard dragging and the
accessibility features?
For keyboard dragging I see to possibilities:
- leave it as is, because its always in one direction because of how
the listeners work
- have axis: "x" limit it to left and right arrows, and axis: "y"
limit it to up and down
(I think both are equally viable, but it should be noted in the readme
depending on which we decide on)
For accessibility, I need to take a look, I didn't think about that, maybe
have the axis indicated in the screen reading? I'm not sure if that is
necessary though
—
Reply to this email directly, view it on GitHub
<#593 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AE4OZC7632MCVTBQDPH4CPDZPIVXBAVCNFSM6AAAAABLZB5DNWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENRSHE3DONJQGM>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
Hmm, I didn't realize that from looking at it. I think it makes sense to have it prevent vertical/horizontal movement for the keyboard too. I'll do some testing and update the PR. |
@isaacHagoel I believe this should address both issues: Setting Setting the Both alerts respect the |
Thanks. I will look into it tomorrow or on Monday.
Moving an item between zones is done by starting a drag (enter) and then
tabbing around if I remember correctly, not via the left/right arrows
…On Fri, Aug 2, 2024 at 11:31 PM Travis Lane ***@***.***> wrote:
@isaacHagoel <https://github.com/isaacHagoel> I believe this should
address both issues:
Setting axis to "y" disables dragging via the left and right arrow keys,
and alerts the screen reader if the user attempts to use the left and right
arrow keys.
Setting the axis to "x" does the same but for the up and down arrow keys.
Both alerts respect the ariaDisabled config property. Any additional
thoughts?
—
Reply to this email directly, view it on GitHub
<#593 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AE4OZC7YGKVNINMHCHIJC6DZPOCZXAVCNFSM6AAAAABLZB5DNWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENRVGQYTEMJUG4>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
} | ||
swap(items, idx, idx + 1); | ||
dispatchFinalizeEvent(node, items, {trigger: TRIGGERS.DROPPED_INTO_ZONE, id: focusedItemId, source: SOURCES.KEYBOARD}); | ||
if (config.axis === "both" || config.axis === "x") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as you can see in the original code, arrow right is exactly like arrow down, it moves the item from index i to index i+1 if possible, without concerning itself with where the items are placed in space.
To achieve the axis limitation might be tricky.
It can potentially be achieved by checking bounding client rects relative to the original position of the dragged item but i think even that won't work in some edge cases
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. Perhaps it makes sense to only have the limit work for pointer drag, since its mainly useful for mobile devices
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's take a step back for a minute. What is the problem that limiting the drag axis is trying to solve from a UX perspective?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At least for the use cases I've seen, there are often times where you're only using drag and drop for reorderability. Here's the example that led me to make this PR:
I have a music player, and both the queue and playlist views have a reorderable list of songs. I want it to be clear that there's no purpose to dragging other than changing the order, so limiting it to only the y-axis indicates that.
It's also nice on mobile devices in the case where your fingers may move slightly and it keeps the element in line with the list its in
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just thinking out loud here... If the feature was to only "allow dragging" in a direction that has other items you could displace, would that achieve the same goal? In the image you included the user can still drag upwards and leave the drop zone, with this implementation they will be able to drag up until the items becomes the first in the list and then moving the mouse further up (or to the side) won't take it further up (same way that locking to the axis doesn't let it move to the sides)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea that would effectively achieve the same UX. I don't see a use case for one and not the other. Would be more work to implement tho I think
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah it would be harder to implement but would be in line with the lib supporting any special arrangement of items as it does now (e.g. grid, board etc).
If you're keen I am happy to brainstorm, otherwise I can take a crack at it but not sure when I will have time (maybe over the weekend). There will need to be a util that tracks positions of items or something like that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy to take a crack at it, there may be a relatively straightforward solution by using the indices of the items. I'm also interested in potentially converting the library to TS to make editing/maintenance easier but that would cause major conflicts with open PRs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like TS and there were attempts at this in the past but because of all the dom manipulation it always ended up as not adding much safety.
Thats a solid point. I'll try to get this working, and maybe look into that afterwards. Do you have any thoughts on what the config option should be called. I think something along the lines of |
or perhaps call it |
We need to spec it out in more detail. I am writing on my phone right now so a bit limited. In my mind it doesn't limit you to a single drop zone but I don't have all the details fleshed out and maybe there will be no choice but to limit it to a drop zone (probably the case). |
That's a good point, probably best to settle on what it will do before naming it. I feel like it makes the most sense to limit it to a single drop zone (less work to implement, and if we don't do that, then we wouldn't have the benefit of disabling dragging the element out of its drop zone), but I'm open to the idea. I think we would only need to consider the cardinal directions (so 90 degrees), since as far as I'm aware, elements can only be arranged vertically or horizontally. Did you have something in mind for the 45 degree options? |
A grid |
hmm, I was envisioning this being useful for lists, didn't think about a grid. Do you have an example of that? I thought grids were done like the scrabble example in the readme |
Right now the lib makes no assumptions about the layout of dndzones. It is easy to make a grid based example (useful for images or cards). I can make one a bit later |
I came to ask about this feature too to make a basic video timeline with multiple tracks, in my case I would like to dynamically "unlock" timelinednd.movThere is a basic repl of it here: https://svelte.dev/playground/b30dad4700cd49d2afc32b6b0709538e?version=5.2.8
Here is a very basic example: https://svelte.dev/playground/bc32f5b9b6134fe790af3508db5e8893?version=5.2.8 |
Very simple solution that allows users tot limit what axis elements can be dragged on. Addresses #579. I've tested this in an existing project of mine and it seems to work as expected