tomusborne / generateblocks

GenerateBlocks is a small collection of lightweight WordPress blocks that can accomplish nearly anything.
https://generateblocks.com
194 stars 20 forks source link

Feature/pattern library multiselect #1149

Closed iansvo closed 10 months ago

iansvo commented 10 months ago

Fixes https://github.com/tomusborne/generateblocks-pro/issues/358

Notes

tomusborne commented 10 months ago

@iansvo I'm seeing some weird behaviors when dragging the items around. It's causing the pattern window to scroll, and it seems the drop area gets lost if you venture too far away while dragging.

https://github.com/tomusborne/generateblocks/assets/20714883/551719b3-b6ea-46a9-9d8f-716ceb13ce82

Tried using Firefox and Brave.

iansvo commented 10 months ago

@tomusborne I'm tempted to say this might have something to do with how the modal is set up, unless there's some scrolljacking going on I'm not aware of.

I can replicate, I have an idea on how this might be adjusted. I'm not sure we need to change the entire library or anything right now either and I think that might be best revisited when we see about the component library concept.

dgwyer commented 10 months ago

Hey Ian, I just took this for a quick spin. Looking good but I ran into the same issues. I added Y-axis locking so the draggable item can only be dragged in the Y-axis. Ref.

This helps a little but I think preventing dragging outside the bounds of the list would be useful too. I'm not sure this is possible with the current library though? What do you think?

https://github.com/tomusborne/generateblocks/assets/1482075/f90a3c36-649b-46ca-803f-165d0466ca67

iansvo commented 10 months ago

@dgwyer yeah I think the axis change makes sense, it seems cleaner for sure! I was thinking about this some more and my opinion (for now anyway) is that we get this in front of people and let them tell us if that kind of thing bothers them.

I think there's potential merit to it for sure, but I'm thinking that we should be cautious for now to not bike shed on this too much. I've just pushed a few layout adjustments that should help this properly scroll too. I notice in your video that the Selected Patterns gets cut off, which is a symptom of how this modal layout was done originally.

More to come soon.

iansvo commented 10 months ago

@tomusborne @dgwyer Latest changes are in which changes the way the modal layout is setup to allow for independent scroll areas and no more sticky/absolute/fixed positioning. Should resolve all scrolling errors (per my tests) and also allow the sidebar to properly scroll as expected.

I think we should do some additional testing and consideration for mobile though. Once you get to about 768px the layout becomes basically unusable.

It seems like we'd want to make sure this works on smaller screens, is that a use case we shouldn't support for some reason?

dgwyer commented 10 months ago

Tested again and it's working better for screens with reduced height.

Couple more observations:

  1. I noticed that when you start dragging an item it 'snaps' away from the mouse cursor which is a little disconcerting (to me). I added sometihng I found to fix this but it only seems to work nicely when the scrollbar is not active. Seems to break when the scrollbar is visible. Not sure why without further investigation. Ref.
  2. The icon used for the drag handle is used in other places throughout GB and GB Pro as a context menu. Maybe we should use something different such as a double set of circles?
  3. It still feels a litlte weird that you can drag items beyond the bounds of the list container. I'm not sure there's a way around this with the current DnD library, as it seems it's mainly designed for maximum flexibility when cross-dragging between lists (which we don't need for reordering patterns)?

I added a commit for items 1. and 2. if it's helpful. 🙂

https://github.com/tomusborne/generateblocks/assets/1482075/f3e2ebf7-288b-4b9b-816a-12b29a04bc00

iansvo commented 10 months ago

I just pushed a few updates:

Here's a quick capture of the functionality as is:

https://github.com/tomusborne/generateblocks/assets/11017412/d0dd5004-f4fc-4dc2-a211-35680d4da3e9

Overall, I think it might be nice to cap the movement at the list bounds, but so far I'm not sure there's an easy way to do that with this library. I think if we're wanting to pursue that we need to just refactor this to use react-dnd directly or a newer alternative like dnd-kit that has support for this kind of thing (and axis alignment).

That's something that could be done in this PR or in a new ticket. I don't think our current implementation would be super hard to move to something else since it's pretty vanilla. So I'm open to the idea of refactoring it to support that behavior now.

I'm tempted to say let's just refactor it so we can eliminate the possible tech debt, since it seems like our needs are going to go beyond this particular library, which also no longer gets active maintenance.

If I had to guess, I'd say a refactor to another library could be done in an afternoon, tops. So I think I'm leaning towards let's just replace it now unless we're happy with how it works currently.

tomusborne commented 10 months ago

It's definitely better, but I always feel like drag-n-drop is one of those things that has to feel good to bring value, otherwise I prefer up/down arrows.

The only thing about it right now that I don't like is that the item goes to the bottom of my cursor, and I have to make sure I drop it based on my cursor (not the item I'm dragging).

Now is the time to choose a different library if it offers better usability, especially if it's only an afternoon of work. We should do that if there's a better alternative for what we need here.

iansvo commented 10 months ago

@tomusborne Latest update is pushed, refactored to dnd-kit and made sure the axis/bounding restrictions are in place.

https://github.com/tomusborne/generateblocks/assets/11017412/800f3b20-8d86-4197-87aa-f92e0f3ca3e5

Changes

Let me know what you think. I'm pretty happy with this, buttery smooth, no weird positioning BS (DragCanvas be damned). UX is pretty nice, I'm stoked to get this in people's hands.

tomusborne commented 10 months ago

So much better now, @iansvo!

Last small bug I'm seeing is the extra space below the buttons in the UI:

https://github.com/tomusborne/generateblocks/assets/20714883/d4e5ee37-7604-4055-bac3-0181bedeb7da

Ideally all of those buttons should sit right up against the bottom of the row.

Other than that it looks good to merge!

iansvo commented 10 months ago

@tomusborne Fixed, had to remove the "compact" size to sort that out apparently.

I also simplified the grid-template-columns to prevent an overflow from happening in some situations.

iansvo commented 10 months ago

@tomusborne Latest change is up:

Notes

Video

https://github.com/tomusborne/generateblocks/assets/11017412/324be1a0-5676-41e7-bdf9-5ee2e9d38cec

Brave image

Firefox image

Safari

image

Let me know if you have any questions.

tomusborne commented 10 months ago

Beautiful! Looks and feels great, and even works as you navigate through categories.

Good to merge!