Skip to content

Conversation

@alpiepho
Copy link

refactor xkcd layout so buttons are always reachable and image doesn't run off screen.

If the image is large, I found that image forces the bottom buttons to be off screen and not reachable. The problem is worse on a Notebook computer.

@andydotxyz
Copy link
Member

This is a good idea, thanks.
Unfortunately as-is this now collapses the image to a tiny size (the min for a ScrollContainer) perhaps it could be big enough to display some of the smaller images by default?
Perhaps using a minsize on the scroller or resizing the window would help?

@alpiepho
Copy link
Author

I went with picking a decent size default xkcd window size and recenter. I tried several ways to resize the image container, but that would not cause the window itself to resize. I also considered resizing the window based on the the image size, but that seemed awkward and would probably introduce a number of edge cases.

Copy link
Member

@andydotxyz andydotxyz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it went a little too far the other way ;)

Good catch on the labels though

form, buttons, x.image))
layout.NewBorderLayout(controlsContainer, nil, nil, nil),
controlsContainer, imageContainer))
w.Resize(fyne.NewSize(1000, 800)) // will limit to screensize
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure this is a sensible start size? it seems very large?
I agree it will (depending on OS) likely limit to screen size, but it doesn't seem like the kind of window that wants to be so big.
Also I don't think we should be requesting center on screen without good reason.

@andydotxyz
Copy link
Member

Do you want to pick this up with a smaller window size?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants