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

SetMinSize() on anything with a MinSize() #1581

Closed
AlbinoGeek opened this issue Nov 19, 2020 · 17 comments
Closed

SetMinSize() on anything with a MinSize() #1581

AlbinoGeek opened this issue Nov 19, 2020 · 17 comments
Labels
bug Something isn't working

Comments

@AlbinoGeek
Copy link
Contributor

Is your feature request related to a problem? Please describe:

Currently, I find myself "extending" widgets literally only to override the MinSize()

Examples include List and Table when used in any compacting layout.

Is it possible to construct a solution with the existing API?

Perhaps I don't understand this question.

Describe the solution you'd like to see:

	t := time.Now()
	data := []struct{ Name, Size, Format, Created, Updated, Accessed string }{
		{"index.html", "21.4 KiB", "text/html", t.AddDate(0, 0, -2).String(), t.AddDate(0, 0, -1).String(), t.String()},
	}

	createLabel := func() fyne.CanvasObject {
		label := widget.NewLabel("X")
		label.SetMinSize(fyne.NewSize(100, 20))
		return label
	}

	list := widget.NewList(func() int { return len(data) }, createLabel, func(lii widget.ListItemID, f fyne.CanvasObject) {
		f.(*widget.Label).Text = data[lii].Name
	})
	list.SetMinSize(fyne.NewSize(200, 100))

	table := widget.NewTable(func() (int, int) { return len(data), 6 }, createLabel, func(tci widget.TableCellID, f fyne.CanvasObject) {
		atom := data[tci.Row]
		switch tci.Col {
		case 1:
			f.(*widget.Label).Text = atom.Name
		case 2:
			f.(*widget.Label).Text = atom.Size
		case 3:
			f.(*widget.Label).Text = atom.Format
		case 4:
			f.(*widget.Label).Text = atom.Created
		case 5:
			f.(*widget.Label).Text = atom.Updated
		case 6:
			f.(*widget.Label).Text = atom.Accessed
		}
	})
	table.SetMinSize(fyne.NewSize(600, 300))

	w.SetContent(container.NewVBox(
		widget.NewCard("Title", "Subtitle", list),
		widget.NewCard("Title2", "Subtitle2", table),
	))
@andydotxyz
Copy link
Member

The Minimum size of any widget is defined as the size which it cannot be smaller than. This is essetial for laying out complex layouts whilst allowing users to resize windows up or down. Whhen you change the MinSize of an item it cannot become smaller. This is a big problem when scaling down to small screen sizes, like for mobile devices.

Table cells and items of List and Tree are special - they are sized based on the template MinSize. Instead of using your desired MinSize hack you should set an indicative template value instead:

		label := widget.NewLabel("Average string")

That is how they are designed.
In v1.4.1 there is an additional Table.SetColumnWidth that gives more fine-grained control.

For all elements their actual size is set by the container as a result of various calculations.
If you do not want your widget to shrink do not put it in a layout that shrinks the content.
There are various layouts available or you can provide your own.

@andydotxyz andydotxyz added the not an issue This doesn't seem right label Nov 19, 2020
@AlbinoGeek
Copy link
Contributor Author

Right, but this also is the issue of Table and List currently have a MinSize that is less than one template element tall -- resulting in an unusable table/list when put inside a compacting layout such as VBox or Card.

@andydotxyz
Copy link
Member

Developers choose the layouts in containers as well as the widgets that get put in them - so can you apply a layout that expands the component you would like to be larger?

Without more concrete example or description of what cannot be done with the current design it is difficult to provide a more constructive solution, sorry.

@stuartmscott
Copy link
Member

Table and List currently have a MinSize that is less than one template element tall

@andydotxyz there is an inconsistency here:

func (l *listRenderer) MinSize() fyne.Size {
	return l.scroller.MinSize()
}

func (t *tableRenderer) MinSize() fyne.Size {
	return t.cellSize
}

func (r *treeRenderer) MinSize() (min fyne.Size) {
	min = r.scroller.MinSize()
	min = min.Max(r.tree.branchMinSize)
	min = min.Max(r.tree.leafMinSize)
	return
}

I think Tree does it correctly

@andydotxyz
Copy link
Member

andydotxyz commented Nov 19, 2020

There will be some inconsistency, but you're right these have deviated too far.
List does have the correct width - it is set indirectly by the Scroll being in vertical-only mode.

Possible changes:

  • List minHeight to be equal to template item height
  • Table minHeight and minWidth to match the template cell (plus padding)

That will fix up the inconsistency I think.

@AlbinoGeek
Copy link
Contributor Author

^ Agreed, at least that would make them usable in a compact layout.

Currently, Table inside VBox gives you a .5 height and (seemingly unrelated) width.

@andydotxyz
Copy link
Member

andydotxyz commented Nov 19, 2020

Looking into this further @stuartmscott, I think that Table is calculating it correctly - I thought it was maybe missing padding but seems not.
List min height may still be wrong.

I have re-run the image tests requesting that tables be minimum size, and they come out as expected...
theme_initial
I don't know what issue you are hitting @AlbinoGeek but it does not seem to be caused by Table.MinSize()

@AlbinoGeek
Copy link
Contributor Author

AlbinoGeek commented Nov 19, 2020

@andydotxyz

image

  • Very squished
	data := [][]string{
		{"Foo", "Bar", "Baz"},
		{"Foo", "Bar", "Baz"},
		{"Foo", "Bar", "Baz"},
	}
	table := widget.NewTable(
		func() (int, int) { return len(data), len(data[0]) },
		func() fyne.CanvasObject {
			return widget.NewLabel("@@@")
		},
		func(tci widget.TableCellID, f fyne.CanvasObject) {
			f.(*widget.Label).SetText(data[tci.Row][tci.Col])
		},
	)

	tc := container.NewAppTabs(
		container.NewTabItem("Table Issue",
			container.NewVBox(
				widget.NewCard("Title", "Subtitle", table),
			),
		),
	)
	w.SetContent(tc)

The issue pops up once you put Table inside a Card inside a VBox


Reducing Variables, without the AppTabs

image

  • Very squished
	w.SetContent(
		container.NewVBox(
			widget.NewCard("Title", "Subtitle", table),
		),
	)

Only a VBox

image

  • Works as expected
	w.SetContent(
		container.NewVBox(
			table,
		),
	)

Only a Card

image

  • Significantly too much padding
	w.SetContent(
		widget.NewCard("Title", "Subtitle", table),
	)

@stuartmscott
Copy link
Member

stuartmscott commented Nov 19, 2020

@AlbinoGeek thanks for sharing this code

@andydotxyz, @okratitan the collections should all return scroller.MinSize().Max(cell|item|branch|leaf.MinSize()+padding)

@andydotxyz
Copy link
Member

andydotxyz commented Nov 19, 2020

Aha! Thanks @AlbinoGeek looks like this is a Card MinSize/Layout bug!
I can get that fixed in the morning

@andydotxyz andydotxyz added bug Something isn't working and removed not an issue This doesn't seem right labels Nov 20, 2020
@andydotxyz
Copy link
Member

The bug reported is now fixed on release/v1.4.x, and will be available in v1.4.2 in a few weeks.
Please test it and let me know if we can mark this ticket is resolved now @AlbinoGeek.

@AlbinoGeek
Copy link
Contributor Author

Just noticed that canvas.Text has a SetMinSize and also draws at the correct baseline, where widget.Label does not.

@andydotxyz
Copy link
Member

the correct baseline is only true as long as you don't expand the height of the text I think?
Because it is rendering only the pixels necessary. It has no vertical alignment so it's your usage of it that is correctly aligning, rather than the text ;)

@andydotxyz
Copy link
Member

SetMinSize is required for Text and Image because the driver will discover the minimum size when it actually renders (due to font loading or image parsing).
In this regard canvas primitives are different to widgets - they don't really have implicit minimum (i.e. how big should a line be) so it allows the minimum setting.

@AlbinoGeek
Copy link
Contributor Author

AlbinoGeek commented Nov 22, 2020

Gitcha! I can then say this is now fixed, save for the vertical baseline issue [covered elsewhere].

@ivoras
Copy link
Contributor

ivoras commented Feb 25, 2024

Resurrecting this issue because entries in dialogs are sized weirdly. This is how a dialog.ShowForm() looks like with this code:

	dialog.ShowForm(
		"Password",
		"Ok",
		"Cancel",
		[]*widget.FormItem{
			{Text: "Password", Widget: passwordEntry},
		},
		func(b bool) {
			if !b {
				return
			}
			win.doOpenFile(frc.URI().String(), passwordEntry.Text)
		},
		win.win,
	)

image

The password entry is too small, and there's no SetMinSize() to correctly size it. But it gets even worse when the text label is longer:

image

@Jacalz
Copy link
Member

Jacalz commented Feb 25, 2024

@ivoras Please don’t resurrect old issues that have been closed. It is better to open a new issue or a discussion depending on the context. In this case however, there is a .Resize() method on the dialog and that will solve your issues mentioned above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants