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

Entry without scrollable content prevents scrolling of outside scroller #1939

Closed
Jacalz opened this issue Feb 10, 2021 · 9 comments
Closed
Assignees
Labels
blocker Items that would block a forthcoming release bug Something isn't working

Comments

@Jacalz
Copy link
Member

Jacalz commented Feb 10, 2021

Describe the bug:

An empty (or just not with enough content to scroll) will prevent the scrolling of an external scroller even though there is nothing to scroll within it.

To Reproduce:

Steps to reproduce the behavior:

  1. Make the window large enough that it can be scrolled.
  2. Move the mouse to be over the label.
  3. Scroll and notice that it works as expected.
  4. Move the mouse to be over the entry.
  5. Scroll and notice that it does not scroll.

Screenshots:

scolling-not-in-entry

Example code:

package main

import (
	"fyne.io/fyne/v2/app"
	"fyne.io/fyne/v2/container"
	"fyne.io/fyne/v2/widget"
)

func main() {
	a := app.New()
	w := a.NewWindow("Entry scroll")

	w.SetContent(container.NewScroll(
		container.NewVBox(
			widget.NewEntry(),
			widget.NewLabel("Scroll on me!"),
		),
	),
	)

	w.ShowAndRun()
}

Device (please complete the following information):

  • OS: Linux
  • Version: 5.10.12
  • Go version: 1.15.7
  • Fyne version: 2.0.0
@Jacalz Jacalz added bug Something isn't working blocker Items that would block a forthcoming release labels Feb 10, 2021
@stuartmscott
Copy link
Member

Agreed this is an issue, though it is not specific to Entry - any nested scrollers will encounter this problem where the inner scroller reaches its limit but cannot defer the scroll event to the outer scroller

@andydotxyz
Copy link
Member

I think you would avoid it by specifying Entry.Wrapping = fyne.TextWrapNone

@andydotxyz
Copy link
Member

I don't really think this is a blocking bug.
I'm even tempted to say it's not a bug, this is a (poor?) design.
Is this an enhancement?

I don't think we should change this behaviour under a bug fix release, it is a larger issues at work here that could result in bahaviour changes not intended.

@Jacalz
Copy link
Member Author

Jacalz commented Feb 11, 2021

Sure, maybe not a bug, but certainly very annoying. While the example gif above of course does not show it, this is making Sparta almost unusable when you try to scroll the form full of entry widgets.

I'd almost say that it is a blocker just because the entry widgets make the issue so much more noticeable.

@andydotxyz
Copy link
Member

Turning off wrapping did not help?

@Jacalz
Copy link
Member Author

Jacalz commented Feb 11, 2021

Turning off wrapping did not help?

Haven’t tried yet. Just posting a reply from my phone.

@andydotxyz
Copy link
Member

It won't fix it, I realised that the type of scroll is not checked - only the presence of a scroller.

I would propose that this is downgraded from blocker by adding something like

	objects := []fyne.CanvasObject{box, line, e.content}
	if e.Wrapping != fyne.TextWrapOff {
		e.scroll = widget.NewScroll(e.content)
		objects = []fyne.CanvasObject{box, line, e.scroll}
	}

This goes in Entry.CreateRenderer for now, it may need to be addressed in updateScrollDirections as well for a complete solution.

@andydotxyz
Copy link
Member

The alternative would be to add some smarts to mouseScrolled in internal/driver/glfw/window.go (plus same in mobile).

	co, _, _ := w.findObjectAtPositionMatching(w.canvas, w.mousePos, func(object fyne.CanvasObject) bool {
		if s, ok := object.(*intWidget.Scroll); ok {
			return s.Direction != intWidget.ScrollNone
		}

		_, ok := object.(fyne.Scrollable)
		return ok
	})

Of course this puts a dependency back on widget, because the direction is not exposed through the Scrollable interface...

@andydotxyz andydotxyz self-assigned this Feb 12, 2021
andydotxyz added a commit to andydotxyz/fyne that referenced this issue Feb 21, 2021
When setting up an entry do not expose a scroll container unless multiline or scrolling is specified.
Fixes fyne-io#1939
@andydotxyz
Copy link
Member

The agreed workaround is on develop and release/v2.0.x

andydotxyz added a commit that referenced this issue Feb 26, 2021
When setting up an entry do not expose a scroll container unless multiline or scrolling is specified.
Fixes #1939
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocker Items that would block a forthcoming release bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants