Mobile: Don't let horizontally draggable block vertical dragging#6256
Mobile: Don't let horizontally draggable block vertical dragging#6256MaxGyver83 wants to merge 3 commits intofyne-io:developfrom
Conversation
|
coverage: 60.348% (-0.06%) from 60.412% — MaxGyver83:dragging-other-direction into fyne-io:develop |
| return false | ||
| }) | ||
| var scrollOtherDirection fyne.CanvasObject | ||
| if scr, ok := co.(*container.Scroll); ok { |
There was a problem hiding this comment.
This may solve the problem for specifically *container.Scroll, but not for other fyne.Scrollable widgets. This may be a good stop-gap but eventually we may need a new Scrollable interface that allows any user-code widget to tell the driver which direction it supports scrolling
There was a problem hiding this comment.
Yes, this is mainly to fix my issue/use case. This could be extended to other draggables but I wasn't sure if this feature makes sense for other draggables at all.
There was a problem hiding this comment.
I think I misunderstood your comment. I was thinking about other Draggables (like sliders). But you are concerned about user-defined Scrollables (which must be Draggable as well to be relevant for this PR). Yes, I think something like a DragDirection/ScrollDirection function sounds useful.
There was a problem hiding this comment.
@dweymouth : Have you ever created a custom scrollable widget? Or seen one in other people's code? This would help me to understand what is really needed. What's the benefit at all of a custom scrollable widget compared to any widget wrapped into a container.Scroll?
There was a problem hiding this comment.
Strictly speaking we don't support extending containers, so maybe it's OK that this does exactly only as much as it does?
There was a problem hiding this comment.
For now. But as fyne.Scrollable is an interface, a widget can implement it without using container.Scroll. (e.g. slider already implements it AFAIK). Heavy custom widgets may be able to manage their internal scroll viewport state smarter than the generic *container.Scroll, having internal knowledge of the objects and layout. So I think it is worth at least keeping an open ticket for revisiting this for all scrollables, but good with merging this now as a stop-gap.
There was a problem hiding this comment.
Regarding sliders: When you have a horizontal slider in a vertically scrollable container and you drag it to the left or the right, I'm not sure if you want the full container to move upwards or downwards at the same time. On the other hand, supporting this would fix #5090, too. But even then it's still undesirable that the sliders move (or even jump as in the video linked there) when you try to drag the container content.
Maybe it makes sense to check if the drag movement is rather vertical or rather horizontal (p.e. greater or less than 45 degree) and then it's considered only a vertical or a horizontal movement.
|
I can see this fixes a specific relating to Scrollers specifically - but there is a super/l-set problem here where scrolling two things in the same direction could be valid and the child hitting the bottom should then propagate to the parent. For a more complete fix I think we need a runtime check on whether a Scrollable item is currently scrollable in the direction being performed? |
This might be useful. But it might also be annoying: When you scroll through a list of options and then at the end of list you start scrolling the full page accidentally. Isn't this a separate issue worth being discussed in its own GitHub issue and possibly fixed in a separate PR? This PR doesn't break this use case and could be the base for further experiments after merging it. |
|
I've just realised that this only works when both scrollers have only a single direction set. What if the parent scroll is scroll-both and the child consumes only 1 direction... |
Yes, sounds reasonable. Do you to have this change in this PR? |
|
If you added the it would probably round out the *Scroller stacking improvements and might be something we can more easily build on for the more generic case |
OK, I have added it in commit 04c4ccf. |
4 participants
Description:
When you are dragging a
container.Scrollthat is scrollable/draggable in only one direction, check if there is anothercontainer.Scroll(at a lower layer) that is draggable in the other direction and then call thedragCallbackon both of them.Fixes #6158.
Checklist: